From d28e1210092710626b3c4bd2e88344b310bab31c Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Wed, 12 Aug 2026 15:43:54 -0700 Subject: [PATCH 1/2] fix: build a fresh synchronizer per factory call in recovery tests The recovery tests shared one MockQueuedSynchronizer instance across every factory call. Because FDv2DataSource closes the previous synchronizer when it switches, recovery rebuilt an already-closed mock, which reported SHUTDOWN immediately and made the synchronizer loop spin between two dead sources. The spin filled the log capture until the test JVM ran out of memory, which is how testDebugUnitTest failed in CI. Co-authored-by: Cursor --- .../sdk/android/FDv2DataSourceTest.java | 58 +++++++++++-------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2DataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2DataSourceTest.java index 5b2fde21..5bec67b0 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2DataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2DataSourceTest.java @@ -517,22 +517,27 @@ public void emptyInitializerListSkipsToSynchronizers() throws Exception { @Test public void fallbackAndRecoveryTasksWellBehaved() throws Exception { - // First sync: changeset then INTERRUPTED; second sync: changeset; recovery brings back first - MockQueuedSynchronizer firstSync = new MockQueuedSynchronizer( - FDv2SourceResult.changeSet(makeChangeSet(false), false), - interrupted()); - MockQueuedSynchronizer secondSync = new MockQueuedSynchronizer( - FDv2SourceResult.changeSet(makeChangeSet(false), false)); - AtomicInteger firstCallCount = new AtomicInteger(0); AtomicInteger secondCallCount = new AtomicInteger(0); MockComponents.MockDataSourceUpdateSink sink = new MockComponents.MockDataSourceUpdateSink(); + // First sync: changeset then INTERRUPTED; second sync: changeset; recovery brings back first. + // Each factory must build a fresh synchronizer, because the data source closes the previous + // one when it switches, and a closed synchronizer only ever reports SHUTDOWN. FDv2DataSource dataSource = buildDataSource(sink, Collections.emptyList(), Arrays.asList( - () -> { firstCallCount.incrementAndGet(); return firstSync; }, - () -> { secondCallCount.incrementAndGet(); return secondSync; }), + () -> { + firstCallCount.incrementAndGet(); + return new MockQueuedSynchronizer( + FDv2SourceResult.changeSet(makeChangeSet(false), false), + interrupted()); + }, + () -> { + secondCallCount.incrementAndGet(); + return new MockQueuedSynchronizer( + FDv2SourceResult.changeSet(makeChangeSet(false), false)); + }), 1, 2); AwaitableCallback startCallback = startDataSource(dataSource); @@ -663,17 +668,22 @@ public void recoveryResetsToFirstAvailableSynchronizer() throws Exception { AtomicInteger firstCallCount = new AtomicInteger(0); AtomicInteger secondCallCount = new AtomicInteger(0); - MockQueuedSynchronizer firstSync = new MockQueuedSynchronizer( - FDv2SourceResult.changeSet(makeChangeSet(false), false), - interrupted()); - MockQueuedSynchronizer secondSync = new MockQueuedSynchronizer( - FDv2SourceResult.changeSet(makeChangeSet(false), false)); - + // Each factory must build a fresh synchronizer, because the data source closes the previous + // one when it switches, and a closed synchronizer only ever reports SHUTDOWN. FDv2DataSource dataSource = buildDataSource(sink, Collections.emptyList(), Arrays.asList( - () -> { firstCallCount.incrementAndGet(); return firstSync; }, - () -> { secondCallCount.incrementAndGet(); return secondSync; }), + () -> { + firstCallCount.incrementAndGet(); + return new MockQueuedSynchronizer( + FDv2SourceResult.changeSet(makeChangeSet(false), false), + interrupted()); + }, + () -> { + secondCallCount.incrementAndGet(); + return new MockQueuedSynchronizer( + FDv2SourceResult.changeSet(makeChangeSet(false), false)); + }), 1, 2); AwaitableCallback startCallback = startDataSource(dataSource); @@ -2024,16 +2034,16 @@ public void orchestrationLogging_fallback_logsInfo() throws Exception { @Test public void orchestrationLogging_recovery_logsInfo() throws Exception { MockComponents.MockDataSourceUpdateSink sink = new MockComponents.MockDataSourceUpdateSink(); - MockQueuedSynchronizer firstSync = new MockQueuedSynchronizer( - FDv2SourceResult.changeSet(makeChangeSet(false), false), - interrupted()); - MockQueuedSynchronizer secondSync = new MockQueuedSynchronizer( - FDv2SourceResult.changeSet(makeChangeSet(false), false)); + // Fresh instances per build: the data source closes the previous synchronizer when it + // switches, and a closed synchronizer only ever reports SHUTDOWN. FDv2DataSource dataSource = buildDataSource(sink, Collections.emptyList(), Arrays.asList( - () -> firstSync, - () -> secondSync), + () -> new MockQueuedSynchronizer( + FDv2SourceResult.changeSet(makeChangeSet(false), false), + interrupted()), + () -> new MockQueuedSynchronizer( + FDv2SourceResult.changeSet(makeChangeSet(false), false))), 1, 2); AwaitableCallback startCallback = startDataSource(dataSource); try { From 69f18b071040fb641a12c83aeac7a906b0a92cdf Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Wed, 12 Aug 2026 15:58:00 -0700 Subject: [PATCH 2/2] fix: rate limit synchronizer rotation so dead sources cannot spin The synchronizer loop advanced to the next source as soon as a session ended, with no lower bound on how long a session had to last. A source that reports SHUTDOWN as soon as it is built therefore let the loop rotate at CPU speed: over a million synchronizers were built in 1.5 seconds in a test that reproduces it. Rotation now pauses when a session ends sooner than it plausibly could have connected, growing the pause up to 30 seconds while sessions keep ending immediately and resetting once one of them lasts. The pause waits on shutdownCause instead of sleeping, so a stop() during it is acted on at once. Co-authored-by: Cursor --- .../sdk/android/FDv2DataSource.java | 50 +++++++++++++++++++ .../sdk/android/FDv2DataSourceTest.java | 31 ++++++++++++ 2 files changed, 81 insertions(+) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2DataSource.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2DataSource.java index 9b8d4056..87eae313 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2DataSource.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2DataSource.java @@ -28,6 +28,8 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; /** @@ -54,6 +56,15 @@ public interface DataSourceFactory { private static final String INITIALIZER_CANCELLED = "Initializer cancelled: {}"; private static final String INITIALIZER_INTERRUPTED = "Initializer interrupted: {}"; + /** + * A synchronizer session shorter than this never really connected to anything, so the rotation + * to the next synchronizer is paused rather than attempted immediately. + */ + private static final long MIN_SYNCHRONIZER_SESSION_MILLIS = 500; + + /** Upper bound for the growing pause between synchronizer sessions that keep ending at once. */ + private static final long MAX_ROTATION_PAUSE_MILLIS = 30_000; + private final List> cacheInitializers; private final SourceManager sourceManager; private final long fallbackTimeoutSeconds; @@ -510,13 +521,36 @@ private void maybeLogSynchronizerStatusChange(@Nullable String sourceName, @NonN logger.info("Synchronizer '{}' reported status: {}.", sourceName, state.name()); } + /** + * Waits before building the next synchronizer. Waiting on {@link #shutdownCause} rather than + * sleeping means a {@link #stop(Callback)} during the pause is acted on right away. + * + * @return false if the data source shut down while waiting, in which case the caller must stop + */ + private boolean pauseBeforeRotation(long pauseMillis) { + logger.debug("Waiting {}ms before trying the next synchronizer.", pauseMillis); + try { + shutdownCause.get(pauseMillis, TimeUnit.MILLISECONDS); + return false; + } catch (TimeoutException e) { + return true; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return false; + } catch (ExecutionException e) { + return false; + } + } + private void runSynchronizers( @NonNull LDContext context, @NonNull DataSourceUpdateSinkV2 sink ) { + long rotationPauseMillis = 0; try { Synchronizer synchronizer = sourceManager.getNextAvailableSynchronizerAndSetActive(); while (synchronizer != null) { + long sessionStartNanos = System.nanoTime(); String synchronizerName = synchronizer.name(); logger.info("Synchronizer '{}' is starting.", synchronizerName); resetSynchronizerStatusDedupe(); @@ -651,6 +685,22 @@ private void runSynchronizers( Thread.currentThread().interrupt(); return; } + + // A source that ends its session at once, such as one reporting SHUTDOWN as soon as + // it is built, must not be allowed to drive this rotation at CPU speed. The pause + // grows while sessions keep ending immediately and resets once one of them lasts. + if (TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - sessionStartNanos) + < MIN_SYNCHRONIZER_SESSION_MILLIS) { + rotationPauseMillis = rotationPauseMillis == 0 + ? MIN_SYNCHRONIZER_SESSION_MILLIS + : Math.min(rotationPauseMillis * 2, MAX_ROTATION_PAUSE_MILLIS); + if (!pauseBeforeRotation(rotationPauseMillis)) { + return; + } + } else { + rotationPauseMillis = 0; + } + synchronizer = sourceManager.getNextAvailableSynchronizerAndSetActive(); } if (!stopCalled.get()) { diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2DataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2DataSourceTest.java index 5bec67b0..a79df9d9 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2DataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2DataSourceTest.java @@ -697,6 +697,37 @@ public void recoveryResetsToFirstAvailableSynchronizer() throws Exception { stopDataSource(dataSource); } + @Test + public void synchronizersThatShutDownImmediatelyDoNotSpin() throws Exception { + MockComponents.MockDataSourceUpdateSink sink = new MockComponents.MockDataSourceUpdateSink(); + AtomicInteger buildCount = new AtomicInteger(0); + + // Every session ends as soon as it starts, so the rotation has nothing to wait on and would + // run at CPU speed if it were not rate limited. + FDv2DataSource dataSource = buildDataSource(sink, + Collections.emptyList(), + Arrays.asList( + () -> { + buildCount.incrementAndGet(); + return new MockQueuedSynchronizer( + FDv2SourceResult.status(FDv2SourceResult.Status.shutdown(), false)); + }, + () -> { + buildCount.incrementAndGet(); + return new MockQueuedSynchronizer( + FDv2SourceResult.status(FDv2SourceResult.Status.shutdown(), false)); + })); + + startDataSource(dataSource); + Thread.sleep(1500); + + // Pauses of 500ms, 1s, 2s and so on allow only a handful of attempts in this window. + int builds = buildCount.get(); + assertTrue("expected rate limited rotation, but saw " + builds + " synchronizers built", + builds <= 6); + stopDataSource(dataSource); + } + @Test public void fallbackMovesToNextSynchronizer() throws Exception { MockComponents.MockDataSourceUpdateSink sink = new MockComponents.MockDataSourceUpdateSink();