diff --git a/.ldrelease/config.yml b/.ldrelease/config.yml index 3357c5c8..8c47abb1 100644 --- a/.ldrelease/config.yml +++ b/.ldrelease/config.yml @@ -18,6 +18,7 @@ jobs: branches: - name: main + - name: 4.x - name: 3.x documentation: diff --git a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientTest.java b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientTest.java index 38c57510..59272e12 100644 --- a/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientTest.java +++ b/launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientTest.java @@ -16,6 +16,7 @@ import java.io.IOException; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; import java.util.concurrent.Future; import static org.junit.Assert.assertEquals; @@ -153,7 +154,7 @@ public void testDoubleClose() throws IOException { @Test public void testInitBackgroundThread() throws Exception { - Future backgroundComplete = new BackgroundThreadExecutor().newFixedThreadPool(1).submit(() -> { + Future backgroundComplete = Executors.newSingleThreadExecutor().submit(() -> { try { try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(), ldUser).get()) { assertTrue(ldClient.isInitialized()); diff --git a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/BackgroundThreadExecutor.java b/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/BackgroundThreadExecutor.java deleted file mode 100644 index a05230a5..00000000 --- a/launchdarkly-android-client-sdk/src/main/java/com/launchdarkly/sdk/android/BackgroundThreadExecutor.java +++ /dev/null @@ -1,52 +0,0 @@ -package com.launchdarkly.sdk.android; - -import android.os.Process; - -import androidx.annotation.NonNull; - -import java.util.concurrent.ExecutorService; -import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.ThreadFactory; -import java.util.concurrent.ThreadPoolExecutor; -import java.util.concurrent.TimeUnit; - -/** - * Executes all threads with priority android.os.Process.THREAD_PRIORITY_BACKGROUND. - */ -class BackgroundThreadExecutor { - - private final ThreadFactory threadFactory; - - BackgroundThreadExecutor() { - this.threadFactory = new PriorityThreadFactory(Process.THREAD_PRIORITY_BACKGROUND); - } - - @SuppressWarnings("SameParameterValue") - ExecutorService newFixedThreadPool(int nThreads) { - return new ThreadPoolExecutor(nThreads, nThreads, 0L, TimeUnit.MILLISECONDS, - new LinkedBlockingQueue(), threadFactory); - } - - private static class PriorityThreadFactory implements ThreadFactory { - - private final int threadPriority; - - PriorityThreadFactory(int threadPriority) { - this.threadPriority = threadPriority; - } - - @Override - public Thread newThread(@NonNull final Runnable runnable) { - return new Thread(() -> { - try { - Process.setThreadPriority(threadPriority); - } catch (Throwable ignored) { - // ignore - } - runnable.run(); - }); - } - - } - -} 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 96b7dcf2..a429e8df 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 @@ -22,7 +22,6 @@ import com.launchdarkly.sdk.json.SerializationException; import java.net.URI; -import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; import okhttp3.OkHttpClient; @@ -63,7 +62,6 @@ final class StreamingDataSource implements DataSource { private final boolean streamEvenInBackground; private volatile boolean running = false; private boolean connection401Error = false; - private final ExecutorService executor; private final DiagnosticStore diagnosticStore; private long eventSourceStarted; private final LDLogger logger; @@ -87,7 +85,6 @@ final class StreamingDataSource implements DataSource { this.streamEvenInBackground = streamEvenInBackground; this.diagnosticStore = ClientContextImpl.get(clientContext).getDiagnosticStore(); this.logger = clientContext.getBaseLogger(); - executor = new BackgroundThreadExecutor().newFixedThreadPool(2); } public void start(@NonNull Callback resultCallback) { @@ -241,12 +238,22 @@ public void stop(final @NonNull Callback onCompleteListener) { logger.debug("Stopping."); // We do this in a separate thread because closing the stream involves a network // operation and we don't want to do a network operation on the main thread. - executor.execute(() -> { + // This code originally created a one-shot thread for shutting down the event source, but at some point + // an Executor was introduced. A thread leak bug was introduced with that Executor because the Executor + // was not cleaned up. The thread leak bug was brought to our attention in + // https://github.com/launchdarkly/android-client-sdk/issues/234 . Over time, the code evolved to no longer + // need the Executor to be long lived. Reverting to the one-shot thread approach is sufficient to address + // the bug. A more appropriate fix would be to refactor/unify the various task executors in the code base + // and pass one of those executors in to be used for this purpose. That refactoring is not without risk and + // will be reserved for a future major version. + new Thread(() -> { + // Moves the current Thread into the background. + android.os.Process.setThreadPriority(android.os.Process.THREAD_PRIORITY_BACKGROUND); stopSync(); if (onCompleteListener != null) { onCompleteListener.onSuccess(null); } - }); + }).start(); } @Override