activemq-0002/0003: candidateConsumers List→Set O(1); ENDED_XA List→Set O(1)
This commit is contained in:
parent
e0486b7301
commit
23fbfdca44
3 changed files with 193 additions and 0 deletions
|
|
@ -0,0 +1,24 @@
|
|||
--- a/activemq-broker/src/main/java/org/apache/activemq/network/DemandForwardingBridgeSupport.java
|
||||
+++ b/activemq-broker/src/main/java/org/apache/activemq/network/DemandForwardingBridgeSupport.java
|
||||
@@ -1558,7 +1558,8 @@ public abstract class DemandForwardingBridgeSupport implements NetworkBridge, Br
|
||||
List<ConsumerId> candidateConsumers = consumerInfo.getNetworkConsumerIds();
|
||||
+ Set<ConsumerId> candidateSet = new HashSet<>(candidateConsumers); /* O(1) lookup in matchFound; was O(C) ArrayList.contains */
|
||||
Collection<Subscription> currentSubs = getRegionSubscriptions(consumerInfo.getDestination());
|
||||
for (Subscription sub : currentSubs) {
|
||||
- List<ConsumerId> networkConsumers = sub.getConsumerInfo().getNetworkConsumerIds();
|
||||
+ List<ConsumerId> networkConsumers = sub.getConsumerInfo().getNetworkConsumerIds(); /* note: cached at call site in callers */
|
||||
if (!networkConsumers.isEmpty()) {
|
||||
- if (matchFound(candidateConsumers, networkConsumers)) {
|
||||
+ if (matchFound(candidateSet, networkConsumers)) {
|
||||
if (isInActiveDurableSub(sub)) {
|
||||
|
||||
@@ -1615,9 +1615,9 @@ public abstract class DemandForwardingBridgeSupport implements NetworkBridge, Br
|
||||
- private boolean matchFound(List<ConsumerId> candidateConsumers, List<ConsumerId> networkConsumers) {
|
||||
+ private boolean matchFound(Set<ConsumerId> candidateConsumers, List<ConsumerId> networkConsumers) {
|
||||
boolean found = false;
|
||||
for (ConsumerId aliasConsumer : networkConsumers) {
|
||||
- if (candidateConsumers.contains(aliasConsumer)) { /* was O(C) ArrayList.contains per alias — defect */
|
||||
+ if (candidateConsumers.contains(aliasConsumer)) { /* O(1) HashSet.contains */
|
||||
found = true;
|
||||
break;
|
||||
}
|
||||
|
|
@ -0,0 +1,22 @@
|
|||
--- a/activemq-client/src/main/java/org/apache/activemq/TransactionContext.java
|
||||
+++ b/activemq-client/src/main/java/org/apache/activemq/TransactionContext.java
|
||||
@@ -70,7 +70,7 @@ public class TransactionContext implements XAResource {
|
||||
// XATransactionId -> ArrayList of TransactionContext objects
|
||||
- private final static HashMap<TransactionId, List<TransactionContext>> ENDED_XA_TRANSACTION_CONTEXTS =
|
||||
- new HashMap<TransactionId, List<TransactionContext>>();
|
||||
+ private final static HashMap<TransactionId, Set<TransactionContext>> ENDED_XA_TRANSACTION_CONTEXTS =
|
||||
+ new HashMap<TransactionId, Set<TransactionContext>>(); /* values are Set for O(1) contains in isInXATransaction */
|
||||
|
||||
@@ -94,7 +94,7 @@ public class TransactionContext implements XAResource {
|
||||
public boolean isInXATransaction() {
|
||||
if (transactionId != null && transactionId.isXATransaction()) {
|
||||
return true;
|
||||
} else {
|
||||
synchronized(ENDED_XA_TRANSACTION_CONTEXTS) {
|
||||
- for(List<TransactionContext> transactions : ENDED_XA_TRANSACTION_CONTEXTS.values()) {
|
||||
- if (transactions.contains(this)) { /* was O(C) ArrayList.contains per XA txn — defect */
|
||||
+ for(Set<TransactionContext> transactions : ENDED_XA_TRANSACTION_CONTEXTS.values()) {
|
||||
+ if (transactions.contains(this)) { /* O(1) HashSet.contains */
|
||||
return true;
|
||||
}
|
||||
}
|
||||
147
defects/activemq/unit/ActiveMQTest2.java
Normal file
147
defects/activemq/unit/ActiveMQTest2.java
Normal file
|
|
@ -0,0 +1,147 @@
|
|||
import java.util.*;
|
||||
|
||||
/**
|
||||
* CWE-407 unit tests for Apache ActiveMQ defects 0002 and 0003.
|
||||
*
|
||||
* activemq-0002: DemandForwardingBridgeSupport.java duplicateSuppressionIsRequired()
|
||||
* candidateConsumers is an ArrayList; matchFound() calls candidateConsumers.contains()
|
||||
* — O(C) per alias per subscription — O(S×N×C) total per network connect.
|
||||
* Fix: convert candidateConsumers to HashSet before the outer sub-scan loop for O(1) lookup.
|
||||
*
|
||||
* activemq-0003: TransactionContext.java isInXATransaction()
|
||||
* ENDED_XA_TRANSACTION_CONTEXTS values are ArrayList<TransactionContext>;
|
||||
* transactions.contains(this) — O(C) linear scan per XA txn bucket.
|
||||
* Fix: change values to Set<TransactionContext> for O(1) contains.
|
||||
*/
|
||||
public class ActiveMQTest2 {
|
||||
|
||||
// Simulate defect: matchFound with ArrayList candidateConsumers
|
||||
static boolean matchFound_list(List<Integer> candidateConsumers, List<Integer> networkConsumers) {
|
||||
for (Integer alias : networkConsumers) {
|
||||
if (candidateConsumers.contains(alias)) return true; // O(C) — defect
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
// Simulate fix: matchFound with HashSet candidateConsumers
|
||||
static boolean matchFound_set(Set<Integer> candidateSet, List<Integer> networkConsumers) {
|
||||
for (Integer alias : networkConsumers) {
|
||||
if (candidateSet.contains(alias)) return true; // O(1)
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
static void testActiveMQ0002() throws Exception {
|
||||
int C = 500; // candidateConsumers per remote subscription
|
||||
int S = 2000; // current subscriptions in the region
|
||||
int N = 3; // networkConsumerIds per existing sub (small, 1–3)
|
||||
|
||||
List<Integer> candidateList = new ArrayList<>(C);
|
||||
for (int i = 0; i < C; i++) candidateList.add(i);
|
||||
Set<Integer> candidateSet = new HashSet<>(candidateList);
|
||||
|
||||
// Build S existing subs, each with N networkConsumerIds (not overlapping — full scan)
|
||||
List<List<Integer>> subs = new ArrayList<>(S);
|
||||
for (int s = 0; s < S; s++) {
|
||||
List<Integer> nc = new ArrayList<>(N);
|
||||
for (int n = 0; n < N; n++) nc.add(C + s * N + n); // no overlap → full C scan each time
|
||||
subs.add(nc);
|
||||
}
|
||||
|
||||
// correctness: last sub has a matching id → find it
|
||||
subs.get(S - 1).set(0, candidateList.get(0));
|
||||
boolean r1 = false, r2 = false;
|
||||
for (List<Integer> nc : subs) {
|
||||
if (matchFound_list(candidateList, nc)) { r1 = true; break; }
|
||||
}
|
||||
for (List<Integer> nc : subs) {
|
||||
if (matchFound_set(candidateSet, nc)) { r2 = true; break; }
|
||||
}
|
||||
assert r1 == r2 : "list and set must agree: " + r1 + " vs " + r2;
|
||||
|
||||
// Reset: no match case (worst case — full scan)
|
||||
subs.get(S - 1).set(0, C + S * N + 99);
|
||||
int REPS = 20;
|
||||
|
||||
long t0 = System.nanoTime();
|
||||
for (int r = 0; r < REPS; r++) {
|
||||
for (List<Integer> nc : subs) matchFound_list(candidateList, nc);
|
||||
}
|
||||
long tList = System.nanoTime() - t0;
|
||||
|
||||
t0 = System.nanoTime();
|
||||
for (int r = 0; r < REPS; r++) {
|
||||
for (List<Integer> nc : subs) matchFound_set(candidateSet, nc);
|
||||
}
|
||||
long tSet = System.nanoTime() - t0;
|
||||
|
||||
double ratio = (double) tList / tSet;
|
||||
System.out.printf("activemq-0002: list=%.3fs set=%.3fs ratio=%.1f×%n",
|
||||
tList / 1e9, tSet / 1e9, ratio);
|
||||
assert ratio > 4 : "Expected >4× speedup, got " + ratio;
|
||||
System.out.println("PASS activemq-0002");
|
||||
}
|
||||
|
||||
// Simulate defect: isInXATransaction with List values
|
||||
static boolean isInXATransaction_list(Map<Integer, List<Integer>> endedContexts, int self) {
|
||||
for (List<Integer> transactions : endedContexts.values()) {
|
||||
if (transactions.contains(self)) return true; // O(C) — defect
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
// Simulate fix: isInXATransaction with Set values
|
||||
static boolean isInXATransaction_set(Map<Integer, Set<Integer>> endedContexts, int self) {
|
||||
for (Set<Integer> transactions : endedContexts.values()) {
|
||||
if (transactions.contains(self)) return true; // O(1)
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
static void testActiveMQ0003() throws Exception {
|
||||
int T = 50; // ended XA transaction buckets
|
||||
int C = 200; // contexts per bucket
|
||||
|
||||
Map<Integer, List<Integer>> listMap = new LinkedHashMap<>();
|
||||
Map<Integer, Set<Integer>> setMap = new LinkedHashMap<>();
|
||||
for (int t = 0; t < T; t++) {
|
||||
List<Integer> lst = new ArrayList<>(C);
|
||||
Set<Integer> set = new LinkedHashSet<>(C);
|
||||
for (int c = 0; c < C; c++) {
|
||||
int ctx = t * C + c;
|
||||
lst.add(ctx);
|
||||
set.add(ctx);
|
||||
}
|
||||
listMap.put(t, lst);
|
||||
setMap.put(t, set);
|
||||
}
|
||||
|
||||
// self not in any bucket → worst case (full scan)
|
||||
int self = T * C + 999;
|
||||
|
||||
// correctness: not present
|
||||
assert !isInXATransaction_list(listMap, self);
|
||||
assert !isInXATransaction_set(setMap, self);
|
||||
|
||||
int REPS = 10_000;
|
||||
long t0 = System.nanoTime();
|
||||
for (int r = 0; r < REPS; r++) isInXATransaction_list(listMap, self);
|
||||
long tList = System.nanoTime() - t0;
|
||||
|
||||
t0 = System.nanoTime();
|
||||
for (int r = 0; r < REPS; r++) isInXATransaction_set(setMap, self);
|
||||
long tSet = System.nanoTime() - t0;
|
||||
|
||||
double ratio = (double) tList / tSet;
|
||||
System.out.printf("activemq-0003: list=%.3fs set=%.3fs ratio=%.1f×%n",
|
||||
tList / 1e9, tSet / 1e9, ratio);
|
||||
assert ratio > 5 : "Expected >5× speedup, got " + ratio;
|
||||
System.out.println("PASS activemq-0003");
|
||||
}
|
||||
|
||||
public static void main(String[] args) throws Exception {
|
||||
testActiveMQ0002();
|
||||
testActiveMQ0003();
|
||||
System.out.println("ALL PASS");
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue