[SDK-1956] Address Bugbot findings in FDv2 code

- Extract DataSourceSetup helper and makePollingRequestor() to eliminate
  duplicated configurer boilerplate in FDv2DataSourceBuilder
- Share builder's sharedExecutor across all components to prevent thread
  pool leaks on mode switches; increase pool size from 2 to 4
- Make FDv2DataSourceBuilder implement Closeable; shut down executor in
  ConnectivityManager.shutDown()
- Honor backgroundUpdatingDisabled in FDv2 mode resolution by adding the
  flag to ModeState and ModeResolutionTable

Made-with: Cursor

Author: aaron-zeisler

launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ConnectivityManager.java

@@ -16,6 +16,8 @@
 import com.launchdarkly.sdk.android.subsystems.EventProcessor;
 import com.launchdarkly.sdk.android.subsystems.TransactionalDataStore;
 
+import java.io.Closeable;
+import java.io.IOException;
 import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.Iterator;
@@ -233,6 +235,11 @@ private synchronized boolean updateDataSource(
         if (useFDv2ModeResolution) {
             // FDv2 mode resolution already accounts for offline/background states via
             // the ModeResolutionTable, so we always rebuild when the mode changed.
+            // Note: unlike FDv1's forceOffline/noNetwork branches above, initialized=true
+            // is not set here eagerly — it is set in the dataSource.start() callback below.
+            // For OFFLINE mode this creates a brief async gap (one executor task) before
+            // isInitialized() returns true, but the OFFLINE data source fires its callback
+            // nearly instantaneously since it has no initializers or synchronizers.
             shouldStopExistingDataSource = mustReinitializeDataSource;
             shouldStartDataSourceIfStopped = true;
         } else if (forceOffline) {
@@ -457,6 +464,12 @@ void shutDown() {
         if (oldDataSource != null) {
             oldDataSource.stop(LDUtil.noOpCallback());
         }
+        if (dataSourceFactory instanceof Closeable) {
+            try {
+                ((Closeable) dataSourceFactory).close();
+            } catch (IOException ignored) {
+            }
+        }
         platformState.removeForegroundChangeListener(foregroundListener);
         platformState.removeConnectivityChangeListener(connectivityChangeListener);
     }
@@ -502,7 +515,8 @@ private ConnectionMode resolveMode() {
         }
         ModeState state = new ModeState(
                 platformState.isForeground(),
-                platformState.isNetworkAvailable()
+                platformState.isNetworkAvailable(),
+                backgroundUpdatingDisabled
         );
         return ModeResolutionTable.MOBILE.resolve(state);
     }

launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/FDv2DataSourceBuilder.java

@@ -14,6 +14,7 @@
 import com.launchdarkly.sdk.android.subsystems.TransactionalDataStore;
 import com.launchdarkly.sdk.internal.http.HttpProperties;
 
+import java.io.Closeable;
 import java.net.URI;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -32,113 +33,60 @@
  * <p>
  * Package-private — not part of the public SDK API.
  */
-class FDv2DataSourceBuilder implements ComponentConfigurer<DataSource> {
+class FDv2DataSourceBuilder implements ComponentConfigurer<DataSource>, Closeable {
 
-    private final Map<ConnectionMode, ModeDefinition> modeTable;
+    private Map<ConnectionMode, ModeDefinition> modeTable;
     private final ConnectionMode startingMode;
 
     private ConnectionMode activeMode;
     private boolean includeInitializers = true; // false during mode switches to skip initializers (CONNMODE 2.0.1)
     private ScheduledExecutorService sharedExecutor;
 
     FDv2DataSourceBuilder() {
-        this(makeDefaultModeTable(), ConnectionMode.STREAMING);
+        this.modeTable = null; // built lazily in build() so lambdas can capture sharedExecutor
+        this.startingMode = ConnectionMode.STREAMING;
     }
 
-    private static Map<ConnectionMode, ModeDefinition> makeDefaultModeTable() {
+    private Map<ConnectionMode, ModeDefinition> makeDefaultModeTable() {
         ComponentConfigurer<Initializer> pollingInitializer = ctx -> {
-            ClientContextImpl impl = ClientContextImpl.get(ctx);
-            TransactionalDataStore store = impl.getTransactionalDataStore();
-            SelectorSource selectorSource = store != null
-                    ? new SelectorSourceFacade(store)
-                    : () -> com.launchdarkly.sdk.fdv2.Selector.EMPTY;
-            URI pollingBase = StandardEndpoints.selectBaseUri(
-                    ctx.getServiceEndpoints().getPollingBaseUri(),
-                    StandardEndpoints.DEFAULT_POLLING_BASE_URI,
-                    "polling", ctx.getBaseLogger());
-            HttpProperties httpProps = LDUtil.makeHttpProperties(ctx);
-            FDv2Requestor requestor = new DefaultFDv2Requestor(
-                    ctx.getEvaluationContext(), pollingBase,
-                    StandardEndpoints.FDV2_POLLING_REQUEST_GET_BASE_PATH,
-                    StandardEndpoints.FDV2_POLLING_REQUEST_REPORT_BASE_PATH,
-                    httpProps, ctx.getHttp().isUseReport(),
-                    ctx.isEvaluationReasons(), null, ctx.getBaseLogger());
-            return new FDv2PollingInitializer(requestor, selectorSource,
-                    Executors.newSingleThreadExecutor(), ctx.getBaseLogger());
+            DataSourceSetup s = new DataSourceSetup(ctx);
+            FDv2Requestor requestor = makePollingRequestor(ctx, s.httpProps);
+            return new FDv2PollingInitializer(requestor, s.selectorSource,
+                    sharedExecutor, ctx.getBaseLogger());
         };
 
         ComponentConfigurer<Synchronizer> pollingSynchronizer = ctx -> {
-            ClientContextImpl impl = ClientContextImpl.get(ctx);
-            TransactionalDataStore store = impl.getTransactionalDataStore();
-            SelectorSource selectorSource = store != null
-                    ? new SelectorSourceFacade(store)
-                    : () -> com.launchdarkly.sdk.fdv2.Selector.EMPTY;
-            URI pollingBase = StandardEndpoints.selectBaseUri(
-                    ctx.getServiceEndpoints().getPollingBaseUri(),
-                    StandardEndpoints.DEFAULT_POLLING_BASE_URI,
-                    "polling", ctx.getBaseLogger());
-            HttpProperties httpProps = LDUtil.makeHttpProperties(ctx);
-            FDv2Requestor requestor = new DefaultFDv2Requestor(
-                    ctx.getEvaluationContext(), pollingBase,
-                    StandardEndpoints.FDV2_POLLING_REQUEST_GET_BASE_PATH,
-                    StandardEndpoints.FDV2_POLLING_REQUEST_REPORT_BASE_PATH,
-                    httpProps, ctx.getHttp().isUseReport(),
-                    ctx.isEvaluationReasons(), null, ctx.getBaseLogger());
-            ScheduledExecutorService exec = Executors.newScheduledThreadPool(1);
-            return new FDv2PollingSynchronizer(requestor, selectorSource, exec,
+            DataSourceSetup s = new DataSourceSetup(ctx);
+            FDv2Requestor requestor = makePollingRequestor(ctx, s.httpProps);
+            return new FDv2PollingSynchronizer(requestor, s.selectorSource,
+                    sharedExecutor,
                     0, PollingDataSourceBuilder.DEFAULT_POLL_INTERVAL_MILLIS, ctx.getBaseLogger());
         };
 
         ComponentConfigurer<Synchronizer> streamingSynchronizer = ctx -> {
-            ClientContextImpl impl = ClientContextImpl.get(ctx);
-            TransactionalDataStore store = impl.getTransactionalDataStore();
-            SelectorSource selectorSource = store != null
-                    ? new SelectorSourceFacade(store)
-                    : () -> com.launchdarkly.sdk.fdv2.Selector.EMPTY;
+            DataSourceSetup s = new DataSourceSetup(ctx);
+            // The streaming synchronizer uses a polling requestor for its internal
+            // polling fallback (e.g. when the stream cannot be established).
+            FDv2Requestor pollingRequestor = makePollingRequestor(ctx, s.httpProps);
             URI streamBase = StandardEndpoints.selectBaseUri(
                     ctx.getServiceEndpoints().getStreamingBaseUri(),
                     StandardEndpoints.DEFAULT_STREAMING_BASE_URI,
                     "streaming", ctx.getBaseLogger());
-            URI pollingBase = StandardEndpoints.selectBaseUri(
-                    ctx.getServiceEndpoints().getPollingBaseUri(),
-                    StandardEndpoints.DEFAULT_POLLING_BASE_URI,
-                    "polling", ctx.getBaseLogger());
-            HttpProperties httpProps = LDUtil.makeHttpProperties(ctx);
-            FDv2Requestor requestor = new DefaultFDv2Requestor(
-                    ctx.getEvaluationContext(), pollingBase,
-                    StandardEndpoints.FDV2_POLLING_REQUEST_GET_BASE_PATH,
-                    StandardEndpoints.FDV2_POLLING_REQUEST_REPORT_BASE_PATH,
-                    httpProps, ctx.getHttp().isUseReport(),
-                    ctx.isEvaluationReasons(), null, ctx.getBaseLogger());
             return new FDv2StreamingSynchronizer(
-                    ctx.getEvaluationContext(), selectorSource, streamBase,
+                    ctx.getEvaluationContext(), s.selectorSource, streamBase,
                     StandardEndpoints.FDV2_STREAMING_REQUEST_BASE_PATH,
-                    requestor,
+                    pollingRequestor,
                     StreamingDataSourceBuilder.DEFAULT_INITIAL_RECONNECT_DELAY_MILLIS,
                     ctx.isEvaluationReasons(), ctx.getHttp().isUseReport(),
-                    httpProps, Executors.newSingleThreadExecutor(),
+                    s.httpProps, sharedExecutor,
                     ctx.getBaseLogger(), null);
         };
 
         ComponentConfigurer<Synchronizer> backgroundPollingSynchronizer = ctx -> {
-            ClientContextImpl impl = ClientContextImpl.get(ctx);
-            TransactionalDataStore store = impl.getTransactionalDataStore();
-            SelectorSource selectorSource = store != null
-                    ? new SelectorSourceFacade(store)
-                    : () -> com.launchdarkly.sdk.fdv2.Selector.EMPTY;
-            URI pollingBase = StandardEndpoints.selectBaseUri(
-                    ctx.getServiceEndpoints().getPollingBaseUri(),
-                    StandardEndpoints.DEFAULT_POLLING_BASE_URI,
-                    "polling", ctx.getBaseLogger());
-            HttpProperties httpProps = LDUtil.makeHttpProperties(ctx);
-            FDv2Requestor requestor = new DefaultFDv2Requestor(
-                    ctx.getEvaluationContext(), pollingBase,
-                    StandardEndpoints.FDV2_POLLING_REQUEST_GET_BASE_PATH,
-                    StandardEndpoints.FDV2_POLLING_REQUEST_REPORT_BASE_PATH,
-                    httpProps, ctx.getHttp().isUseReport(),
-                    ctx.isEvaluationReasons(), null, ctx.getBaseLogger());
-            ScheduledExecutorService exec = Executors.newScheduledThreadPool(1);
-            return new FDv2PollingSynchronizer(requestor, selectorSource, exec,
+            DataSourceSetup s = new DataSourceSetup(ctx);
+            FDv2Requestor requestor = makePollingRequestor(ctx, s.httpProps);
+            return new FDv2PollingSynchronizer(requestor, s.selectorSource,
+                    sharedExecutor,
                     0, LDConfig.DEFAULT_BACKGROUND_POLL_INTERVAL_MILLIS, ctx.getBaseLogger());
         };
 
@@ -207,6 +155,18 @@ ModeDefinition getModeDefinition(@NonNull ConnectionMode mode) {
 
     @Override
     public DataSource build(ClientContext clientContext) {
+        if (sharedExecutor == null) {
+            // Pool size 4: supports the FDv2DataSource main loop (1), an active synchronizer
+            // such as the streaming event loop (1), FDv2DataSource condition timers for
+            // fallback/recovery (up to 2 short-lived scheduled tasks). Only one mode is active
+            // at a time (teardown/rebuild), so this pool serves all components.
+            sharedExecutor = Executors.newScheduledThreadPool(4);
+        }
+
+        if (modeTable == null) {
+            modeTable = Collections.unmodifiableMap(makeDefaultModeTable());
+        }
+
         ConnectionMode mode = activeMode != null ? activeMode : startingMode;
 
         ModeDefinition modeDef = modeTable.get(mode);
@@ -223,10 +183,6 @@ public DataSource build(ClientContext clientContext) {
                     "FDv2DataSource requires a DataSourceUpdateSinkV2 implementation");
         }
 
-        if (sharedExecutor == null) {
-            sharedExecutor = Executors.newScheduledThreadPool(2);
-        }
-
         List<FDv2DataSource.DataSourceFactory<Initializer>> initFactories =
                 includeInitializers ? resolved.getInitializerFactories() : Collections.<FDv2DataSource.DataSourceFactory<Initializer>>emptyList();
 
@@ -243,6 +199,14 @@ public DataSource build(ClientContext clientContext) {
         );
     }
 
+    @Override
+    public void close() {
+        if (sharedExecutor != null) {
+            sharedExecutor.shutdownNow();
+            sharedExecutor = null;
+        }
+    }
+
     private static ResolvedModeDefinition resolve(
             ModeDefinition def, ClientContext clientContext
     ) {
@@ -256,4 +220,43 @@ private static ResolvedModeDefinition resolve(
         }
         return new ResolvedModeDefinition(initFactories, syncFactories);
     }
+
+    /**
+     * Holds the shared infrastructure needed by all FDv2 data source components:
+     * a {@link SelectorSource} backed by the {@link TransactionalDataStore} (or an empty
+     * fallback if none is configured), and the {@link HttpProperties} for the current
+     * client configuration. Polling-specific setup (the {@link FDv2Requestor}) is built
+     * separately via {@link #makePollingRequestor}.
+     */
+    private static final class DataSourceSetup {
+        final SelectorSource selectorSource;
+        final HttpProperties httpProps;
+
+        DataSourceSetup(ClientContext ctx) {
+            TransactionalDataStore store = ClientContextImpl.get(ctx).getTransactionalDataStore();
+            this.selectorSource = store != null
+                    ? new SelectorSourceFacade(store)
+                    : () -> com.launchdarkly.sdk.fdv2.Selector.EMPTY;
+            this.httpProps = LDUtil.makeHttpProperties(ctx);
+        }
+    }
+
+    /**
+     * Builds a {@link DefaultFDv2Requestor} configured for polling endpoints. Used
+     * directly by polling components and as the fallback requestor for the streaming
+     * synchronizer (which needs it for internal polling fallback when the stream cannot
+     * be established).
+     */
+    private static FDv2Requestor makePollingRequestor(ClientContext ctx, HttpProperties httpProps) {
+        URI pollingBase = StandardEndpoints.selectBaseUri(
+                ctx.getServiceEndpoints().getPollingBaseUri(),
+                StandardEndpoints.DEFAULT_POLLING_BASE_URI,
+                "polling", ctx.getBaseLogger());
+        return new DefaultFDv2Requestor(
+                ctx.getEvaluationContext(), pollingBase,
+                StandardEndpoints.FDV2_POLLING_REQUEST_GET_BASE_PATH,
+                StandardEndpoints.FDV2_POLLING_REQUEST_REPORT_BASE_PATH,
+                httpProps, ctx.getHttp().isUseReport(),
+                ctx.isEvaluationReasons(), null, ctx.getBaseLogger());
+    }
 }

launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ModeResolutionTable.java

@@ -13,6 +13,7 @@
  * The {@link #MOBILE} constant defines the Android default resolution table:
  * <ol>
  *   <li>No network → {@link ConnectionMode#OFFLINE}</li>
+ *   <li>Background + background updating disabled → {@link ConnectionMode#OFFLINE}</li>
  *   <li>Background → {@link ConnectionMode#BACKGROUND}</li>
  *   <li>Foreground → {@link ConnectionMode#STREAMING}</li>
  * </ol>
@@ -28,6 +29,9 @@ final class ModeResolutionTable {
             new ModeResolutionEntry(
                     state -> !state.isNetworkAvailable(),
                     ConnectionMode.OFFLINE),
+            new ModeResolutionEntry(
+                    state -> !state.isForeground() && state.isBackgroundUpdatingDisabled(),
+                    ConnectionMode.OFFLINE),
             new ModeResolutionEntry(
                     state -> !state.isForeground(),
                     ConnectionMode.BACKGROUND),

launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/ModeState.java

@@ -15,10 +15,12 @@ final class ModeState {
 
     private final boolean foreground;
     private final boolean networkAvailable;
+    private final boolean backgroundUpdatingDisabled;
 
-    ModeState(boolean foreground, boolean networkAvailable) {
+    ModeState(boolean foreground, boolean networkAvailable, boolean backgroundUpdatingDisabled) {
         this.foreground = foreground;
         this.networkAvailable = networkAvailable;
+        this.backgroundUpdatingDisabled = backgroundUpdatingDisabled;
     }
 
     boolean isForeground() {
@@ -28,4 +30,8 @@ boolean isForeground() {
     boolean isNetworkAvailable() {
         return networkAvailable;
     }
+
+    boolean isBackgroundUpdatingDisabled() {
+        return backgroundUpdatingDisabled;
+    }
 }

launchdarkly-android-client-sdk/src/test/java/com/launchdarkly/sdk/android/FDv2DataSourceBuilderTest.java

@@ -1,8 +1,10 @@
 package com.launchdarkly.sdk.android;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -22,6 +24,7 @@
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.Map;
+import java.util.concurrent.ScheduledExecutorService;
 
 public class FDv2DataSourceBuilderTest {
 
@@ -172,6 +175,55 @@ public void getModeDefinition_sameObjectUsedForEquivalenceCheck() {
                 == builder.getModeDefinition(ConnectionMode.POLLING));
     }
 
+    @Test
+    public void close_shutsDownSharedExecutor() {
+        FDv2DataSourceBuilder builder = new FDv2DataSourceBuilder();
+        // build() lazily creates the sharedExecutor
+        builder.build(makeClientContext());
+
+        // Access the executor via reflection to verify shutdown
+        ScheduledExecutorService executor = getSharedExecutor(builder);
+        assertNotNull(executor);
+        assertFalse(executor.isShutdown());
+
+        builder.close();
+        assertTrue(executor.isShutdown());
+    }
+
+    @Test
+    public void build_reusesSharedExecutorAcrossRebuilds() {
+        FDv2DataSourceBuilder builder = new FDv2DataSourceBuilder();
+        builder.build(makeClientContext());
+        ScheduledExecutorService first = getSharedExecutor(builder);
+
+        builder.build(makeClientContext());
+        ScheduledExecutorService second = getSharedExecutor(builder);
+
+        assertSame(first, second);
+        assertFalse(first.isShutdown());
+        builder.close();
+    }
+
+    @Test
+    public void close_isIdempotent() {
+        FDv2DataSourceBuilder builder = new FDv2DataSourceBuilder();
+        builder.build(makeClientContext());
+
+        builder.close();
+        // second close should not throw
+        builder.close();
+    }
+
+    private static ScheduledExecutorService getSharedExecutor(FDv2DataSourceBuilder builder) {
+        try {
+            java.lang.reflect.Field f = FDv2DataSourceBuilder.class.getDeclaredField("sharedExecutor");
+            f.setAccessible(true);
+            return (ScheduledExecutorService) f.get(builder);
+        } catch (Exception e) {
+            throw new RuntimeException(e);
+        }
+    }
+
     @Test
     public void setActiveMode_notInTable_throws() {
         Map<ConnectionMode, ModeDefinition> customTable = new LinkedHashMap<>();