Skip to content

Commit 02a30fa

Browse files
fix: prevent ConcurrentModificationException in copy() when setAttribute() is called concurrently (BUG-8620)
Agent-Logs-Url: https://github.com/optimizely/android-sdk/sessions/9734d248-0b4a-4815-a88f-e54529087a55 Co-authored-by: muzahidul-opti <129880873+muzahidul-opti@users.noreply.github.com>
1 parent bb99641 commit 02a30fa

3 files changed

Lines changed: 97 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ See [Feature Rollout docs](https://support.optimizely.com/hc/en-us/articles/4555
1616
### Fixes and Improvements
1717

1818
- Update cmab error handling ([#522](https://github.com/optimizely/android-sdk/pull/522))
19+
- Fix `ConcurrentModificationException` in `decide()` when `setAttribute()` is called concurrently ([#507](https://github.com/optimizely/android-sdk/issues/507), BUG-8620)
1920

2021

2122
## 5.1.1

android-sdk/src/main/java/com/optimizely/ab/android/sdk/OptimizelyUserContextAndroid.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,31 @@ public OptimizelyUserContextAndroid(@NonNull Optimizely optimizely,
8484
super(optimizely, userId, attributes, forcedDecisionsMap, qualifiedSegments, shouldIdentifyUser);
8585
}
8686

87+
/**
88+
* Returns a copy of this user context with all current attributes, forced decisions, and qualified segments.
89+
*
90+
* <p>This override holds the mutex of the synchronized attributes map (and qualified segments list,
91+
* if present) while the copy is being made. This prevents a {@link java.util.ConcurrentModificationException}
92+
* that would otherwise occur when another thread calls {@link #setAttribute} concurrently, because
93+
* {@link java.util.Collections#synchronizedMap} only guards individual operations — not the bulk
94+
* iteration that happens inside {@code new HashMap<>(attributes)} during the copy constructor.</p>
95+
*
96+
* @return A new {@link OptimizelyUserContext} with the same state as this instance.
97+
*/
98+
@Override
99+
public OptimizelyUserContext copy() {
100+
Map<String, Object> attrs = getAttributes();
101+
List<String> segs = getQualifiedSegments();
102+
synchronized (attrs) {
103+
if (segs == null) {
104+
return super.copy();
105+
}
106+
synchronized (segs) {
107+
return super.copy();
108+
}
109+
}
110+
}
111+
87112
// ==================================================================
88113
// [SYNCHRONOUS DECIDE METHODS]
89114
// - "decideSync" methods skip async decision like cmab for backward compatibility.

android-sdk/src/test/java/com/optimizely/ab/android/sdk/OptimizelyUserContextAndroidTest.java

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,14 @@
3535
import java.util.HashMap;
3636
import java.util.List;
3737
import java.util.Map;
38+
import java.util.concurrent.CountDownLatch;
39+
import java.util.concurrent.TimeUnit;
40+
import java.util.concurrent.atomic.AtomicReference;
3841

3942
import static org.junit.Assert.assertEquals;
4043
import static org.junit.Assert.assertNotNull;
44+
import static org.junit.Assert.assertNull;
45+
import static org.junit.Assert.assertTrue;
4146
import static org.mockito.ArgumentMatchers.any;
4247
import static org.mockito.ArgumentMatchers.eq;
4348
import static org.mockito.Mockito.doReturn;
@@ -410,4 +415,70 @@ public void testDecideAllBlocking_withoutOptions() throws Exception {
410415
assertEquals(mockDecisionsMap, result);
411416
}
412417

418+
// ===========================================
419+
// Regression test for BUG-8620:
420+
// ConcurrentModificationException in copy() when setAttribute() is called concurrently
421+
// ===========================================
422+
423+
@Test
424+
public void testCopy_noConcurrentModificationExceptionWithConcurrentSetAttribute()
425+
throws InterruptedException {
426+
Map<String, Object> attributes = new HashMap<>();
427+
for (int i = 0; i < 50; i++) {
428+
attributes.put("key" + i, "value" + i);
429+
}
430+
431+
OptimizelyUserContextAndroid userContext = new OptimizelyUserContextAndroid(
432+
mockOptimizely,
433+
TEST_USER_ID,
434+
attributes
435+
);
436+
437+
int numThreads = 10;
438+
int iterations = 200;
439+
CountDownLatch startLatch = new CountDownLatch(1);
440+
CountDownLatch doneLatch = new CountDownLatch(numThreads * 2);
441+
AtomicReference<Throwable> error = new AtomicReference<>();
442+
443+
// Writer threads: call setAttribute() concurrently while copy() runs
444+
for (int t = 0; t < numThreads; t++) {
445+
final int threadNum = t;
446+
new Thread(() -> {
447+
try {
448+
startLatch.await();
449+
for (int i = 0; i < iterations; i++) {
450+
userContext.setAttribute("concurrentKey" + threadNum + "_" + i, "val");
451+
}
452+
} catch (Throwable e) {
453+
error.compareAndSet(null, e);
454+
} finally {
455+
doneLatch.countDown();
456+
}
457+
}).start();
458+
}
459+
460+
// Reader threads: call copy() concurrently while setAttribute() runs
461+
for (int t = 0; t < numThreads; t++) {
462+
new Thread(() -> {
463+
try {
464+
startLatch.await();
465+
for (int i = 0; i < iterations; i++) {
466+
userContext.copy();
467+
}
468+
} catch (Throwable e) {
469+
error.compareAndSet(null, e);
470+
} finally {
471+
doneLatch.countDown();
472+
}
473+
}).start();
474+
}
475+
476+
startLatch.countDown();
477+
boolean completed = doneLatch.await(15, TimeUnit.SECONDS);
478+
479+
assertTrue("Threads did not complete within timeout", completed);
480+
assertNull("Unexpected exception during concurrent copy/setAttribute: " + error.get(),
481+
error.get());
482+
}
483+
413484
}

0 commit comments

Comments
 (0)