From 1850bd1700cc155a68c44e565794796a29b51f32 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Thu, 1 Oct 2026 18:55:33 -0700 Subject: [PATCH 01/14] feat: Retry FDv1 data sources after unexpected errors instead of stopping --- .../com/launchdarkly/sdktest/TestService.java | 4 +- launchdarkly-android-client-sdk/build.gradle | 2 +- .../sdk/android/AndroidTaskExecutorTest.java | 46 ------- .../sdk/android/AndroidTaskExecutor.java | 6 - .../sdk/android/ComponentsImpl.java | 1 - .../android/FDv2StreamingSynchronizer.java | 2 +- .../sdk/android/PollingDataSource.java | 54 ++++++--- .../sdk/android/PollingRetryState.java | 79 ++++++++++++ .../sdk/android/StreamingDataSource.java | 63 +++++----- .../sdk/android/TaskExecutor.java | 10 -- .../sdk/android/PollingDataSourceTest.java | 7 -- .../sdk/android/StreamingDataSourceTest.java | 114 +----------------- .../sdk/android/ManualTaskExecutor.java | 7 -- .../sdk/android/SimpleTestTaskExecutor.java | 6 - 14 files changed, 149 insertions(+), 252 deletions(-) create mode 100644 launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingRetryState.java diff --git a/contract-tests/src/main/java/com/launchdarkly/sdktest/TestService.java b/contract-tests/src/main/java/com/launchdarkly/sdktest/TestService.java index b6618d9e0..50acf5e51 100644 --- a/contract-tests/src/main/java/com/launchdarkly/sdktest/TestService.java +++ b/contract-tests/src/main/java/com/launchdarkly/sdktest/TestService.java @@ -43,7 +43,9 @@ public class TestService extends NanoHTTPD { "evaluation-hooks", "track-hooks", "client-per-context-summaries", - "client-event-source-http-errors" + "client-event-source-http-errors", + "retry-conformance-fdv1-streaming", + "retry-conformance-fdv1-polling" }; private static final String MIME_JSON = "application/json"; static final Gson gson = new GsonBuilder() diff --git a/launchdarkly-android-client-sdk/build.gradle b/launchdarkly-android-client-sdk/build.gradle index 3b7566cb1..05e9fc44c 100644 --- a/launchdarkly-android-client-sdk/build.gradle +++ b/launchdarkly-android-client-sdk/build.gradle @@ -73,7 +73,7 @@ ext {} ext.versions = [ "androidAnnotation": "1.2.0", "androidAppcompat": "1.1.0", - "eventsource": "4.2.0", + "eventsource": "5.0.0", "gson": "2.13.2", "jacksonCore": "2.10.5", "jacksonDatabind": "2.10.5.1", diff --git a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/AndroidTaskExecutorTest.java b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/AndroidTaskExecutorTest.java index 5b857468a..e8b421971 100644 --- a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/AndroidTaskExecutorTest.java +++ b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/AndroidTaskExecutorTest.java @@ -2,10 +2,8 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNotSame; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; @@ -24,11 +22,9 @@ import org.junit.Test; import org.junit.runner.RunWith; -import java.util.ArrayList; import java.util.concurrent.BlockingQueue; import java.util.concurrent.CountDownLatch; import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -122,46 +118,4 @@ public void scheduleTaskCatchesAndLogsExceptions() throws InterruptedException { assertThat(message2.getText(), containsString(this.getClass().getName())); // stacktrace } - @Test - public void repeatingTasks() throws InterruptedException { - BlockingQueue executedA = new LinkedBlockingQueue<>(), - executedB = new LinkedBlockingQueue<>(), - executedC = new LinkedBlockingQueue<>(); - ScheduledFuture taskA = taskExecutor.startRepeatingTask( - () -> { - executedA.add(System.currentTimeMillis()); - }, - 10, - 10 - ); - ScheduledFuture taskB = taskExecutor.startRepeatingTask( - () -> { - executedB.add(System.currentTimeMillis()); - }, - 10, - 20 - ); - ScheduledFuture taskC = taskExecutor.startRepeatingTask( - () -> { - executedC.add(System.currentTimeMillis()); - }, - 10000, - 20 - ); - try { - Thread.sleep(100); - - assertEquals("C task should not have executed yet", 0, executedC.size()); - } finally { - taskA.cancel(false); - taskB.cancel(false); - taskC.cancel(false); - } - executedA.drainTo(new ArrayList<>()); - // After stopping the task, let's tolerate it firing one more time, but no more than that. - if (executedA.poll(50, TimeUnit.MILLISECONDS) != null) { - assertNull("A task should have stopped executing", - executedA.poll(50, TimeUnit.MILLISECONDS)); - } - } } diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/AndroidTaskExecutor.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/AndroidTaskExecutor.java index b19c204dd..50197a230 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/AndroidTaskExecutor.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/AndroidTaskExecutor.java @@ -42,12 +42,6 @@ public ScheduledFuture scheduleTask(Runnable action, long delayMillis) { return executor.schedule(wrapActionWithErrorHandling(action), delayMillis, TimeUnit.MILLISECONDS); } - @Override - public ScheduledFuture startRepeatingTask(Runnable action, long initialDelayMillis, long intervalMillis) { - return executor.scheduleAtFixedRate(wrapActionWithErrorHandling(action), - initialDelayMillis, intervalMillis, TimeUnit.MILLISECONDS); - } - @Override public void close() { executor.shutdownNow(); diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java index 8c9cc0454..34b47a2fe 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ComponentsImpl.java @@ -292,7 +292,6 @@ public DataSource build(ClientContext clientContext) { pollInterval, maxNumPolls, clientContextImpl.getFetcher(), - clientContextImpl.getPlatformState(), clientContextImpl.getTaskExecutor(), clientContext.getBaseLogger() ); diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2StreamingSynchronizer.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2StreamingSynchronizer.java index ef8e0d5f8..ad6daf6dd 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2StreamingSynchronizer.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2StreamingSynchronizer.java @@ -195,8 +195,8 @@ private void startStream() { } EventSource es = new EventSource.Builder(connectStrategy) - .retryDelay(initialReconnectDelayMillis, TimeUnit.MILLISECONDS) .retryDelayStrategy(RetryDelayStrategy.defaultStrategy() + .initialDelay(initialReconnectDelayMillis, TimeUnit.MILLISECONDS) .maxDelay(MAX_RECONNECT_TIME_MS, TimeUnit.MILLISECONDS)) .errorStrategy(ErrorStrategy.alwaysContinue()) .build(); diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java index a68697d27..c0eb3ad9c 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java @@ -24,10 +24,11 @@ final class PollingDataSource implements DataSource { final long pollIntervalMillis; // visible for testing long numberOfPollsRemaining; // visible for testing private final FeatureFetcher fetcher; - private final PlatformState platformState; private final TaskExecutor taskExecutor; private final LDLogger logger; - final AtomicReference> currentPollTask = new AtomicReference<>(); // visible for testing + private final PollingRetryState retryState; + private volatile boolean stopped; + private final AtomicReference> currentPollTask = new AtomicReference<>(); /** * @param context that this data source will fetch data for @@ -38,7 +39,6 @@ final class PollingDataSource implements DataSource { * @param pollIntervalMillis interval in millis between each polling request * @param maxNumberOfPolls the maximum number of polling attempts, use Long.MAX for effectively unlimited. * @param fetcher that will be used for each fetch - * @param platformState used for making decisions based on platform state * @param taskExecutor that will be used to schedule the polling tasks * @param logger for logging */ @@ -49,7 +49,6 @@ final class PollingDataSource implements DataSource { long pollIntervalMillis, long maxNumberOfPolls, FeatureFetcher fetcher, - PlatformState platformState, TaskExecutor taskExecutor, LDLogger logger ) { @@ -59,9 +58,9 @@ final class PollingDataSource implements DataSource { this.pollIntervalMillis = pollIntervalMillis; this.numberOfPollsRemaining = maxNumberOfPolls; this.fetcher = fetcher; - this.platformState = platformState; this.taskExecutor = taskExecutor; this.logger = logger; + this.retryState = new PollingRetryState(pollIntervalMillis); } @Override @@ -73,16 +72,14 @@ public void start(final Callback resultCallback) { return; } - Runnable pollRunnable = () -> poll(resultCallback); logger.debug("Scheduling polling task with interval of {}ms, starting after {}ms, with number of polls {}", pollIntervalMillis, initialDelayMillis, numberOfPollsRemaining); - ScheduledFuture task = taskExecutor.startRepeatingTask(pollRunnable, - initialDelayMillis, pollIntervalMillis); - currentPollTask.set(task); + schedulePoll(resultCallback, initialDelayMillis); } @Override public void stop(Callback completionCallback) { + stopped = true; ScheduledFuture task = currentPollTask.getAndSet(null); if (task != null) { task.cancel(true); @@ -90,18 +87,35 @@ public void stop(Callback completionCallback) { completionCallback.onSuccess(null); } + private void schedulePoll(Callback resultCallback, long delayMillis) { + if (stopped) { + return; + } + currentPollTask.set(taskExecutor.scheduleTask(() -> poll(resultCallback), delayMillis)); + } + private void poll(Callback resultCallback) { - // poll if there are polls remaining - if (numberOfPollsRemaining > 0) { - numberOfPollsRemaining--; - ConnectivityManager.fetchAndSetData(fetcher, context, dataSourceUpdateSink, - resultCallback, logger); - } else { - // terminate if we have no polls remaining - ScheduledFuture task = currentPollTask.getAndSet(null); - if (task != null) { - task.cancel(true); - } + if (numberOfPollsRemaining <= 0) { + return; } + numberOfPollsRemaining--; + + // Each poll schedules the next one, so the wait can grow after a failure. + ConnectivityManager.fetchAndSetData(fetcher, context, dataSourceUpdateSink, + new Callback() { + @Override + public void onSuccess(Boolean result) { + retryState.recordSuccess(); + resultCallback.onSuccess(result); + schedulePoll(resultCallback, retryState.nextDelayMillis()); + } + + @Override + public void onError(Throwable e) { + retryState.recordFailure(e); + resultCallback.onError(e); + schedulePoll(resultCallback, retryState.nextDelayMillis()); + } + }, logger); } } diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingRetryState.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingRetryState.java new file mode 100644 index 000000000..e22aa2408 --- /dev/null +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingRetryState.java @@ -0,0 +1,79 @@ +package com.launchdarkly.sdk.android; + +import androidx.annotation.Nullable; + +import com.launchdarkly.eventsource.RetryDelayStrategy; + +import java.util.concurrent.TimeUnit; + +/** + * Computes the wait before a polling data source polls again. + *

+ * A poll interval is already a wait, so an ordinary failure does not extend it. Only a failure + * that is unlikely to correct itself, such as a rejected credential, moves polling into a + * regime that waits longer, and it stays there until enough consecutive polls succeed. + *

+ * This class is not thread safe. + */ +final class PollingRetryState { + private static final int RESET_THRESHOLD_SUCCESSES = 2; + private static final long EXTENDED_INITIAL_DELAY_MS = 300_000; + private static final long EXTENDED_MAX_DELAY_MS = 3_600_000; + + private final long pollIntervalMillis; + + private RetryDelayStrategy extendedDelay; + private boolean inExtendedRegime; + private int consecutiveSuccesses; + + PollingRetryState(long pollIntervalMillis) { + this.pollIntervalMillis = pollIntervalMillis; + this.extendedDelay = freshExtendedDelay(); + } + + // Neither bound drops below the poll interval, which would make a failing source poll more + // often than a healthy one. + private RetryDelayStrategy freshExtendedDelay() { + return RetryDelayStrategy.defaultStrategy() + .initialDelay(Math.max(EXTENDED_INITIAL_DELAY_MS, pollIntervalMillis), + TimeUnit.MILLISECONDS) + .maxDelay(Math.max(EXTENDED_MAX_DELAY_MS, pollIntervalMillis), + TimeUnit.MILLISECONDS); + } + + void recordFailure(@Nullable Throwable error) { + consecutiveSuccesses = 0; + if (!inExtendedRegime) { + if (isUnexpected(error)) { + inExtendedRegime = true; + extendedDelay = freshExtendedDelay(); + } + return; + } + extendedDelay = extendedDelay.getNext(); + } + + // True for an HTTP status the service is unlikely to stop returning, such as a 401. + private static boolean isUnexpected(@Nullable Throwable error) { + return error instanceof LDInvalidResponseCodeFailure + && !LDUtil.isHttpErrorRecoverable( + ((LDInvalidResponseCodeFailure) error).getResponseCode()); + } + + void recordSuccess() { + consecutiveSuccesses++; + if (consecutiveSuccesses >= RESET_THRESHOLD_SUCCESSES) { + inExtendedRegime = false; + extendedDelay = freshExtendedDelay(); + } + } + + long nextDelayMillis() { + // A success shows the service works, so polling returns to its interval at once even + // though the regime has not reset yet. + if (!inExtendedRegime || consecutiveSuccesses > 0) { + return pollIntervalMillis; + } + return Math.max(pollIntervalMillis, extendedDelay.getDelayMillis()); + } +} diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java index 366254b28..fc076b5a1 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java @@ -48,26 +48,30 @@ final class StreamingDataSource implements DataSource { private static final String PATCH = "patch"; private static final String DELETE = "delete"; - private static final long MAX_RECONNECT_TIME_MS = 300_000; // 5 minutes - private static final long READ_TIMEOUT_MS = 300_000; // 5 minutes is the standard read timeout used for all LaunchDarkly stream connections, based on // an expectation that the server will send heartbeats at a shorter interval than that - private BackgroundEventSource es; + private static final long MAX_RECONNECT_TIME_MS = 30_000; + private static final long EXTENDED_INITIAL_RECONNECT_DELAY_MS = 300_000; + private static final long EXTENDED_MAX_RECONNECT_TIME_MS = 3_600_000; + + // volatile because the connection error handler reads it on the stream thread to switch + // retry regimes, while start() and stopSync() write it from other threads. + private volatile BackgroundEventSource es; private final LDContext context; private final HttpProperties httpProperties; private final boolean evaluationReasons; final int initialReconnectDelayMillis; // visible for testing + private final RetryDelayStrategy normalRetryDelay; + private final RetryDelayStrategy extendedRetryDelay; private final boolean useReport; private final URI streamUri; private final DataSourceUpdateSink dataSourceUpdateSink; private final FeatureFetcher fetcher; private final boolean streamEvenInBackground; + // volatile because stopSync() writes it from the thread stop() spawns, and start() reads it. private volatile boolean running = false; - // volatile because it is written from the EventSource background thread (onError) - // and read from start(), which may be invoked on a different thread. - private volatile boolean connection401Error = false; private final DiagnosticStore diagnosticStore; private long eventSourceStarted; private final LDLogger logger; @@ -88,13 +92,19 @@ final class StreamingDataSource implements DataSource { this.evaluationReasons = clientContext.isEvaluationReasons(); this.useReport = clientContext.getHttp().isUseReport(); this.initialReconnectDelayMillis = initialReconnectDelayMillis; + this.normalRetryDelay = RetryDelayStrategy.defaultStrategy() + .initialDelay(initialReconnectDelayMillis, TimeUnit.MILLISECONDS) + .maxDelay(MAX_RECONNECT_TIME_MS, TimeUnit.MILLISECONDS); + this.extendedRetryDelay = RetryDelayStrategy.defaultStrategy() + .initialDelay(EXTENDED_INITIAL_RECONNECT_DELAY_MS, TimeUnit.MILLISECONDS) + .maxDelay(EXTENDED_MAX_RECONNECT_TIME_MS, TimeUnit.MILLISECONDS); this.streamEvenInBackground = streamEvenInBackground; this.diagnosticStore = ClientContextImpl.get(clientContext).getDiagnosticStore(); this.logger = clientContext.getBaseLogger(); } public void start(@NonNull Callback resultCallback) { - if (!running && !connection401Error) { + if (!running) { logger.debug("Starting."); BackgroundEventHandler handler = new BackgroundEventHandler() { @@ -133,24 +143,8 @@ public void onError(Throwable t) { diagnosticStore.recordStreamInit(eventSourceStarted, (int) (System.currentTimeMillis() - eventSourceStarted), true); } int code = ((StreamHttpErrorException) t).getCode(); - if (!LDUtil.isHttpErrorRecoverable(code)) { - logger.error("Encountered non-retriable error: {}. Aborting connection to stream. Verify correct Mobile Key and Stream URI", code); - running = false; - // Set the connection401Error guard before notifying the callback. A consumer - // may react to the error by synchronously calling start() again, and the - // guard must already be set so that retry is a no-op. - if (code == 401) { - connection401Error = true; - } - resultCallback.onError(new LDInvalidResponseCodeFailure("Unexpected Response Code From Stream Connection", t, code, false)); - if (code == 401) { - dataSourceUpdateSink.shutDown(); - } - stop(null); - } else { - eventSourceStarted = System.currentTimeMillis(); - resultCallback.onError(new LDInvalidResponseCodeFailure("Unexpected Response Code From Stream Connection", t, code, true)); - } + eventSourceStarted = System.currentTimeMillis(); + resultCallback.onError(new LDInvalidResponseCodeFailure("Unexpected Response Code From Stream Connection", t, code, true)); } else { resultCallback.onError(new LDFailure("Network error in stream connection", t, LDFailure.FailureType.NETWORK_FAILURE)); } @@ -173,20 +167,21 @@ public void onError(Throwable t) { } EventSource.Builder esBuilder = new EventSource.Builder(connectStrategy) - .retryDelay(initialReconnectDelayMillis, TimeUnit.MILLISECONDS) - .retryDelayStrategy(RetryDelayStrategy.defaultStrategy() - .maxDelay(MAX_RECONNECT_TIME_MS, TimeUnit.MILLISECONDS)); + .retryDelayStrategy(normalRetryDelay) + .retryDelayStrategy(extendedRetryDelay); eventSourceStarted = System.currentTimeMillis(); es = new BackgroundEventSource.Builder(handler, esBuilder) - // The stream thread asks this handler, before it reconnects, whether an - // error ends the stream. The onError callback above runs on a different - // thread, so a stop() from there can arrive after a fast reconnect has - // already opened a new connection. A decision made here cannot lose that race. + // The stream thread calls this handler after a failure and before it + // computes the reconnect delay, so a regime switched here applies to the + // reconnect that follows this failure. .connectionErrorHandler(t -> { - if (t instanceof StreamHttpErrorException && + BackgroundEventSource current = es; + if (current != null && t instanceof StreamHttpErrorException && !LDUtil.isHttpErrorRecoverable(((StreamHttpErrorException) t).getCode())) { - return ConnectionErrorHandler.Action.SHUTDOWN; + current.getEventSource() + .activateRetryDelayStrategy(extendedRetryDelay); + logger.info("Classified failure as unexpected. Verify correct Mobile Key and Stream URI. Engaging extended backoff"); } return ConnectionErrorHandler.Action.PROCEED; }) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/TaskExecutor.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/TaskExecutor.java index eeb82181c..741f8abe6 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/TaskExecutor.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/TaskExecutor.java @@ -27,14 +27,4 @@ interface TaskExecutor extends Closeable { * @return a ScheduledFuture that can be used to cancel the task */ ScheduledFuture scheduleTask(Runnable action, long delayMillis); - - /** - * Schedules an action to be run repeatedly at intervals. It will not be done on the main thread. - * - * @param action the action to execute at each interval - * @param initialDelayMillis milliseconds to wait before the first execution - * @param intervalMillis milliseconds between executions - * @return a ScheduledFuture that can be used to cancel the task - */ - ScheduledFuture startRepeatingTask(Runnable action, long initialDelayMillis, long intervalMillis); } diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java index d16fdbc05..fea421eea 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java @@ -3,9 +3,7 @@ import static com.launchdarkly.sdk.android.AssertHelpers.requireNoMoreValues; import static com.launchdarkly.sdk.android.AssertHelpers.requireValue; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertTrue; import com.launchdarkly.sdk.LDContext; import com.launchdarkly.sdk.LDValue; @@ -25,7 +23,6 @@ import java.util.Map; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; public class PollingDataSourceTest { @@ -260,7 +257,6 @@ public void terminatesAfterMaxNumberOfPolls() throws Exception { 50, 2, // maximum number of requests is 2 clientContext.getFetcher(), - clientContext.getPlatformState(), clientContext.getTaskExecutor(), clientContext.getBaseLogger() ); @@ -271,8 +267,6 @@ public void terminatesAfterMaxNumberOfPolls() throws Exception { try { ds.start(LDUtil.noOpCallback()); - ScheduledFuture pollTask = ds.currentPollTask.get(); - assertFalse(pollTask.isCancelled()); LDContext context1 = requireValue(fetcher.receivedContexts, 500, TimeUnit.MILLISECONDS); @@ -280,7 +274,6 @@ public void terminatesAfterMaxNumberOfPolls() throws Exception { // if a third request is sent, this will fail here requireNoMoreValues(fetcher.receivedContexts, 200, TimeUnit.MILLISECONDS); - assertTrue(pollTask.isCancelled()); } finally { ds.stop(LDUtil.noOpCallback()); } diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java index aa1bbef88..402c86f87 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java @@ -635,48 +635,6 @@ public void startSendsRequestWithReportAndReasons() throws Exception { // --- start(): error handling verified via HttpServer --- - @Test - public void startWithHttp401ShutsDownSink() throws Exception { - try (HttpServer server = HttpServer.start(Handlers.status(401))) { - StreamingDataSource sds = makeStreamingDataSource( - server.getUri(), false, false); - TrackingCallback callback = new TrackingCallback(); - startDataSource(sds, callback); - - Throwable error = callback.awaitError(); - assertNotNull(error); - assertTrue(error instanceof LDInvalidResponseCodeFailure); - LDInvalidResponseCodeFailure failure = (LDInvalidResponseCodeFailure) error; - assertEquals(401, failure.getResponseCode()); - assertFalse(failure.isRetryable()); - // shutDown() runs on the EventSource background thread, so poll briefly - // to allow that thread to complete before asserting. - long deadline = System.currentTimeMillis() + 1000; - while (!dataSourceUpdateSink.shutDownCalled && System.currentTimeMillis() < deadline) { - Thread.sleep(10); - } - assertTrue(dataSourceUpdateSink.shutDownCalled); - } - } - - @Test - public void startWithHttp403ReportsNonRetriableWithoutShutDown() throws Exception { - try (HttpServer server = HttpServer.start(Handlers.status(403))) { - StreamingDataSource sds = makeStreamingDataSource( - server.getUri(), false, false); - TrackingCallback callback = new TrackingCallback(); - startDataSource(sds, callback); - - Throwable error = callback.awaitError(); - assertNotNull(error); - assertTrue(error instanceof LDInvalidResponseCodeFailure); - LDInvalidResponseCodeFailure failure = (LDInvalidResponseCodeFailure) error; - assertEquals(403, failure.getResponseCode()); - assertFalse(failure.isRetryable()); - assertFalse(dataSourceUpdateSink.shutDownCalled); - } - } - @Test public void startWithHttp500ReportsRetriableError() throws Exception { try (HttpServer server = HttpServer.start(Handlers.status(500))) { @@ -771,7 +729,7 @@ public void startWithNetworkErrorReportsNetworkFailure() throws Exception { } @Test - public void startWithHttp401PreventsSubsequentStart() throws Exception { + public void startWhileAlreadyRunningIsANoOp() throws Exception { try (HttpServer server = HttpServer.start(Handlers.status(401))) { StreamingDataSource sds = makeStreamingDataSource( server.getUri(), false, false); @@ -780,7 +738,7 @@ public void startWithHttp401PreventsSubsequentStart() throws Exception { assertNotNull(callback1.awaitError()); - // Second start should be a no-op due to connection401Error flag + // The data source is still running and retrying, so it ignores the second start. TrackingCallback callback2 = new TrackingCallback(); startDataSource(sds, callback2); @@ -790,74 +748,6 @@ public void startWithHttp401PreventsSubsequentStart() throws Exception { } } - // --- start(): no reconnect after an unrecoverable HTTP error --- - - @Test - public void unrecoverableErrorOnInitialConnectDoesNotReconnect() throws Exception { - String putEvent = makeSseEvent("put", VALID_PUT_JSON); - - // A second request would get a working stream. The SDK must not make it. - try (HttpServer server = HttpServer.start(Handlers.sequential( - Handlers.status(401), - Handlers.all( - Handlers.SSE.start(), - Handlers.SSE.event(putEvent), - Handlers.SSE.leaveOpen())))) { - - StreamingDataSource sds = makeStreamingDataSource( - server.getUri(), dataSourceUpdateSink, false, false, 1); - TrackingCallback callback = new TrackingCallback(); - sds.start(callback); - - Throwable error = callback.awaitError(); - assertNotNull(error); - assertFalse(((LDInvalidResponseCodeFailure) error).isRetryable()); - - server.getRecorder().requireRequest(); - server.getRecorder().requireNoRequests(500, TimeUnit.MILLISECONDS); - assertNull("no stream data expected after the error", - callback.successes.poll(100, TimeUnit.MILLISECONDS)); - } - } - - @Test - public void unrecoverableErrorOnReconnectDoesNotReconnectAgain() throws Exception { - String putEvent = makeSseEvent("put", VALID_PUT_JSON); - - try (HttpServer server = HttpServer.start(Handlers.sequential( - // The first connection delivers data, then the server ends the stream. - Handlers.all( - Handlers.SSE.start(), - Handlers.SSE.event(putEvent)), - // The reconnect gets an unrecoverable status. - Handlers.status(403), - // A third request would get a working stream. The SDK must not make it. - Handlers.all( - Handlers.SSE.start(), - Handlers.SSE.event(putEvent), - Handlers.SSE.leaveOpen())))) { - - StreamingDataSource sds = makeStreamingDataSource( - server.getUri(), dataSourceUpdateSink, false, false, 1); - TrackingCallback callback = new TrackingCallback(); - sds.start(callback); - - assertNotNull(callback.awaitSuccess()); - - // The dropped stream reports a network failure first; the 403 follows. - Throwable error = callback.awaitError(); - while (error != null && !(error instanceof LDInvalidResponseCodeFailure)) { - error = callback.awaitError(); - } - assertNotNull(error); - assertEquals(403, ((LDInvalidResponseCodeFailure) error).getResponseCode()); - - server.getRecorder().requireRequest(); - server.getRecorder().requireRequest(); - server.getRecorder().requireNoRequests(500, TimeUnit.MILLISECONDS); - } - } - // --- start(): SSE event processing via HttpServer --- @Test diff --git a/shared-test-code/src/main/java/com/launchdarkly/sdk/android/ManualTaskExecutor.java b/shared-test-code/src/main/java/com/launchdarkly/sdk/android/ManualTaskExecutor.java index 0f48f4a27..88b67bffd 100644 --- a/shared-test-code/src/main/java/com/launchdarkly/sdk/android/ManualTaskExecutor.java +++ b/shared-test-code/src/main/java/com/launchdarkly/sdk/android/ManualTaskExecutor.java @@ -51,13 +51,6 @@ public ScheduledFuture scheduleTask(Runnable action, long delayMillis) { return future; } - @Override - public ScheduledFuture startRepeatingTask(Runnable action, long initialDelayMillis, long intervalMillis) { - ManualScheduledFuture future = new ManualScheduledFuture(action); - pending.add(future); - return future; - } - @Override public void close() { pending.clear(); diff --git a/shared-test-code/src/main/java/com/launchdarkly/sdk/android/SimpleTestTaskExecutor.java b/shared-test-code/src/main/java/com/launchdarkly/sdk/android/SimpleTestTaskExecutor.java index f53c0dcbf..47f09845b 100644 --- a/shared-test-code/src/main/java/com/launchdarkly/sdk/android/SimpleTestTaskExecutor.java +++ b/shared-test-code/src/main/java/com/launchdarkly/sdk/android/SimpleTestTaskExecutor.java @@ -29,12 +29,6 @@ public ScheduledFuture scheduleTask(Runnable action, long delayMillis) { return executor.schedule(action, delayMillis, TimeUnit.MILLISECONDS); } - @Override - public ScheduledFuture startRepeatingTask(Runnable action, long initialDelayMillis, long intervalMillis) { - return executor.scheduleAtFixedRate(action, - initialDelayMillis, intervalMillis, TimeUnit.MILLISECONDS); - } - @Override public void close() { executor.shutdownNow(); From 3b827e9d30d0e09fc995edaf4c003de918500a61 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Thu, 8 Oct 2026 23:14:10 -0700 Subject: [PATCH 02/14] fix: Stop polling on stop() and keep polling when a result callback throws --- .../sdk/android/PollingDataSource.java | 34 +++-- .../sdk/android/PollingDataSourceTest.java | 143 ++++++++++++++++++ 2 files changed, 167 insertions(+), 10 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java index c0eb3ad9c..48e79cd86 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java @@ -88,10 +88,12 @@ public void stop(Callback completionCallback) { } private void schedulePoll(Callback resultCallback, long delayMillis) { - if (stopped) { - return; - } - currentPollTask.set(taskExecutor.scheduleTask(() -> poll(resultCallback), delayMillis)); + currentPollTask.set(taskExecutor.scheduleTask(() -> { + if (stopped) { + return; + } + poll(resultCallback); + }, delayMillis)); } private void poll(Callback resultCallback) { @@ -105,16 +107,28 @@ private void poll(Callback resultCallback) { new Callback() { @Override public void onSuccess(Boolean result) { - retryState.recordSuccess(); - resultCallback.onSuccess(result); - schedulePoll(resultCallback, retryState.nextDelayMillis()); + if (stopped) { + return; + } + try { + retryState.recordSuccess(); + resultCallback.onSuccess(result); + } finally { + schedulePoll(resultCallback, retryState.nextDelayMillis()); + } } @Override public void onError(Throwable e) { - retryState.recordFailure(e); - resultCallback.onError(e); - schedulePoll(resultCallback, retryState.nextDelayMillis()); + if (stopped) { + return; + } + try { + retryState.recordFailure(e); + resultCallback.onError(e); + } finally { + schedulePoll(resultCallback, retryState.nextDelayMillis()); + } } }, logger); } diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java index fea421eea..598ff45e1 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java @@ -20,9 +20,12 @@ import org.junit.Test; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; public class PollingDataSourceTest { @@ -279,6 +282,146 @@ public void terminatesAfterMaxNumberOfPolls() throws Exception { } } + private PollingDataSource makeDataSource(FeatureFetcher fetcherToUse, TaskExecutor executor, + long pollIntervalMillis) { + return new PollingDataSource( + CONTEXT, + dataSourceUpdateSink, + 0, + pollIntervalMillis, + Long.MAX_VALUE, + fetcherToUse, + executor, + logging.logger + ); + } + + @Test + public void pollArmedBeforeStopDoesNotFetchWhenItFires() throws Exception { + // Schedules tasks for real, but returns no handle, so stop has nothing to cancel. + TaskExecutor executorWithoutCancellation = new TaskExecutor() { + @Override + public void executeOnMainThread(Runnable action) { + taskExecutor.executeOnMainThread(action); + } + + @Override + public ScheduledFuture scheduleTask(Runnable action, long delayMillis) { + taskExecutor.scheduleTask(action, delayMillis); + return null; + } + + @Override + public void close() {} + }; + + PollingDataSource ds = new PollingDataSource( + CONTEXT, dataSourceUpdateSink, 100, 50, Long.MAX_VALUE, + fetcher, executorWithoutCancellation, logging.logger); + fetcher.setupSuccessResponse("{}"); + + // Stop the data source while its first poll waits to run. + ds.start(LDUtil.noOpCallback()); + ds.stop(LDUtil.noOpCallback()); + + // A stopped data source sends no request, even from a poll it could not cancel. + requireNoMoreValues(fetcher.receivedContexts, 500, TimeUnit.MILLISECONDS); + } + + @Test + public void fetchResultIsIgnoredWhenStopHappensWhileFetchIsInFlight() throws Exception { + BlockingQueue> heldFetches = new LinkedBlockingQueue<>(); + FeatureFetcher holdingFetcher = new FeatureFetcher() { + @Override + public void fetch(LDContext context, Callback callback) { + heldFetches.add(callback); + } + + @Override + public void close() {} + }; + + PollingDataSource ds = makeDataSource(holdingFetcher, taskExecutor, 50); + List results = new ArrayList<>(); + + // Start a poll and wait until its request is outstanding. + ds.start(new Callback() { + @Override + public void onSuccess(Boolean result) { + results.add("success"); + } + + @Override + public void onError(Throwable e) { + results.add("error"); + } + }); + Callback heldFetch = requireValue(heldFetches, 500, TimeUnit.MILLISECONDS); + + // Stop the data source, then let the outstanding request complete. + ds.stop(LDUtil.noOpCallback()); + heldFetch.onSuccess("{}"); + + // A result that arrives after stop does not reach the consumer. + assertEquals(0, results.size()); + } + + @Test + public void pollingContinuesWhenResultCallbackThrowsOnSuccess() throws Exception { + PollingDataSource ds = makeDataSource(fetcher, taskExecutor, 50); + + fetcher.setupSuccessResponse("{}"); + fetcher.setupSuccessResponse("{}"); + + try { + // Throw out of the success callback of every poll. + ds.start(new Callback() { + @Override + public void onSuccess(Boolean result) { + throw new RuntimeException("consumer failure"); + } + + @Override + public void onError(Throwable e) { + } + }); + requireValue(fetcher.receivedContexts, 500, TimeUnit.MILLISECONDS); + + // A throwing consumer does not end the poll chain. + requireValue(fetcher.receivedContexts, 500, TimeUnit.MILLISECONDS); + } finally { + ds.stop(LDUtil.noOpCallback()); + } + } + + @Test + public void pollingContinuesWhenResultCallbackThrowsOnError() throws Exception { + PollingDataSource ds = makeDataSource(fetcher, taskExecutor, 50); + + fetcher.setupErrorResponse(new LDFailure("network error", LDFailure.FailureType.NETWORK_FAILURE)); + fetcher.setupSuccessResponse("{}"); + + try { + // Throw out of the error callback, which the first poll triggers by failing. + ds.start(new Callback() { + @Override + public void onSuccess(Boolean result) { + } + + @Override + public void onError(Throwable e) { + throw new RuntimeException("consumer failure"); + } + }); + requireValue(fetcher.receivedContexts, 500, TimeUnit.MILLISECONDS); + + // A failed poll whose consumer also throws still leads to the next poll. + requireValue(fetcher.receivedContexts, 500, TimeUnit.MILLISECONDS); + } finally { + ds.stop(LDUtil.noOpCallback()); + } + } + private class MockFetcher implements FeatureFetcher { BlockingQueue receivedContexts = new LinkedBlockingQueue<>(); BlockingQueue responses = new LinkedBlockingQueue<>(); From a97373e1c67e08698771f5f30d9603e7dbcd5756 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Thu, 8 Oct 2026 23:44:53 -0700 Subject: [PATCH 03/14] test: Add unit tests for PollingRetryState --- .../sdk/android/PollingRetryStateTest.java | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingRetryStateTest.java diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingRetryStateTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingRetryStateTest.java new file mode 100644 index 000000000..449a4910f --- /dev/null +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingRetryStateTest.java @@ -0,0 +1,99 @@ +package com.launchdarkly.sdk.android; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +public class PollingRetryStateTest { + private static final long POLL_INTERVAL_MS = 1_000; + private static final long EXTENDED_INITIAL_DELAY_MS = 300_000; + + @Test + public void ordinaryFailuresKeepPollInterval() { + Throwable[] ordinaryFailures = { + new LDFailure("network error", LDFailure.FailureType.NETWORK_FAILURE), + new LDInvalidResponseCodeFailure("bad request", 400, true), + new LDInvalidResponseCodeFailure("request timeout", 408, true), + new LDInvalidResponseCodeFailure("too many requests", 429, true), + new LDInvalidResponseCodeFailure("server error", 503, true), + }; + + for (Throwable failure : ordinaryFailures) { + PollingRetryState state = new PollingRetryState(POLL_INTERVAL_MS); + + state.recordFailure(failure); + + // A poll interval is already a wait, so an ordinary failure does not extend it. + assertEquals(failure.toString(), POLL_INTERVAL_MS, state.nextDelayMillis()); + } + } + + @Test + public void unexpectedStatusesExtendTheWait() { + int[] unexpectedStatuses = {401, 403, 404, 405}; + + for (int status : unexpectedStatuses) { + PollingRetryState state = new PollingRetryState(POLL_INTERVAL_MS); + + state.recordFailure(new LDInvalidResponseCodeFailure("status " + status, status, true)); + + // The wait becomes minutes long. Jitter can halve a delay. + assertTrue("status " + status, + state.nextDelayMillis() >= EXTENDED_INITIAL_DELAY_MS / 2); + } + } + + @Test + public void extendedWaitNeverDropsBelowPollInterval() { + // This poll interval is longer than the extended delay. + PollingRetryState state = new PollingRetryState(3_600_000); + + state.recordFailure(new LDInvalidResponseCodeFailure("unauthorized", 401, true)); + + // A failing source never polls more often than a healthy one. + assertEquals(3_600_000, state.nextDelayMillis()); + } + + @Test + public void successReturnsToPollIntervalImmediately() { + PollingRetryState state = new PollingRetryState(POLL_INTERVAL_MS); + state.recordFailure(new LDInvalidResponseCodeFailure("unauthorized", 401, true)); + + // The wait is extended before the success. + assertTrue(state.nextDelayMillis() >= EXTENDED_INITIAL_DELAY_MS / 2); + + // One poll succeeds. + state.recordSuccess(); + + // A success shows that the service works, so polling resumes at its interval. + assertEquals(POLL_INTERVAL_MS, state.nextDelayMillis()); + } + + @Test + public void oneSuccessDoesNotEndTheExtendedRegime() { + PollingRetryState state = new PollingRetryState(POLL_INTERVAL_MS); + state.recordFailure(new LDInvalidResponseCodeFailure("unauthorized", 401, true)); + + // One poll succeeds, then the next one fails for an ordinary reason. + state.recordSuccess(); + state.recordFailure(new LDFailure("network error", LDFailure.FailureType.NETWORK_FAILURE)); + + // One success is not enough to leave the extended regime. + assertTrue(state.nextDelayMillis() >= EXTENDED_INITIAL_DELAY_MS / 2); + } + + @Test + public void twoSuccessesEndTheExtendedRegime() { + PollingRetryState state = new PollingRetryState(POLL_INTERVAL_MS); + state.recordFailure(new LDInvalidResponseCodeFailure("unauthorized", 401, true)); + + // Two polls succeed, then the next one fails for an ordinary reason. + state.recordSuccess(); + state.recordSuccess(); + state.recordFailure(new LDFailure("network error", LDFailure.FailureType.NETWORK_FAILURE)); + + // Enough successes end the extended regime, so the wait is the poll interval again. + assertEquals(POLL_INTERVAL_MS, state.nextDelayMillis()); + } +} From a54d5a25b9dad15bae3b8b0ad13b36e49a28c889 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Fri, 9 Oct 2026 09:12:32 -0700 Subject: [PATCH 04/14] docs: Correct shutDown and SET_OFFLINE docs and restore the unexpected-error log to ERROR --- .../launchdarkly/sdk/android/ConnectionInformation.java | 6 +++--- .../com/launchdarkly/sdk/android/StreamingDataSource.java | 3 ++- .../sdk/android/subsystems/DataSourceUpdateSink.java | 7 +++++-- .../sdk/android/subsystems/DataSourceUpdateSinkV2.java | 7 +++++-- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectionInformation.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectionInformation.java index a8a62fd8d..15af7cc68 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectionInformation.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectionInformation.java @@ -38,9 +38,9 @@ enum ConnectionMode { OFFLINE(false), /** - * The SDK has been explicitly set offline, either in the initial configuration, by - * {@link LDClient#setOffline()}, or as a result of failed authentication to LaunchDarkly. The SDK will stay - * offline unless {@link LDClient#setOnline()} is called. + * The SDK has been explicitly set offline, either in the initial configuration or by + * {@link LDClient#setOffline()}. The SDK will stay offline unless + * {@link LDClient#setOnline()} is called. */ SET_OFFLINE(false), diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java index fc076b5a1..167a2c5dd 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java @@ -181,7 +181,8 @@ public void onError(Throwable t) { !LDUtil.isHttpErrorRecoverable(((StreamHttpErrorException) t).getCode())) { current.getEventSource() .activateRetryDelayStrategy(extendedRetryDelay); - logger.info("Classified failure as unexpected. Verify correct Mobile Key and Stream URI. Engaging extended backoff"); + logger.error("Encountered unexpected error: {}. Verify correct Mobile Key and Stream URI. Engaging extended backoff", + ((StreamHttpErrorException) t).getCode()); } return ConnectionErrorHandler.Action.PROCEED; }) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/subsystems/DataSourceUpdateSink.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/subsystems/DataSourceUpdateSink.java index c1d8be9a9..9d1353dbe 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/subsystems/DataSourceUpdateSink.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/subsystems/DataSourceUpdateSink.java @@ -44,12 +44,15 @@ public interface DataSourceUpdateSink { void setStatus(@NonNull ConnectionInformation.ConnectionMode connectionMode, @Nullable Throwable failure); /** - * Informs the SDK that the data source is being permanently shut down due to an unrecoverable - * problem reported by LaunchDarkly, such as the mobile key being invalid. + * Informs the SDK that the data source is permanently shutting down and will send no more + * data. *

* This implies that the SDK should also stop other components that communicate with * LaunchDarkly, such as the event processor. It also changes the connection mode to * {@link com.launchdarkly.sdk.android.ConnectionInformation.ConnectionMode#SHUTDOWN}. + *

+ * The data sources in this SDK do not call this method. They retry after a failure instead, + * and they wait longer after a failure that is unlikely to correct itself. */ void shutDown(); } diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/subsystems/DataSourceUpdateSinkV2.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/subsystems/DataSourceUpdateSinkV2.java index 81d97b007..435ab0ab9 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/subsystems/DataSourceUpdateSinkV2.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/subsystems/DataSourceUpdateSinkV2.java @@ -20,13 +20,16 @@ public interface DataSourceUpdateSinkV2 extends TransactionalDataSourceUpdateSin void setStatus(@NonNull DataSourceState state, @Nullable Throwable failure); /** - * Informs the SDK that the data source is being permanently shut down due to an - * unrecoverable problem reported by LaunchDarkly, such as the mobile key being invalid. + * Informs the SDK that the data source is permanently shutting down and will send no + * more data. *

* This implies that the SDK should also stop other components that communicate * with LaunchDarkly, such as the event processor. It also changes the connection * mode to * {@link com.launchdarkly.sdk.android.ConnectionInformation.ConnectionMode#SHUTDOWN}. + *

+ * The data sources in this SDK do not call this method. They retry after a failure + * instead, and they wait longer after a failure that is unlikely to correct itself. */ void shutDown(); } From 162bf0d5a502dc75bbc9058d5b681213d879de95 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Fri, 9 Oct 2026 09:26:44 -0700 Subject: [PATCH 05/14] test: Assert that a 401 keeps the stream retrying without shutting down the sink --- .../sdk/android/StreamingDataSourceTest.java | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java index 402c86f87..3401acd13 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java @@ -748,6 +748,26 @@ public void startWhileAlreadyRunningIsANoOp() throws Exception { } } + @Test + public void startWithHttp401KeepsTheSinkRunning() throws Exception { + try (HttpServer server = HttpServer.start(Handlers.status(401))) { + StreamingDataSource sds = makeStreamingDataSource( + server.getUri(), false, false); + TrackingCallback callback = new TrackingCallback(); + + startDataSource(sds, callback); + Throwable error = callback.awaitError(); + + // A rejected credential is retryable now, because the stream keeps reconnecting. + assertNotNull(error); + assertTrue(error instanceof LDInvalidResponseCodeFailure); + LDInvalidResponseCodeFailure failure = (LDInvalidResponseCodeFailure) error; + assertEquals(401, failure.getResponseCode()); + assertTrue(failure.isRetryable()); + assertFalse(dataSourceUpdateSink.shutDownCalled); + } + } + // --- start(): SSE event processing via HttpServer --- @Test From e1cb5920f353235a23122d4ca4474f68220ac9e8 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Fri, 9 Oct 2026 09:49:47 -0700 Subject: [PATCH 06/14] refactor: Add isHttpErrorUnexpected and use it from the FDv1 data sources --- .../com/launchdarkly/sdk/android/LDUtil.java | 9 +++++++++ .../sdk/android/PollingRetryState.java | 2 +- .../sdk/android/StreamingDataSource.java | 2 +- .../com/launchdarkly/sdk/android/LDUtilTest.java | 16 ++++++++++++++++ 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDUtil.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDUtil.java index ae8ed1f5b..4a77849b2 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDUtil.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/LDUtil.java @@ -230,6 +230,15 @@ static boolean isHttpErrorRecoverable(int statusCode) { return true; } + /** + * Tests whether an HTTP error status represents a condition that is unlikely to resolve on its own. + * @param statusCode the HTTP status + * @return true if the status is unlikely to change, such as a rejected credential; false otherwise + */ + static boolean isHttpErrorUnexpected(int statusCode) { + return !isHttpErrorRecoverable(statusCode); + } + static void logExceptionAtErrorLevel(LDLogger logger, Throwable ex, String msgFormat, Object... msgArgs) { logException(logger, ex, true, msgFormat, msgArgs); } diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingRetryState.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingRetryState.java index e22aa2408..7796e05fc 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingRetryState.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingRetryState.java @@ -56,7 +56,7 @@ void recordFailure(@Nullable Throwable error) { // True for an HTTP status the service is unlikely to stop returning, such as a 401. private static boolean isUnexpected(@Nullable Throwable error) { return error instanceof LDInvalidResponseCodeFailure - && !LDUtil.isHttpErrorRecoverable( + && LDUtil.isHttpErrorUnexpected( ((LDInvalidResponseCodeFailure) error).getResponseCode()); } diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java index 167a2c5dd..5ee1ab92e 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java @@ -178,7 +178,7 @@ public void onError(Throwable t) { .connectionErrorHandler(t -> { BackgroundEventSource current = es; if (current != null && t instanceof StreamHttpErrorException && - !LDUtil.isHttpErrorRecoverable(((StreamHttpErrorException) t).getCode())) { + LDUtil.isHttpErrorUnexpected(((StreamHttpErrorException) t).getCode())) { current.getEventSource() .activateRetryDelayStrategy(extendedRetryDelay); logger.error("Encountered unexpected error: {}. Verify correct Mobile Key and Stream URI. Engaging extended backoff", diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/LDUtilTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/LDUtilTest.java index 01b1d4114..0d60cfb45 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/LDUtilTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/LDUtilTest.java @@ -27,4 +27,20 @@ public void testSanitizeSpaces() { Assert.assertEquals("--hello--", LDUtil.sanitizeSpaces(" hello ")); Assert.assertEquals("world", LDUtil.sanitizeSpaces("world")); } + + @Test + public void testIsHttpErrorUnexpected() { + // Statuses that are unlikely to resolve on their own. + Assert.assertTrue(LDUtil.isHttpErrorUnexpected(401)); + Assert.assertTrue(LDUtil.isHttpErrorUnexpected(403)); + Assert.assertTrue(LDUtil.isHttpErrorUnexpected(404)); + Assert.assertTrue(LDUtil.isHttpErrorUnexpected(405)); + + // Statuses that the service is likely to stop returning. + Assert.assertFalse(LDUtil.isHttpErrorUnexpected(400)); + Assert.assertFalse(LDUtil.isHttpErrorUnexpected(408)); + Assert.assertFalse(LDUtil.isHttpErrorUnexpected(429)); + Assert.assertFalse(LDUtil.isHttpErrorUnexpected(500)); + Assert.assertFalse(LDUtil.isHttpErrorUnexpected(503)); + } } From e32d25eac82aa463eac81f108c539c2b367ec2f9 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Fri, 9 Oct 2026 10:45:22 -0700 Subject: [PATCH 07/14] test: Assert that the HTTP status selects the streaming retry regime --- .../sdk/android/StreamingDataSourceTest.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java index 3401acd13..0821c5d06 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java @@ -768,6 +768,36 @@ public void startWithHttp401KeepsTheSinkRunning() throws Exception { } } + @Test + public void recoverableErrorReconnectsWithoutExtendedBackoff() throws Exception { + try (HttpServer server = HttpServer.start(Handlers.status(503))) { + StreamingDataSource sds = makeStreamingDataSource( + server.getUri(), dataSourceUpdateSink, false, false, 1); + TrackingCallback callback = new TrackingCallback(); + + startDataSource(sds, callback); + + // The normal regime starts at the configured 1ms delay, so the retry arrives at once. + server.getRecorder().requireRequest(); + server.getRecorder().requireRequest(); + } + } + + @Test + public void unexpectedErrorReconnectsWithExtendedBackoff() throws Exception { + try (HttpServer server = HttpServer.start(Handlers.status(401))) { + StreamingDataSource sds = makeStreamingDataSource( + server.getUri(), dataSourceUpdateSink, false, false, 1); + TrackingCallback callback = new TrackingCallback(); + + startDataSource(sds, callback); + server.getRecorder().requireRequest(); + + // The extended regime waits minutes, so no retry arrives in this window. + server.getRecorder().requireNoRequests(500, TimeUnit.MILLISECONDS); + } + } + // --- start(): SSE event processing via HttpServer --- @Test From 49eb0ed579a274485a4de4e557931cdcaa656015 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Fri, 9 Oct 2026 11:13:58 -0700 Subject: [PATCH 08/14] test: Make the extended reconnect delays injectable and assert the retry happens --- .../sdk/android/StreamingDataSource.java | 20 +++++++++- .../sdk/android/StreamingDataSourceTest.java | 40 ++++++++++++++++--- 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java index 5ee1ab92e..3bf6ab92b 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/StreamingDataSource.java @@ -83,6 +83,22 @@ final class StreamingDataSource implements DataSource { @NonNull FeatureFetcher fetcher, int initialReconnectDelayMillis, boolean streamEvenInBackground + ) { + this(clientContext, context, dataSourceUpdateSink, fetcher, initialReconnectDelayMillis, + streamEvenInBackground, EXTENDED_INITIAL_RECONNECT_DELAY_MS, + EXTENDED_MAX_RECONNECT_TIME_MS); + } + + @VisibleForTesting + StreamingDataSource( + @NonNull ClientContext clientContext, + @NonNull LDContext context, + @NonNull DataSourceUpdateSink dataSourceUpdateSink, + @NonNull FeatureFetcher fetcher, + int initialReconnectDelayMillis, + boolean streamEvenInBackground, + long extendedInitialReconnectDelayMillis, + long extendedMaxReconnectTimeMillis ) { this.context = context; this.dataSourceUpdateSink = dataSourceUpdateSink; @@ -96,8 +112,8 @@ final class StreamingDataSource implements DataSource { .initialDelay(initialReconnectDelayMillis, TimeUnit.MILLISECONDS) .maxDelay(MAX_RECONNECT_TIME_MS, TimeUnit.MILLISECONDS); this.extendedRetryDelay = RetryDelayStrategy.defaultStrategy() - .initialDelay(EXTENDED_INITIAL_RECONNECT_DELAY_MS, TimeUnit.MILLISECONDS) - .maxDelay(EXTENDED_MAX_RECONNECT_TIME_MS, TimeUnit.MILLISECONDS); + .initialDelay(extendedInitialReconnectDelayMillis, TimeUnit.MILLISECONDS) + .maxDelay(extendedMaxReconnectTimeMillis, TimeUnit.MILLISECONDS); this.streamEvenInBackground = streamEvenInBackground; this.diagnosticStore = ClientContextImpl.get(clientContext).getDiagnosticStore(); this.logger = clientContext.getBaseLogger(); diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java index 0821c5d06..c0be925de 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/StreamingDataSourceTest.java @@ -148,6 +148,17 @@ private StreamingDataSource makeStreamingDataSource( MockComponents.MockDataSourceUpdateSink sink, boolean evaluationReasons, boolean useReport, int initialReconnectDelayMillis) { + ClientContext clientContext = makeDataSourceClientContext( + streamBaseUri, sink, evaluationReasons, useReport); + return (StreamingDataSource) Components.streamingDataSource() + .initialReconnectDelayMillis(initialReconnectDelayMillis) + .build(clientContext); + } + + private ClientContext makeDataSourceClientContext( + URI streamBaseUri, + MockComponents.MockDataSourceUpdateSink sink, + boolean evaluationReasons, boolean useReport) { LDConfig.Builder configBuilder = new LDConfig.Builder(AutoEnvAttributes.Disabled) .serviceEndpoints(Components.serviceEndpoints().streaming(streamBaseUri)) .evaluationReasons(evaluationReasons); @@ -158,11 +169,7 @@ private StreamingDataSource makeStreamingDataSource( configBuilder.build(), MOBILE_KEY, "", perEnvironmentData, makeFeatureFetcher(), CONTEXT, logging.logger, platformState, environmentReporter, taskExecutor); - ClientContext clientContext = ClientContextImpl.forDataSource( - baseClientContext, sink, CONTEXT, false, false); - return (StreamingDataSource) Components.streamingDataSource() - .initialReconnectDelayMillis(initialReconnectDelayMillis) - .build(clientContext); + return ClientContextImpl.forDataSource(baseClientContext, sink, CONTEXT, false, false); } private static String makeSseEvent(String type, String data) { @@ -798,6 +805,29 @@ public void unexpectedErrorReconnectsWithExtendedBackoff() throws Exception { } } + @Test + public void unexpectedErrorRetriesAfterTheExtendedDelay() throws Exception { + try (HttpServer server = HttpServer.start(Handlers.status(401))) { + ClientContext clientContext = makeDataSourceClientContext( + server.getUri(), dataSourceUpdateSink, false, false); + long extendedInitialDelayMillis = 400; + long extendedMaxDelayMillis = 800; + StreamingDataSource sds = new StreamingDataSource( + clientContext, CONTEXT, dataSourceUpdateSink, makeFeatureFetcher(), + 1, false, extendedInitialDelayMillis, extendedMaxDelayMillis); + TrackingCallback callback = new TrackingCallback(); + + startDataSource(sds, callback); + server.getRecorder().requireRequest(); + + // Jitter can halve the extended delay, so a retry cannot arrive this soon. + server.getRecorder().requireNoRequests(100, TimeUnit.MILLISECONDS); + + // The stream does retry once the extended delay elapses. + server.getRecorder().requireRequest(); + } + } + // --- start(): SSE event processing via HttpServer --- @Test From 80b04fcf06529675842b7e22a20ad81b0e619433 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 12 Oct 2026 15:43:01 -0700 Subject: [PATCH 09/14] fix: Always complete the startup callback when its data source goes away --- .../sdk/android/ConnectivityManager.java | 47 ++++- .../sdk/android/ConnectivityManagerTest.java | 174 ++++++++++++++++++ 2 files changed, 212 insertions(+), 9 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java index cbeaf9fb2..c24fe920e 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java @@ -82,7 +82,8 @@ class ConnectivityManager implements ContextDataManager.ContextSwitchListener { private final AtomicReference currentContext = new AtomicReference<>(); private volatile ContextDataManager.ContextDataManagerView currentView; - @Nullable private volatile Callback pendingStartUpCallback; + @Nullable private volatile Callback pendingStartupCallback; + private final PendingStartupCallbackHandler pendingStartupCallbackHandler = new PendingStartupCallbackHandler(); // The DataSourceUpdateSinkImpl receives flag updates and status updates from the DataSource. // Data operations (init, upsert, apply) are routed through a ContextDataManager.ContextDataManagerView @@ -143,6 +144,19 @@ public void shutDown() { } } + private class PendingStartupCallbackHandler implements Callback { + @Override + public void onSuccess(Void result) { + clearPendingStartupCallback(result); + } + + @Override + public void onError(Throwable error) { + // updateDataSource never actually calls onError, but if it did, this would be right. + clearPendingStartupCallback(error); + } + } + ConnectivityManager(@NonNull final ClientContext clientContext, @NonNull final ComponentConfigurer dataSourceFactory, @NonNull final EventProcessor eventProcessor, @@ -206,6 +220,22 @@ public void shutDown() { platformState.addForegroundChangeListener(foregroundListener); } + private synchronized void clearPendingStartupCallback(Void result) { + if (pendingStartupCallback == null) { + return; + } + pendingStartupCallback.onSuccess(result); + pendingStartupCallback = null; + } + + private synchronized void clearPendingStartupCallback(Throwable error) { + if (pendingStartupCallback == null) { + return; + } + pendingStartupCallback.onError(error); + pendingStartupCallback = null; + } + @Override public synchronized void onContextChanged( @NonNull LDContext context, @@ -233,13 +263,7 @@ public synchronized void onContextChanged( platformState.isForeground() ); - Callback effectiveCallback; - if (pendingStartUpCallback != null) { - effectiveCallback = LDUtil.compositeCallback(Arrays.asList(pendingStartUpCallback, onCompletion)); - pendingStartUpCallback = null; - } else { - effectiveCallback = onCompletion; - } + Callback effectiveCallback = LDUtil.compositeCallback(Arrays.asList(pendingStartupCallbackHandler, onCompletion)); ModeState state = snapshotModeState(); updateEventProcessor(forcedOffline.get(), state.isNetworkAvailable(), state.isForeground()); @@ -337,6 +361,7 @@ private boolean updateDataSource( DataSource oldDataSource = currentDataSource.getAndSet(null); if (oldDataSource != null) { logger.debug("Stopping current data source"); + clearPendingStartupCallback((Void) null); oldDataSource.stop(LDUtil.noOpCallback()); } } @@ -567,11 +592,14 @@ synchronized boolean startUp( @NonNull Callback onCompletion ) { if (closed.get() || started.get()) { + // LDClient doesn't call startUp if the class has already been started, and treats it + // as a success. Handle it the same way, just in case a refactor makes it possible. + onCompletion.onSuccess(null); return false; } initialized = false; - pendingStartUpCallback = onCompletion; + pendingStartupCallback = onCompletion; started.set(true); // CDM immediately calls onContextChanged(...) after registration and that will @@ -586,6 +614,7 @@ synchronized boolean startUp( * is being closed, or if we receive an error that indicates the mobile key is invalid. */ void shutDown() { + clearPendingStartupCallback((Void) null); if (closed.getAndSet(true)) { return; } diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java index 26cc2595e..dbd794898 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java @@ -58,6 +58,7 @@ import java.util.concurrent.BlockingQueue; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; @@ -869,6 +870,179 @@ public void startupCallbackIsInvokedOnCompletion() throws ExecutionException { callback.await(); } + /** + * A data source that accepts its start callback and never reports a result, so only something + * other than the data source itself can complete startup. + */ + private ComponentConfigurer makeSilentDataSourceFactory( + BlockingQueue> startCallbacks + ) { + return clientContext -> { + receivedClientContexts.add(clientContext); + return new DataSource() { + @Override + public void start(@NonNull Callback resultCallback) { + startCallbacks.add(resultCallback); + } + + @Override + public void stop(@NonNull Callback completionCallback) { + completionCallback.onSuccess(null); + } + + @Override + public boolean needsRefresh(boolean newInBackground, LDContext newEvaluationContext) { + return true; + } + }; + }; + } + + @Test + public void startupCallbackIsInvokedWhenAStateChangeReplacesTheFirstDataSource() throws Exception { + eventProcessor.setOffline(false); + eventProcessor.setInBackground(false); + replayAll(); + + BlockingQueue> startCallbacks = new LinkedBlockingQueue<>(); + createTestManager(defaultTestConfig(false, false), makeSilentDataSourceFactory(startCallbacks)); + + AwaitableCallback startupCallback = new AwaitableCallback<>(); + connectivityManager.startUp(contextDataManager, startupCallback); + requireValue(startCallbacks, 1, TimeUnit.SECONDS, "data source start"); + + resetAll(); + eventProcessor.setOffline(false); + eventProcessor.setInBackground(true); + eventProcessor.flush(); + replayAll(); + + // Moving to the background replaces the data source before it ever reports. + mockPlatformState.setAndNotifyForegroundChangeListeners(false); + + // Startup completes, because no data source will ever report for it. + startupCallback.await(5000); + } + + @Test + public void startupCallbackIsInvokedWhenTheClientIsClosedDuringStartup() throws Exception { + eventProcessor.setOffline(false); + eventProcessor.setInBackground(false); + replayAll(); + + BlockingQueue> startCallbacks = new LinkedBlockingQueue<>(); + createTestManager(defaultTestConfig(false, false), makeSilentDataSourceFactory(startCallbacks)); + + AwaitableCallback startupCallback = new AwaitableCallback<>(); + connectivityManager.startUp(contextDataManager, startupCallback); + requireValue(startCallbacks, 1, TimeUnit.SECONDS, "data source start"); + + // Close while the first request is still outstanding. + connectivityManager.shutDown(); + + // Startup completes, because the data source is gone and will never report. + startupCallback.await(5000); + } + + @Test + public void startupCallbackIsInvokedWhenTheManagerHasAlreadyStarted() throws Exception { + eventProcessor.setOffline(false); + eventProcessor.setInBackground(false); + replayAll(); + + createTestManager(defaultTestConfig(false, false), makeSuccessfulDataSourceFactory()); + awaitStartUp(); + + // A second startUp is refused, but its caller is still waiting on a callback. + AwaitableCallback secondStartupCallback = new AwaitableCallback<>(); + connectivityManager.startUp(contextDataManager, secondStartupCallback); + + secondStartupCallback.await(5000); + } + + @Test + public void startupCallbackIsInvokedWhenTheManagerHasAlreadyBeenClosed() throws Exception { + eventProcessor.setOffline(false); + eventProcessor.setInBackground(false); + replayAll(); + + createTestManager(defaultTestConfig(false, false), makeSuccessfulDataSourceFactory()); + connectivityManager.shutDown(); + + // Starting a closed manager is refused, but its caller is still waiting on a callback. + AwaitableCallback startupCallback = new AwaitableCallback<>(); + connectivityManager.startUp(contextDataManager, startupCallback); + + startupCallback.await(5000); + } + + @Test + public void startupCallbackIsInvokedOnlyOnceWhenTheDataSourceReportsRepeatedly() { + eventProcessor.setOffline(false); + eventProcessor.setInBackground(false); + replayAll(); + + // A polling data source reports the result of every poll on the same callback. + ComponentConfigurer repeatedlyReportingDataSource = clientContext -> { + receivedClientContexts.add(clientContext); + return new DataSource() { + @Override + public void start(@NonNull Callback resultCallback) { + clientContext.getDataSourceUpdateSink() + .setStatus(ConnectionMode.POLLING, null); + resultCallback.onSuccess(true); + resultCallback.onSuccess(true); + resultCallback.onSuccess(true); + } + + @Override + public void stop(@NonNull Callback completionCallback) { + completionCallback.onSuccess(null); + } + + @Override + public boolean needsRefresh(boolean newInBackground, LDContext newEvaluationContext) { + return true; + } + }; + }; + createTestManager(defaultTestConfig(false, false), repeatedlyReportingDataSource); + + AtomicInteger completions = new AtomicInteger(); + connectivityManager.startUp(contextDataManager, new Callback() { + @Override + public void onSuccess(Void result) { + completions.incrementAndGet(); + } + + @Override + public void onError(Throwable e) { + } + }); + + // LDClient counts these to decide when init is done, so an extra call resolves init early. + assertEquals(1, completions.get()); + } + + @Test + public void identifyCallbackIsInvokedAfterStartup() throws Exception { + // One pair per context change: startup, then the switch below. + eventProcessor.setOffline(false); + eventProcessor.setInBackground(false); + eventProcessor.setOffline(false); + eventProcessor.setInBackground(false); + replayAll(); + + createTestManager(defaultTestConfig(false, false), makeSuccessfulDataSourceFactory()); + awaitStartUp(); + + // A context switch after startup runs the identify path rather than the startup path. + AwaitableCallback identifyCallback = new AwaitableCallback<>(); + contextDataManager.switchToContext(LDContext.create("other-context"), false, identifyCallback); + + identifyCallback.await(5000); + } + private ComponentConfigurer makeSuccessfulDataSourceFactory() { return clientContext -> makeSuccessfulDataSource(clientContext); } From 889fc9c54363843283546d7cc2b6fa87de9f4450 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Mon, 12 Oct 2026 16:40:31 -0700 Subject: [PATCH 10/14] fix: Always complete the identify callback when its data source goes away --- .../sdk/android/ConnectivityManager.java | 38 +++++-- .../sdk/android/ConnectivityManagerTest.java | 100 ++++++++++++++++++ 2 files changed, 127 insertions(+), 11 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java index c24fe920e..423f508e7 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java @@ -82,8 +82,10 @@ class ConnectivityManager implements ContextDataManager.ContextSwitchListener { private final AtomicReference currentContext = new AtomicReference<>(); private volatile ContextDataManager.ContextDataManagerView currentView; - @Nullable private volatile Callback pendingStartupCallback; + @Nullable private Callback pendingStartupCallback; private final PendingStartupCallbackHandler pendingStartupCallbackHandler = new PendingStartupCallbackHandler(); + // The callback waiting on the data source that updateDataSource most recently created. + @Nullable private Callback pendingDataSourceCallback; // The DataSourceUpdateSinkImpl receives flag updates and status updates from the DataSource. // Data operations (init, upsert, apply) are routed through a ContextDataManager.ContextDataManagerView @@ -147,7 +149,7 @@ public void shutDown() { private class PendingStartupCallbackHandler implements Callback { @Override public void onSuccess(Void result) { - clearPendingStartupCallback(result); + clearPendingStartupCallback(); } @Override @@ -220,20 +222,31 @@ public void onError(Throwable error) { platformState.addForegroundChangeListener(foregroundListener); } - private synchronized void clearPendingStartupCallback(Void result) { - if (pendingStartupCallback == null) { + private synchronized void clearPendingStartupCallback() { + Callback callback = pendingStartupCallback; + if (callback == null) { return; } - pendingStartupCallback.onSuccess(result); pendingStartupCallback = null; + callback.onSuccess(null); + } + + private synchronized void clearPendingDataSourceCallback() { + Callback callback = pendingDataSourceCallback; + if (callback == null) { + return; + } + pendingDataSourceCallback = null; + callback.onSuccess(null); } private synchronized void clearPendingStartupCallback(Throwable error) { - if (pendingStartupCallback == null) { + Callback callback = pendingStartupCallback; + if (callback == null) { return; } - pendingStartupCallback.onError(error); pendingStartupCallback = null; + callback.onError(error); } @Override @@ -361,7 +374,8 @@ private boolean updateDataSource( DataSource oldDataSource = currentDataSource.getAndSet(null); if (oldDataSource != null) { logger.debug("Stopping current data source"); - clearPendingStartupCallback((Void) null); + clearPendingDataSourceCallback(); + clearPendingStartupCallback(); oldDataSource.stop(LDUtil.noOpCallback()); } } @@ -395,19 +409,20 @@ private boolean updateDataSource( DataSource dataSource = dataSourceFactory.build(clientContext); currentDataSource.set(dataSource); previousModeState.set(newState); + pendingDataSourceCallback = onCompletion; dataSource.start(new Callback() { @Override public void onSuccess(Boolean result) { initialized = true; updateConnectionInfoForSuccess(connectionInformation.getConnectionMode()); - onCompletion.onSuccess(null); + clearPendingDataSourceCallback(); } @Override public void onError(Throwable error) { updateConnectionInfoForError(connectionInformation.getConnectionMode(), error); - onCompletion.onSuccess(null); + clearPendingDataSourceCallback(); } }); @@ -614,7 +629,8 @@ synchronized boolean startUp( * is being closed, or if we receive an error that indicates the mobile key is invalid. */ void shutDown() { - clearPendingStartupCallback((Void) null); + clearPendingDataSourceCallback(); + clearPendingStartupCallback(); if (closed.getAndSet(true)) { return; } diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java index dbd794898..6083e8dfc 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java @@ -1043,6 +1043,106 @@ public void identifyCallbackIsInvokedAfterStartup() throws Exception { identifyCallback.await(5000); } + /** + * Completes startup with a reporting data source, then builds data sources that accept their + * start callback and never report, so an identify that creates one stays pending. + */ + private ComponentConfigurer makeSuccessfulThenSilentDataSourceFactory() { + AtomicInteger builds = new AtomicInteger(); + return clientContext -> { + if (builds.getAndIncrement() == 0) { + return makeSuccessfulDataSource(clientContext); + } + receivedClientContexts.add(clientContext); + return new DataSource() { + @Override + public void start(@NonNull Callback resultCallback) { + } + + @Override + public void stop(@NonNull Callback completionCallback) { + completionCallback.onSuccess(null); + } + + @Override + public boolean needsRefresh(boolean newInBackground, LDContext newEvaluationContext) { + return true; + } + }; + }; + } + + @Test + public void identifyCallbackIsInvokedWhenAStateChangeDiscardsItsDataSource() throws Exception { + // One pair for startup's context change. + eventProcessor.setOffline(false); + eventProcessor.setInBackground(false); + replayAll(); + + createTestManager(defaultTestConfig(false, false), makeSuccessfulThenSilentDataSourceFactory()); + awaitStartUp(); + + // One pair for the identify's context change, then one for going offline. + resetAll(); + eventProcessor.setOffline(false); + eventProcessor.setInBackground(false); + eventProcessor.setOffline(true); + eventProcessor.setInBackground(false); + replayAll(); + + // Identify, then go offline, which discards the data source the identify waits on. + AwaitableCallback identifyCallback = new AwaitableCallback<>(); + contextDataManager.switchToContext(LDContext.create("other-context"), false, identifyCallback); + connectivityManager.setForceOffline(true); + + // The identify completes, because no data source will ever report for it. + identifyCallback.await(5000); + } + + @Test + public void identifyCallbackIsInvokedWhenASecondIdentifySupersedesIt() throws Exception { + // One pair per context change: startup, then each of the two identifies. + eventProcessor.setOffline(false); + eventProcessor.setInBackground(false); + eventProcessor.setOffline(false); + eventProcessor.setInBackground(false); + eventProcessor.setOffline(false); + eventProcessor.setInBackground(false); + replayAll(); + + createTestManager(defaultTestConfig(false, false), makeSuccessfulThenSilentDataSourceFactory()); + awaitStartUp(); + + // A second identify discards the data source the first one waits on. + AwaitableCallback firstIdentify = new AwaitableCallback<>(); + contextDataManager.switchToContext(LDContext.create("context-b"), false, firstIdentify); + contextDataManager.switchToContext(LDContext.create("context-c"), false, new AwaitableCallback<>()); + + // The superseded identify completes rather than waiting forever. + firstIdentify.await(5000); + } + + @Test + public void identifyCallbackIsInvokedWhenTheClientIsClosedDuringIdentify() throws Exception { + // One pair per context change: startup, then the identify. + eventProcessor.setOffline(false); + eventProcessor.setInBackground(false); + eventProcessor.setOffline(false); + eventProcessor.setInBackground(false); + replayAll(); + + createTestManager(defaultTestConfig(false, false), makeSuccessfulThenSilentDataSourceFactory()); + awaitStartUp(); + + // Close while the identify's request is still outstanding. + AwaitableCallback identifyCallback = new AwaitableCallback<>(); + contextDataManager.switchToContext(LDContext.create("other-context"), false, identifyCallback); + connectivityManager.shutDown(); + + // The identify completes, because the data source is gone and will never report. + identifyCallback.await(5000); + } + private ComponentConfigurer makeSuccessfulDataSourceFactory() { return clientContext -> makeSuccessfulDataSource(clientContext); } From bbc71e9ac5e3916a8ea534ce82dde88d53ea0336 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Tue, 13 Oct 2026 10:30:43 -0700 Subject: [PATCH 11/14] Revert "fix: Always complete the identify callback when its data source goes away" This reverts commit b2eefc4ae2da27205f5ef87ccc33db8b19622721. --- .../sdk/android/ConnectivityManager.java | 38 ++----- .../sdk/android/ConnectivityManagerTest.java | 100 ------------------ 2 files changed, 11 insertions(+), 127 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java index 423f508e7..c24fe920e 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java @@ -82,10 +82,8 @@ class ConnectivityManager implements ContextDataManager.ContextSwitchListener { private final AtomicReference currentContext = new AtomicReference<>(); private volatile ContextDataManager.ContextDataManagerView currentView; - @Nullable private Callback pendingStartupCallback; + @Nullable private volatile Callback pendingStartupCallback; private final PendingStartupCallbackHandler pendingStartupCallbackHandler = new PendingStartupCallbackHandler(); - // The callback waiting on the data source that updateDataSource most recently created. - @Nullable private Callback pendingDataSourceCallback; // The DataSourceUpdateSinkImpl receives flag updates and status updates from the DataSource. // Data operations (init, upsert, apply) are routed through a ContextDataManager.ContextDataManagerView @@ -149,7 +147,7 @@ public void shutDown() { private class PendingStartupCallbackHandler implements Callback { @Override public void onSuccess(Void result) { - clearPendingStartupCallback(); + clearPendingStartupCallback(result); } @Override @@ -222,31 +220,20 @@ public void onError(Throwable error) { platformState.addForegroundChangeListener(foregroundListener); } - private synchronized void clearPendingStartupCallback() { - Callback callback = pendingStartupCallback; - if (callback == null) { + private synchronized void clearPendingStartupCallback(Void result) { + if (pendingStartupCallback == null) { return; } + pendingStartupCallback.onSuccess(result); pendingStartupCallback = null; - callback.onSuccess(null); - } - - private synchronized void clearPendingDataSourceCallback() { - Callback callback = pendingDataSourceCallback; - if (callback == null) { - return; - } - pendingDataSourceCallback = null; - callback.onSuccess(null); } private synchronized void clearPendingStartupCallback(Throwable error) { - Callback callback = pendingStartupCallback; - if (callback == null) { + if (pendingStartupCallback == null) { return; } + pendingStartupCallback.onError(error); pendingStartupCallback = null; - callback.onError(error); } @Override @@ -374,8 +361,7 @@ private boolean updateDataSource( DataSource oldDataSource = currentDataSource.getAndSet(null); if (oldDataSource != null) { logger.debug("Stopping current data source"); - clearPendingDataSourceCallback(); - clearPendingStartupCallback(); + clearPendingStartupCallback((Void) null); oldDataSource.stop(LDUtil.noOpCallback()); } } @@ -409,20 +395,19 @@ private boolean updateDataSource( DataSource dataSource = dataSourceFactory.build(clientContext); currentDataSource.set(dataSource); previousModeState.set(newState); - pendingDataSourceCallback = onCompletion; dataSource.start(new Callback() { @Override public void onSuccess(Boolean result) { initialized = true; updateConnectionInfoForSuccess(connectionInformation.getConnectionMode()); - clearPendingDataSourceCallback(); + onCompletion.onSuccess(null); } @Override public void onError(Throwable error) { updateConnectionInfoForError(connectionInformation.getConnectionMode(), error); - clearPendingDataSourceCallback(); + onCompletion.onSuccess(null); } }); @@ -629,8 +614,7 @@ synchronized boolean startUp( * is being closed, or if we receive an error that indicates the mobile key is invalid. */ void shutDown() { - clearPendingDataSourceCallback(); - clearPendingStartupCallback(); + clearPendingStartupCallback((Void) null); if (closed.getAndSet(true)) { return; } diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java index 6083e8dfc..dbd794898 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java @@ -1043,106 +1043,6 @@ public void identifyCallbackIsInvokedAfterStartup() throws Exception { identifyCallback.await(5000); } - /** - * Completes startup with a reporting data source, then builds data sources that accept their - * start callback and never report, so an identify that creates one stays pending. - */ - private ComponentConfigurer makeSuccessfulThenSilentDataSourceFactory() { - AtomicInteger builds = new AtomicInteger(); - return clientContext -> { - if (builds.getAndIncrement() == 0) { - return makeSuccessfulDataSource(clientContext); - } - receivedClientContexts.add(clientContext); - return new DataSource() { - @Override - public void start(@NonNull Callback resultCallback) { - } - - @Override - public void stop(@NonNull Callback completionCallback) { - completionCallback.onSuccess(null); - } - - @Override - public boolean needsRefresh(boolean newInBackground, LDContext newEvaluationContext) { - return true; - } - }; - }; - } - - @Test - public void identifyCallbackIsInvokedWhenAStateChangeDiscardsItsDataSource() throws Exception { - // One pair for startup's context change. - eventProcessor.setOffline(false); - eventProcessor.setInBackground(false); - replayAll(); - - createTestManager(defaultTestConfig(false, false), makeSuccessfulThenSilentDataSourceFactory()); - awaitStartUp(); - - // One pair for the identify's context change, then one for going offline. - resetAll(); - eventProcessor.setOffline(false); - eventProcessor.setInBackground(false); - eventProcessor.setOffline(true); - eventProcessor.setInBackground(false); - replayAll(); - - // Identify, then go offline, which discards the data source the identify waits on. - AwaitableCallback identifyCallback = new AwaitableCallback<>(); - contextDataManager.switchToContext(LDContext.create("other-context"), false, identifyCallback); - connectivityManager.setForceOffline(true); - - // The identify completes, because no data source will ever report for it. - identifyCallback.await(5000); - } - - @Test - public void identifyCallbackIsInvokedWhenASecondIdentifySupersedesIt() throws Exception { - // One pair per context change: startup, then each of the two identifies. - eventProcessor.setOffline(false); - eventProcessor.setInBackground(false); - eventProcessor.setOffline(false); - eventProcessor.setInBackground(false); - eventProcessor.setOffline(false); - eventProcessor.setInBackground(false); - replayAll(); - - createTestManager(defaultTestConfig(false, false), makeSuccessfulThenSilentDataSourceFactory()); - awaitStartUp(); - - // A second identify discards the data source the first one waits on. - AwaitableCallback firstIdentify = new AwaitableCallback<>(); - contextDataManager.switchToContext(LDContext.create("context-b"), false, firstIdentify); - contextDataManager.switchToContext(LDContext.create("context-c"), false, new AwaitableCallback<>()); - - // The superseded identify completes rather than waiting forever. - firstIdentify.await(5000); - } - - @Test - public void identifyCallbackIsInvokedWhenTheClientIsClosedDuringIdentify() throws Exception { - // One pair per context change: startup, then the identify. - eventProcessor.setOffline(false); - eventProcessor.setInBackground(false); - eventProcessor.setOffline(false); - eventProcessor.setInBackground(false); - replayAll(); - - createTestManager(defaultTestConfig(false, false), makeSuccessfulThenSilentDataSourceFactory()); - awaitStartUp(); - - // Close while the identify's request is still outstanding. - AwaitableCallback identifyCallback = new AwaitableCallback<>(); - contextDataManager.switchToContext(LDContext.create("other-context"), false, identifyCallback); - connectivityManager.shutDown(); - - // The identify completes, because the data source is gone and will never report. - identifyCallback.await(5000); - } - private ComponentConfigurer makeSuccessfulDataSourceFactory() { return clientContext -> makeSuccessfulDataSource(clientContext); } From 4b9d0522b147446f4fc2e675e19baeb111bee19a Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Tue, 13 Oct 2026 10:30:43 -0700 Subject: [PATCH 12/14] Revert "fix: Always complete the startup callback when its data source goes away" This reverts commit 2f3fdae54d13581736f3f258b0e56240571915d3. --- .../sdk/android/ConnectivityManager.java | 47 +---- .../sdk/android/ConnectivityManagerTest.java | 174 ------------------ 2 files changed, 9 insertions(+), 212 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java index c24fe920e..cbeaf9fb2 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java @@ -82,8 +82,7 @@ class ConnectivityManager implements ContextDataManager.ContextSwitchListener { private final AtomicReference currentContext = new AtomicReference<>(); private volatile ContextDataManager.ContextDataManagerView currentView; - @Nullable private volatile Callback pendingStartupCallback; - private final PendingStartupCallbackHandler pendingStartupCallbackHandler = new PendingStartupCallbackHandler(); + @Nullable private volatile Callback pendingStartUpCallback; // The DataSourceUpdateSinkImpl receives flag updates and status updates from the DataSource. // Data operations (init, upsert, apply) are routed through a ContextDataManager.ContextDataManagerView @@ -144,19 +143,6 @@ public void shutDown() { } } - private class PendingStartupCallbackHandler implements Callback { - @Override - public void onSuccess(Void result) { - clearPendingStartupCallback(result); - } - - @Override - public void onError(Throwable error) { - // updateDataSource never actually calls onError, but if it did, this would be right. - clearPendingStartupCallback(error); - } - } - ConnectivityManager(@NonNull final ClientContext clientContext, @NonNull final ComponentConfigurer dataSourceFactory, @NonNull final EventProcessor eventProcessor, @@ -220,22 +206,6 @@ public void onError(Throwable error) { platformState.addForegroundChangeListener(foregroundListener); } - private synchronized void clearPendingStartupCallback(Void result) { - if (pendingStartupCallback == null) { - return; - } - pendingStartupCallback.onSuccess(result); - pendingStartupCallback = null; - } - - private synchronized void clearPendingStartupCallback(Throwable error) { - if (pendingStartupCallback == null) { - return; - } - pendingStartupCallback.onError(error); - pendingStartupCallback = null; - } - @Override public synchronized void onContextChanged( @NonNull LDContext context, @@ -263,7 +233,13 @@ public synchronized void onContextChanged( platformState.isForeground() ); - Callback effectiveCallback = LDUtil.compositeCallback(Arrays.asList(pendingStartupCallbackHandler, onCompletion)); + Callback effectiveCallback; + if (pendingStartUpCallback != null) { + effectiveCallback = LDUtil.compositeCallback(Arrays.asList(pendingStartUpCallback, onCompletion)); + pendingStartUpCallback = null; + } else { + effectiveCallback = onCompletion; + } ModeState state = snapshotModeState(); updateEventProcessor(forcedOffline.get(), state.isNetworkAvailable(), state.isForeground()); @@ -361,7 +337,6 @@ private boolean updateDataSource( DataSource oldDataSource = currentDataSource.getAndSet(null); if (oldDataSource != null) { logger.debug("Stopping current data source"); - clearPendingStartupCallback((Void) null); oldDataSource.stop(LDUtil.noOpCallback()); } } @@ -592,14 +567,11 @@ synchronized boolean startUp( @NonNull Callback onCompletion ) { if (closed.get() || started.get()) { - // LDClient doesn't call startUp if the class has already been started, and treats it - // as a success. Handle it the same way, just in case a refactor makes it possible. - onCompletion.onSuccess(null); return false; } initialized = false; - pendingStartupCallback = onCompletion; + pendingStartUpCallback = onCompletion; started.set(true); // CDM immediately calls onContextChanged(...) after registration and that will @@ -614,7 +586,6 @@ synchronized boolean startUp( * is being closed, or if we receive an error that indicates the mobile key is invalid. */ void shutDown() { - clearPendingStartupCallback((Void) null); if (closed.getAndSet(true)) { return; } diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java index dbd794898..26cc2595e 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/ConnectivityManagerTest.java @@ -58,7 +58,6 @@ import java.util.concurrent.BlockingQueue; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; @@ -870,179 +869,6 @@ public void startupCallbackIsInvokedOnCompletion() throws ExecutionException { callback.await(); } - /** - * A data source that accepts its start callback and never reports a result, so only something - * other than the data source itself can complete startup. - */ - private ComponentConfigurer makeSilentDataSourceFactory( - BlockingQueue> startCallbacks - ) { - return clientContext -> { - receivedClientContexts.add(clientContext); - return new DataSource() { - @Override - public void start(@NonNull Callback resultCallback) { - startCallbacks.add(resultCallback); - } - - @Override - public void stop(@NonNull Callback completionCallback) { - completionCallback.onSuccess(null); - } - - @Override - public boolean needsRefresh(boolean newInBackground, LDContext newEvaluationContext) { - return true; - } - }; - }; - } - - @Test - public void startupCallbackIsInvokedWhenAStateChangeReplacesTheFirstDataSource() throws Exception { - eventProcessor.setOffline(false); - eventProcessor.setInBackground(false); - replayAll(); - - BlockingQueue> startCallbacks = new LinkedBlockingQueue<>(); - createTestManager(defaultTestConfig(false, false), makeSilentDataSourceFactory(startCallbacks)); - - AwaitableCallback startupCallback = new AwaitableCallback<>(); - connectivityManager.startUp(contextDataManager, startupCallback); - requireValue(startCallbacks, 1, TimeUnit.SECONDS, "data source start"); - - resetAll(); - eventProcessor.setOffline(false); - eventProcessor.setInBackground(true); - eventProcessor.flush(); - replayAll(); - - // Moving to the background replaces the data source before it ever reports. - mockPlatformState.setAndNotifyForegroundChangeListeners(false); - - // Startup completes, because no data source will ever report for it. - startupCallback.await(5000); - } - - @Test - public void startupCallbackIsInvokedWhenTheClientIsClosedDuringStartup() throws Exception { - eventProcessor.setOffline(false); - eventProcessor.setInBackground(false); - replayAll(); - - BlockingQueue> startCallbacks = new LinkedBlockingQueue<>(); - createTestManager(defaultTestConfig(false, false), makeSilentDataSourceFactory(startCallbacks)); - - AwaitableCallback startupCallback = new AwaitableCallback<>(); - connectivityManager.startUp(contextDataManager, startupCallback); - requireValue(startCallbacks, 1, TimeUnit.SECONDS, "data source start"); - - // Close while the first request is still outstanding. - connectivityManager.shutDown(); - - // Startup completes, because the data source is gone and will never report. - startupCallback.await(5000); - } - - @Test - public void startupCallbackIsInvokedWhenTheManagerHasAlreadyStarted() throws Exception { - eventProcessor.setOffline(false); - eventProcessor.setInBackground(false); - replayAll(); - - createTestManager(defaultTestConfig(false, false), makeSuccessfulDataSourceFactory()); - awaitStartUp(); - - // A second startUp is refused, but its caller is still waiting on a callback. - AwaitableCallback secondStartupCallback = new AwaitableCallback<>(); - connectivityManager.startUp(contextDataManager, secondStartupCallback); - - secondStartupCallback.await(5000); - } - - @Test - public void startupCallbackIsInvokedWhenTheManagerHasAlreadyBeenClosed() throws Exception { - eventProcessor.setOffline(false); - eventProcessor.setInBackground(false); - replayAll(); - - createTestManager(defaultTestConfig(false, false), makeSuccessfulDataSourceFactory()); - connectivityManager.shutDown(); - - // Starting a closed manager is refused, but its caller is still waiting on a callback. - AwaitableCallback startupCallback = new AwaitableCallback<>(); - connectivityManager.startUp(contextDataManager, startupCallback); - - startupCallback.await(5000); - } - - @Test - public void startupCallbackIsInvokedOnlyOnceWhenTheDataSourceReportsRepeatedly() { - eventProcessor.setOffline(false); - eventProcessor.setInBackground(false); - replayAll(); - - // A polling data source reports the result of every poll on the same callback. - ComponentConfigurer repeatedlyReportingDataSource = clientContext -> { - receivedClientContexts.add(clientContext); - return new DataSource() { - @Override - public void start(@NonNull Callback resultCallback) { - clientContext.getDataSourceUpdateSink() - .setStatus(ConnectionMode.POLLING, null); - resultCallback.onSuccess(true); - resultCallback.onSuccess(true); - resultCallback.onSuccess(true); - } - - @Override - public void stop(@NonNull Callback completionCallback) { - completionCallback.onSuccess(null); - } - - @Override - public boolean needsRefresh(boolean newInBackground, LDContext newEvaluationContext) { - return true; - } - }; - }; - createTestManager(defaultTestConfig(false, false), repeatedlyReportingDataSource); - - AtomicInteger completions = new AtomicInteger(); - connectivityManager.startUp(contextDataManager, new Callback() { - @Override - public void onSuccess(Void result) { - completions.incrementAndGet(); - } - - @Override - public void onError(Throwable e) { - } - }); - - // LDClient counts these to decide when init is done, so an extra call resolves init early. - assertEquals(1, completions.get()); - } - - @Test - public void identifyCallbackIsInvokedAfterStartup() throws Exception { - // One pair per context change: startup, then the switch below. - eventProcessor.setOffline(false); - eventProcessor.setInBackground(false); - eventProcessor.setOffline(false); - eventProcessor.setInBackground(false); - replayAll(); - - createTestManager(defaultTestConfig(false, false), makeSuccessfulDataSourceFactory()); - awaitStartUp(); - - // A context switch after startup runs the identify path rather than the startup path. - AwaitableCallback identifyCallback = new AwaitableCallback<>(); - contextDataManager.switchToContext(LDContext.create("other-context"), false, identifyCallback); - - identifyCallback.await(5000); - } - private ComponentConfigurer makeSuccessfulDataSourceFactory() { return clientContext -> makeSuccessfulDataSource(clientContext); } From 1694184aa1db6d335baec3e046d8ff1e00b3cdaa Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Tue, 13 Oct 2026 10:38:56 -0700 Subject: [PATCH 13/14] chore: Revert the post-stop poll guards, which belong with the ConnectivityManager work --- .../sdk/android/PollingDataSource.java | 6 --- .../sdk/android/PollingDataSourceTest.java | 40 ------------------- 2 files changed, 46 deletions(-) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java index 48e79cd86..0c7f2a3a6 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java @@ -107,9 +107,6 @@ private void poll(Callback resultCallback) { new Callback() { @Override public void onSuccess(Boolean result) { - if (stopped) { - return; - } try { retryState.recordSuccess(); resultCallback.onSuccess(result); @@ -120,9 +117,6 @@ public void onSuccess(Boolean result) { @Override public void onError(Throwable e) { - if (stopped) { - return; - } try { retryState.recordFailure(e); resultCallback.onError(e); diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java index 598ff45e1..8ca1996c3 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java @@ -20,8 +20,6 @@ import org.junit.Test; import java.io.IOException; -import java.util.ArrayList; -import java.util.List; import java.util.Map; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; @@ -328,44 +326,6 @@ public void close() {} requireNoMoreValues(fetcher.receivedContexts, 500, TimeUnit.MILLISECONDS); } - @Test - public void fetchResultIsIgnoredWhenStopHappensWhileFetchIsInFlight() throws Exception { - BlockingQueue> heldFetches = new LinkedBlockingQueue<>(); - FeatureFetcher holdingFetcher = new FeatureFetcher() { - @Override - public void fetch(LDContext context, Callback callback) { - heldFetches.add(callback); - } - - @Override - public void close() {} - }; - - PollingDataSource ds = makeDataSource(holdingFetcher, taskExecutor, 50); - List results = new ArrayList<>(); - - // Start a poll and wait until its request is outstanding. - ds.start(new Callback() { - @Override - public void onSuccess(Boolean result) { - results.add("success"); - } - - @Override - public void onError(Throwable e) { - results.add("error"); - } - }); - Callback heldFetch = requireValue(heldFetches, 500, TimeUnit.MILLISECONDS); - - // Stop the data source, then let the outstanding request complete. - ds.stop(LDUtil.noOpCallback()); - heldFetch.onSuccess("{}"); - - // A result that arrives after stop does not reach the consumer. - assertEquals(0, results.size()); - } - @Test public void pollingContinuesWhenResultCallbackThrowsOnSuccess() throws Exception { PollingDataSource ds = makeDataSource(fetcher, taskExecutor, 50); From ca35aff238f39d3e08148609458602f03930de41 Mon Sep 17 00:00:00 2001 From: Bee Klimt Date: Wed, 14 Oct 2026 16:22:19 -0700 Subject: [PATCH 14/14] fix: Do not schedule a poll after the task executor closes --- .../sdk/android/PollingDataSource.java | 3 +++ .../sdk/android/PollingDataSourceTest.java | 25 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java index 0c7f2a3a6..68e07663c 100644 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java +++ b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/PollingDataSource.java @@ -88,6 +88,9 @@ public void stop(Callback completionCallback) { } private void schedulePoll(Callback resultCallback, long delayMillis) { + if (stopped) { + return; + } currentPollTask.set(taskExecutor.scheduleTask(() -> { if (stopped) { return; diff --git a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java index 8ca1996c3..4fd7e7339 100644 --- a/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java +++ b/launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/PollingDataSourceTest.java @@ -294,6 +294,31 @@ private PollingDataSource makeDataSource(FeatureFetcher fetcherToUse, TaskExecut ); } + @Test + public void resultArrivingAfterTheExecutorShutsDownDoesNotScheduleAnotherPoll() throws Exception { + BlockingQueue> heldFetches = new LinkedBlockingQueue<>(); + FeatureFetcher holdingFetcher = new FeatureFetcher() { + @Override + public void fetch(LDContext context, Callback callback) { + heldFetches.add(callback); + } + + @Override + public void close() {} + }; + + PollingDataSource ds = makeDataSource(holdingFetcher, taskExecutor, 50); + + // Start a poll, then shut everything down while its request is still outstanding. + ds.start(LDUtil.noOpCallback()); + Callback heldFetch = requireValue(heldFetches, 500, TimeUnit.MILLISECONDS); + ds.stop(LDUtil.noOpCallback()); + taskExecutor.close(); + + // Scheduling onto a closed executor throws, so the failed request must not reschedule. + heldFetch.onError(new LDFailure("network error", LDFailure.FailureType.NETWORK_FAILURE)); + } + @Test public void pollArmedBeforeStopDoesNotFetchWhenItFires() throws Exception { // Schedules tasks for real, but returns no handle, so stop has nothing to cancel.