Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prepare 4.3.1 release #240

Merged
merged 11 commits into from
Feb 29, 2024
1 change: 1 addition & 0 deletions .ldrelease/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ jobs:

branches:
- name: main
- name: 4.x
- name: 3.x

documentation:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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<Boolean> resultCallback) {
Expand Down Expand Up @@ -241,12 +238,22 @@ public void stop(final @NonNull Callback<Void> 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
Expand Down