Skip to content

Commit 3065edb

Browse files
LaunchDarklyReleaseBotember-stevenstanderson-ldld-repository-standards[bot]kparkinson-ld
authored
prepare 5.0.4 release (#235)
## [5.0.4] - 2024-02-27 ### Fixed: - Improved thread usage in streaming connections. --------- Co-authored-by: Ember Stevens <[email protected]> Co-authored-by: Ember Stevens <[email protected]> Co-authored-by: Todd Anderson <[email protected]> Co-authored-by: tanderson-ld <[email protected]> Co-authored-by: ld-repository-standards[bot] <113625520+ld-repository-standards[bot]@users.noreply.github.com> Co-authored-by: Kane Parkinson <[email protected]> Co-authored-by: LaunchDarklyReleaseBot <[email protected]> Co-authored-by: Matthew M. Keeler <[email protected]> Co-authored-by: Louis Chan <[email protected]>
1 parent 1f21f40 commit 3065edb

File tree

3 files changed

+14
-58
lines changed

3 files changed

+14
-58
lines changed

launchdarkly-android-client-sdk/src/androidTest/java/com/launchdarkly/sdk/android/LDClientTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import java.io.IOException;
2424
import java.util.concurrent.ExecutionException;
25+
import java.util.concurrent.Executors;
2526
import java.util.concurrent.Future;
2627

2728
@RunWith(AndroidJUnit4.class)
@@ -154,7 +155,7 @@ public void testDoubleClose() throws IOException {
154155

155156
@Test
156157
public void testInitBackgroundThread() throws Exception {
157-
Future<?> backgroundComplete = new BackgroundThreadExecutor().newFixedThreadPool(1).submit(() -> {
158+
Future<?> backgroundComplete = Executors.newSingleThreadExecutor().submit(() -> {
158159
try {
159160
try (LDClient ldClient = LDClient.init(application, makeOfflineConfig(), ldContext).get()) {
160161
assertTrue(ldClient.isInitialized());

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

Lines changed: 0 additions & 52 deletions
This file was deleted.

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import com.launchdarkly.sdk.json.SerializationException;
2323

2424
import java.net.URI;
25-
import java.util.concurrent.ExecutorService;
2625
import java.util.concurrent.TimeUnit;
2726

2827
import okhttp3.OkHttpClient;
@@ -63,7 +62,6 @@ final class StreamingDataSource implements DataSource {
6362
private final boolean streamEvenInBackground;
6463
private volatile boolean running = false;
6564
private boolean connection401Error = false;
66-
private final ExecutorService executor;
6765
private final DiagnosticStore diagnosticStore;
6866
private long eventSourceStarted;
6967
private final LDLogger logger;
@@ -87,7 +85,6 @@ final class StreamingDataSource implements DataSource {
8785
this.streamEvenInBackground = streamEvenInBackground;
8886
this.diagnosticStore = ClientContextImpl.get(clientContext).getDiagnosticStore();
8987
this.logger = clientContext.getBaseLogger();
90-
executor = new BackgroundThreadExecutor().newFixedThreadPool(2);
9188
}
9289

9390
public void start(@NonNull Callback<Boolean> resultCallback) {
@@ -241,12 +238,22 @@ public void stop(final @NonNull Callback<Void> onCompleteListener) {
241238
logger.debug("Stopping.");
242239
// We do this in a separate thread because closing the stream involves a network
243240
// operation and we don't want to do a network operation on the main thread.
244-
executor.execute(() -> {
241+
// This code originally created a one-shot thread for shutting down the event source, but at some point
242+
// an Executor was introduced. A thread leak bug was introduced with that Executor because the Executor
243+
// was not cleaned up. The thread leak bug was brought to our attention in
244+
// https://github.com/launchdarkly/android-client-sdk/issues/234 . Over time, the code evolved to no longer
245+
// need the Executor to be long lived. Reverting to the one-shot thread approach is sufficient to address
246+
// the bug. A more appropriate fix would be to refactor/unify the various task executors in the code base
247+
// and pass one of those executors in to be used for this purpose. That refactoring is not without risk and
248+
// will be reserved for a future major version.
249+
new Thread(() -> {
250+
// Moves the current Thread into the background.
251+
android.os.Process.setThreadPriority(android.os.Process.THREAD_PRIORITY_BACKGROUND);
245252
stopSync();
246253
if (onCompleteListener != null) {
247254
onCompleteListener.onSuccess(null);
248255
}
249-
});
256+
}).start();
250257
}
251258

252259
@Override

0 commit comments

Comments
 (0)