chore: clearing selector and fixing fdv2 race conditions by tanderson-ld · Pull Request #347 · launchdarkly/android-client-sdk

tanderson-ld · 2026-04-15T15:34:25Z

Requirements

I have added test coverage for new or changed functionality
I have followed the repository's pull request submission guidelines
I have validated my changes against all supported platform versions

Related issues

SDK-2072

Describe the solution you've provided

As I was working on the clearing selector story, I noticed we had a race condition that is reachable in FDv2. This race condition did not manifest itself in FDv1 due to version checking. This version checking does not exist in FDv2.

The race condition is that a data source that is in the middle of shutting down may still write data onto the context in the case of rapid switch from Context A -> Context B -> Context A. We thought the context equality checks were sufficient, but as you can see, Context A (first) would equal Context A (last).

The solution to this is to make it such that the ContextDataManager hands out ContextDataManagerViews to the ConnectivityManager along with the context that the ConnectivityManager will use. When data arrives, that data flows through the ContextDataManagerView.

When a context change occurs, the ContextDataManagerView is atomically invalidated before the next context is active, ensuring that no previous datasource can write data.

Note

Medium Risk
Touches core context-switching and data-source lifecycle wiring; mistakes could cause missed updates or identify/startup callbacks not firing, especially under rapid context changes in FDv2.

Overview
Prevents FDv2 race conditions during rapid context switches by introducing ContextDataManager.ContextDataManagerView objects that are invalidated on every context change, causing old data sources to silently drop writes and return an empty selector.

Refactors ConnectivityManager to register as a ContextSwitchListener, rebuild data sources in onContextChanged, and route all data updates through the current view while passing that view through ClientContextImpl as a SelectorSource (replacing TransactionalDataStore/SelectorSourceFacade, which is removed). Startup/identify flows are updated to use the new callback-composition and switchToContext(..., onCompletion) semantics, with expanded test coverage for view invalidation and callback behavior.

^{Reviewed by Cursor Bugbot for commit e6caba2. Bugbot is set up for automated code reviews on this repo. Configure here.}

tanderson-ld · 2026-04-15T15:37:02Z

+    private final SelectorSource selectorSource;

-    /** Used by FDv1 code paths that do not need a {@link TransactionalDataStore}. */
+    /** Used by FDv1 code paths that do not need a {@link SelectorSource}. */


For reviewers: The transactional data store was more than strictly necessary, so tidying that up.

tanderson-ld · 2026-04-15T15:38:19Z

    private final ConnectionInformationState connectionInformation;
    private final PersistentDataStoreWrapper.PerEnvironmentData environmentStore;
    private final EventProcessor eventProcessor;
-    private final TransactionalDataStore transactionalDataStore;


For reviewers: Writing data is now done through the ContextDataManager.ContextDataManagerView

tanderson-ld · 2026-04-15T15:39:38Z

        this.taskExecutor = ClientContextImpl.get(clientContext).getTaskExecutor();
        this.logger = clientContext.getBaseLogger();

-        currentContext.set(clientContext.getEvaluationContext());


For reviewers: The context is now given to the ConnectivityManager via onContextChanged which fires during startup to set the initial context.

tanderson-ld · 2026-04-15T15:41:08Z

+        this.currentContext.set(context);
+        this.currentView = view;

-        if (oldContext == context || oldContext.equals(context)) {


For reviewers: A nice consequence of these changes is that the ConnectivityManager gets dumber. It respects whatever context the ContextDataManager sends via onContextChanged and does not do any checks against it. "Oh, you want a data source for that context, coming right up!"

tanderson-ld · 2026-04-15T15:42:11Z

    }

-    private synchronized boolean updateDataSource(
+    private boolean updateDataSource(


For reviewers: This synchronization was unnecessary. Sync on private methods is usually a smell.

tanderson-ld · 2026-04-15T15:43:09Z

+            updateConnectionInfoForSuccess(ConnectionInformation.ConnectionMode.OFFLINE);
        } else if (!newState.isForeground() && newState.isBackgroundUpdatingDisabled()) {
-            dataSourceUpdateSink.setStatus(ConnectionInformation.ConnectionMode.BACKGROUND_DISABLED, null);
+            updateConnectionInfoForSuccess(ConnectionInformation.ConnectionMode.BACKGROUND_DISABLED);


For reviewers: Instead of going through sink impl and then back to a private method of this class, just go straight to the private method in this class.

tanderson-ld · 2026-04-15T15:49:19Z


-        ModeState state = snapshotModeState();
-        updateEventProcessor(forcedOffline.get(), state.isNetworkAvailable(), state.isForeground());
-        return updateDataSource(true, state, onCompletion);


For reviewers: This logic moved to onContextChanged.

tanderson-ld · 2026-04-15T15:51:45Z

-     *
-     * @param context to swtich to
-     * @param onCompletion callback that indicates when the switching is done
-     */


For reviewers: switchToContext no longer exists, instead the connectivity manager listens to synchronous context changes from the ContextDataManager. This allows the context data manager to have confidence views have been invalidated and disseminated in an atomic way.

Wow! This is a cool change that I wouldn't have thought of 👍

tanderson-ld · 2026-04-20T15:38:56Z

For reviewers: Not required anymore

aaron-zeisler · 2026-04-20T17:33:23Z

     * @param newData the new flag data
     */
-    public void initData(
+    @VisibleForTesting


Nice, I wasn't aware of this annotation. Is the function still technically public?

This is an Android annotation and doesn't have any impact on visibility or behavior at runtime. Just informative. Tools can sometimes pick up on the annotations to provide compile time errors and such.

The visibility actually went down as it is no longer public. Default visibility is package private, and since the tests are compiled as part of the same package, the test can see it.

aaron-zeisler · 2026-04-20T17:42:20Z

+    public void setContextSwitchListener(@NonNull ContextSwitchListener listener) {
+        this.contextSwitchListener = listener;
+        listener.onContextChanged(currentContext, currentView, LDUtil.noOpCallback());
+    }


Should this use the synchronized keyword? Or is it within a lock because it's only called from startUp()?

Good eye. This is intentionally not in a lock and matches switchToContext. At the moment ConnectivityManager is a well behaved listener and does its tasks quickly and so it can be synchronous and inside the lock.

But I wrote this not assuming the listener is well behaved and I didn't want the listeners work to hold the lock.

aaron-zeisler

This looks good 👍 I have one non-blocking question about using the synchronized keyword in setContextSwitchListener().

…and-fixes

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 6fa3f6b. Configure here.}

chore: clearing selector and fixing fdv2 race conditions

48b7edb

tanderson-ld requested a review from a team as a code owner April 15, 2026 15:34

tanderson-ld commented Apr 15, 2026

View reviewed changes

cursor Bot reviewed Apr 15, 2026

View reviewed changes

Comment thread ...darkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java

Comment thread ...darkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java Outdated

tanderson-ld commented Apr 15, 2026

View reviewed changes

adding tests

4e44292

tanderson-ld requested a review from aaron-zeisler April 15, 2026 20:06

tanderson-ld commented Apr 20, 2026

View reviewed changes

aaron-zeisler reviewed Apr 20, 2026

View reviewed changes

aaron-zeisler approved these changes Apr 20, 2026

View reviewed changes

Merge remote-tracking branch 'origin' into ta/SDK-2072/fdv2-identify-…

6fa3f6b

…and-fixes

cursor Bot reviewed Apr 20, 2026

View reviewed changes

Comment thread ...darkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java

aaron-zeisler reviewed Apr 20, 2026

View reviewed changes

Comment thread ...hdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ClientContextImpl.java

aaron-zeisler reviewed Apr 20, 2026

View reviewed changes

Comment thread ...darkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ContextDataManager.java

addressing review comments, fixing skip cache issue

e6caba2

tanderson-ld merged commit 526e56e into main Apr 21, 2026
8 checks passed

tanderson-ld deleted the ta/SDK-2072/fdv2-identify-and-fixes branch April 21, 2026 18:58

Conversation

tanderson-ld commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tanderson-ld Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tanderson-ld Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tanderson-ld Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaron-zeisler left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tanderson-ld commented Apr 15, 2026 •

edited

Loading

tanderson-ld Apr 15, 2026 •

edited

Loading

tanderson-ld Apr 15, 2026 •

edited

Loading

tanderson-ld Apr 15, 2026 •

edited

Loading