Skip to content

Commit 7a42160

Browse files
turtledreamsturtledreams
authored
Merge pull request #512 from Countly/internal-lock-rc
Internal lock RC
2 parents 5a4808f + 74e116d commit 7a42160

File tree

3 files changed

+482
-226
lines changed

3 files changed

+482
-226
lines changed

‎sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,6 +545,80 @@ public void validateMergeReceivedResponse() throws Exception {
545545
Assert.assertEquals(0, ((JSONObject) vals.get("t")).length());
546546
}
547547

548+
/**
549+
* Concurrency test: simulate rapid merge operations while concurrently reading values
550+
* to ensure no ConcurrentModificationException or data corruption occurs after introducing
551+
* fine-grained remoteConfigLock and CopyOnWriteArrayList for callbacks.
552+
*/
553+
@Test
554+
public void concurrentMergeAndReads() throws Exception {
555+
CountlyConfig cc = new CountlyConfig(TestUtils.getContext(), "appkey", "http://test.count.ly").setDeviceId("dev1").setLoggingEnabled(true);
556+
cc.immediateRequestGenerator = new ImmediateRequestGenerator() {
557+
@Override public ImmediateRequestI CreateImmediateRequestMaker() { return (a,b,c,d,e,f,g) -> { if (f!=null) f.callback(null); }; }
558+
@Override public ImmediateRequestI CreatePreflightRequestMaker() { return (a,b,c,d,e,f,g) -> { if (f!=null) f.callback(null); }; }
559+
};
560+
Countly countly = new Countly().init(cc);
561+
562+
RemoteConfigValueStore rcvsA = RemoteConfigValueStore.dataFromString("{\"k1\":123,\"k2\":\"v2\"}", false);
563+
RemoteConfigValueStore rcvsB = RemoteConfigValueStore.dataFromString("{\"k2\":777,\"k3\":{}}", false);
564+
RemoteConfigValueStore rcvsC = RemoteConfigValueStore.dataFromString("{\"k4\":true,\"k5\":55.5}", false);
565+
566+
RemoteConfigValueStore[] arr = new RemoteConfigValueStore[] { rcvsA, rcvsB, rcvsC };
567+
568+
final int MERGE_THREADS = 3;
569+
final int READ_THREADS = 4;
570+
final int MERGE_OPS = 150;
571+
final int READ_OPS = 600;
572+
573+
Thread[] mergers = new Thread[MERGE_THREADS];
574+
Thread[] readers = new Thread[READ_THREADS];
575+
final Exception[] failure = { null };
576+
577+
for (int i = 0; i < MERGE_THREADS; i++) {
578+
final int idx = i;
579+
mergers[i] = new Thread(() -> {
580+
try {
581+
for (int j = 0; j < MERGE_OPS; j++) {
582+
RemoteConfigValueStore pick = arr[(idx + j) % arr.length];
583+
boolean clear = (j % 10) == 0; // occasionally force a clear path
584+
countly.moduleRemoteConfig.mergeCheckResponseIntoCurrentValues(clear, RemoteConfigHelper.DownloadedValuesIntoMap(pick.values));
585+
}
586+
} catch (Exception ex) {
587+
failure[0] = ex;
588+
}
589+
}, "RC-Merger-" + i);
590+
mergers[i].start();
591+
}
592+
593+
for (int i = 0; i < READ_THREADS; i++) {
594+
readers[i] = new Thread(() -> {
595+
try {
596+
for (int j = 0; j < READ_OPS; j++) {
597+
countly.remoteConfig().getValue("k1");
598+
countly.remoteConfig().getValues();
599+
countly.remoteConfig().getAllValuesAndEnroll();
600+
}
601+
} catch (Exception ex) {
602+
failure[0] = ex;
603+
}
604+
}, "RC-Reader-" + i);
605+
readers[i].start();
606+
}
607+
608+
for (Thread t : mergers) { t.join(); }
609+
for (Thread t : readers) { t.join(); }
610+
611+
if (failure[0] != null) {
612+
Assert.fail("Concurrency test failure: " + failure[0]);
613+
}
614+
615+
// resulting map should not contain partially written entries
616+
Map<String, RCData> finalVals = countly.remoteConfig().getValues();
617+
for (Map.Entry<String, RCData> e : finalVals.entrySet()) {
618+
Assert.assertNotNull("Null RCData encountered for key=" + e.getKey(), e.getValue());
619+
}
620+
}
621+
548622
static void assertCValueCachedState(Map<String, RCData> rcValues, boolean valuesAreCached) {
549623
for (Map.Entry<String, RCData> entry : rcValues.entrySet()) {
550624
if (valuesAreCached) {

‎sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigAnrTest.java‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
package ly.count.android.sdk;
2+
3+
import static org.junit.Assert.assertTrue;
4+
import static org.junit.Assert.assertFalse;
5+
6+
import android.content.Context;
7+
import android.os.Looper;
8+
9+
import androidx.test.ext.junit.runners.AndroidJUnit4;
10+
import androidx.test.platform.app.InstrumentationRegistry;
11+
12+
import org.junit.After;
13+
import org.junit.Before;
14+
import org.junit.Test;
15+
import org.junit.runner.RunWith;
16+
17+
import java.util.concurrent.CountDownLatch;
18+
import java.util.concurrent.TimeUnit;
19+
import java.util.concurrent.atomic.AtomicLong;
20+
21+
/**
22+
* Test measuring Remote Config access latency under artificial contention on the global Countly lock.
23+
* After narrowing synchronization in RemoteConfig getters, these calls should NOT block for the full
24+
* duration that another thread holds Countly's global monitor.
25+
*/
26+
@RunWith(AndroidJUnit4.class)
27+
public class RemoteConfigAnrTest {
28+
29+
private Context context;
30+
31+
@Before
32+
public void setUp() {
33+
context = InstrumentationRegistry.getInstrumentation().getTargetContext();
34+
CountlyConfig cc = new CountlyConfig(context, "app", "https://server.url")
35+
.setLoggingEnabled(true);
36+
Countly.sharedInstance().init(cc);
37+
}
38+
39+
@After
40+
public void tearDown() {
41+
Countly.sharedInstance().halt();
42+
}
43+
44+
@Test
45+
public void testRemoteConfigAccessUnderContention() throws InterruptedException {
46+
// Baseline latencies
47+
long baselineGetValue = measureLatency(() -> Countly.sharedInstance().remoteConfig().getValue("test_key"));
48+
long baselineGetValues = measureLatency(() -> Countly.sharedInstance().remoteConfig().getValues());
49+
long baselineGetValueAndEnroll = measureLatency(() -> Countly.sharedInstance().remoteConfig().getValueAndEnroll("test_key_2"));
50+
long baselineGetAllValuesAndEnroll = measureLatency(() -> Countly.sharedInstance().remoteConfig().getAllValuesAndEnroll());
51+
52+
// Create contention by holding global Countly lock for 4 seconds
53+
CountDownLatch lockAcquired = new CountDownLatch(1);
54+
CountDownLatch releaseLock = new CountDownLatch(1);
55+
Thread locker = new Thread(() -> {
56+
synchronized (Countly.sharedInstance()) {
57+
lockAcquired.countDown();
58+
try { Thread.sleep(4000); } catch (InterruptedException ignored) {}
59+
releaseLock.countDown();
60+
}
61+
}, "CountlyGlobalLockHolder");
62+
locker.start();
63+
assertTrue("Background thread failed to acquire Countly lock", lockAcquired.await(1, TimeUnit.SECONDS));
64+
65+
long contestedGetValue = measureLatency(() -> Countly.sharedInstance().remoteConfig().getValue("test_key"));
66+
long contestedGetValues = measureLatency(() -> Countly.sharedInstance().remoteConfig().getValues());
67+
long contestedGetValueAndEnroll = measureLatency(() -> Countly.sharedInstance().remoteConfig().getValueAndEnroll("test_key_2"));
68+
long contestedGetAllValuesAndEnroll = measureLatency(() -> Countly.sharedInstance().remoteConfig().getAllValuesAndEnroll());
69+
70+
releaseLock.await(5, TimeUnit.SECONDS);
71+
72+
System.out.println("[RCContention] baseline getValue=" + baselineGetValue + "ms, contested=" + contestedGetValue + "ms");
73+
System.out.println("[RCContention] baseline getValues=" + baselineGetValues + "ms, contested=" + contestedGetValues + "ms");
74+
System.out.println("[RCContention] baseline getValueAndEnroll=" + baselineGetValueAndEnroll + "ms, contested=" + contestedGetValueAndEnroll + "ms");
75+
System.out.println("[RCContention] baseline getAllValuesAndEnroll=" + baselineGetAllValuesAndEnroll + "ms, contested=" + contestedGetAllValuesAndEnroll + "ms");
76+
77+
// Reasonable baseline expectations (< 500ms each)
78+
assertTrue("Baseline getValue high", baselineGetValue < 500);
79+
assertTrue("Baseline getValues high", baselineGetValues < 500);
80+
assertTrue("Baseline getValueAndEnroll high", baselineGetValueAndEnroll < 500);
81+
assertTrue("Baseline getAllValuesAndEnroll high", baselineGetAllValuesAndEnroll < 500);
82+
83+
// Contested calls should not block near 4s. Ensure < 1500ms and not >=3000ms.
84+
assertTrue("Contested getValue too high: " + contestedGetValue, contestedGetValue < 1500);
85+
assertTrue("Contested getValues too high: " + contestedGetValues, contestedGetValues < 1500);
86+
assertTrue("Contested getValueAndEnroll too high: " + contestedGetValueAndEnroll, contestedGetValueAndEnroll < 1500);
87+
assertTrue("Contested getAllValuesAndEnroll too high: " + contestedGetAllValuesAndEnroll, contestedGetAllValuesAndEnroll < 1500);
88+
89+
assertFalse("getValue appears blocked, ANR adjacent", contestedGetValue >= 3000);
90+
assertFalse("getValues appears blocked, ANR adjacent", contestedGetValues >= 3000);
91+
assertFalse("getValueAndEnroll appears blocked, ANR adjacent", contestedGetValueAndEnroll >= 3000);
92+
assertFalse("getAllValuesAndEnroll appears blocked, ANR adjacent", contestedGetAllValuesAndEnroll >= 3000);
93+
94+
// Relative inflation guard: contested no more than 10x baseline (allows some noise)
95+
assertTrue("getValue inflation too large", contestedGetValue <= Math.max(50, baselineGetValue * 10));
96+
assertTrue("getValues inflation too large", contestedGetValues <= Math.max(50, baselineGetValues * 10));
97+
assertTrue("getValueAndEnroll inflation too large", contestedGetValueAndEnroll <= Math.max(50, baselineGetValueAndEnroll * 10));
98+
assertTrue("getAllValuesAndEnroll inflation too large", contestedGetAllValuesAndEnroll <= Math.max(50, baselineGetAllValuesAndEnroll * 10));
99+
}
100+
101+
private long measureLatency(Runnable r) {
102+
AtomicLong start = new AtomicLong();
103+
AtomicLong end = new AtomicLong();
104+
InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {
105+
start.set(System.currentTimeMillis());
106+
r.run();
107+
end.set(System.currentTimeMillis());
108+
});
109+
return end.get() - start.get();
110+
}
111+
112+
@Test
113+
public void stressTestRemoteConfigAccessUnderContention() throws InterruptedException {
114+
final int ITERS = 100;
115+
116+
// Warm-up baseline (single call per method) to initialize any lazy structures
117+
measureLatency(() -> Countly.sharedInstance().remoteConfig().getValue("stress_key"));
118+
measureLatency(() -> Countly.sharedInstance().remoteConfig().getValues());
119+
120+
long maxGetValue = 0, maxGetValues = 0;
121+
long sumGetValue = 0, sumGetValues = 0;
122+
123+
// Start contention thread holding global lock intermittently
124+
CountDownLatch startSignal = new CountDownLatch(1);
125+
CountDownLatch doneSignal = new CountDownLatch(1);
126+
Thread contender = new Thread(() -> {
127+
startSignal.countDown();
128+
long endTime = System.currentTimeMillis() + 4000; // run for ~4s
129+
while (System.currentTimeMillis() < endTime) {
130+
synchronized (Countly.sharedInstance()) {
131+
try { Thread.sleep(80); } catch (InterruptedException ignored) {}
132+
}
133+
try { Thread.sleep(20); } catch (InterruptedException ignored) {}
134+
}
135+
doneSignal.countDown();
136+
}, "CountlyIntermittentLocker");
137+
contender.start();
138+
assertTrue("Failed to start contention thread", startSignal.await(1, TimeUnit.SECONDS));
139+
140+
for (int i = 0; i < ITERS; i++) {
141+
long gv = measureLatency(() -> Countly.sharedInstance().remoteConfig().getValue("stress_key"));
142+
long gvs = measureLatency(() -> Countly.sharedInstance().remoteConfig().getValues());
143+
144+
sumGetValue += gv;
145+
sumGetValues += gvs;
146+
if (gv > maxGetValue) maxGetValue = gv;
147+
if (gvs > maxGetValues) maxGetValues = gvs;
148+
}
149+
150+
doneSignal.await(5, TimeUnit.SECONDS);
151+
152+
double avgGetValue = sumGetValue / (double) ITERS;
153+
double avgGetValues = sumGetValues / (double) ITERS;
154+
155+
System.out.println("[RCStress] iterations=" + ITERS +
156+
" getValue avg=" + avgGetValue + "ms max=" + maxGetValue +
157+
" | getValues avg=" + avgGetValues + "ms max=" + maxGetValues);
158+
159+
// average should be under 200ms, max should not exceed 1000ms under intermittent contention.
160+
assertTrue("getValue average too high: " + avgGetValue, avgGetValue < 200);
161+
assertTrue("getValues average too high: " + avgGetValues, avgGetValues < 200);
162+
assertTrue("getValue max spike too large: " + maxGetValue, maxGetValue < 1000);
163+
assertTrue("getValues max spike too large: " + maxGetValues, maxGetValues < 1000);
164+
}
165+
}

0 commit comments

Comments
 (0)