fix(tests): stop streaming data sources in StreamingDataSourceTest

[abelonogov-ld]

What leaked

StreamingDataSourceTest called start(...) on 16 data sources and never stopped a single one. Every StreamingDataSource.start() builds a BackgroundEventSource, which owns two single-thread executors (okhttp-eventsource-stream[...], okhttp-eventsource-events[...]), an OkHttp client, and an open connection to the test's HttpServer. Nothing ever closed them, so they survived for the life of the forked test JVM and were inherited by every test class Gradle ran afterwards in the same JVM.

There was a second, less obvious half to this. Even adding stop() would not have helped, because StreamingDataSource.stop() is a silent no-op in unit tests:

new Thread(() -> {
    android.os.Process.setThreadPriority(android.os.Process.THREAD_PRIORITY_BACKGROUND); // throws
    stopSync();                      // never reached
    if (onCompleteListener != null) { onCompleteListener.onSuccess(null); }  // never reached
}).start();

Local unit tests run against the mockable android.jar, where every method throws. A probe test confirmed it:

setThreadPriority THREW: java.lang.RuntimeException: Method setThreadPriority in
android.os.Process not mocked.

So the shutdown thread died on its first line, stopSync() never ran, the EventSource was never closed, and the completion callback never fired. This also means the SDK's own internal stop(null) on the non-retriable-error path (401/403) was a no-op under unit test.

Evidence

Instrumented the class with a per-test census of Thread.getAllStackTraces(), removed before commit.

Running the class by itself, thread counts grew monotonically across the 40 tests:

	first test	last test
Total JVM threads	10	65
okhttp-eventsource-stream	0	17
okhttp-eventsource-events	0	17

Running the class as part of the full module suite, the JVM went from 94 threads to 146 across this one class — 52 threads added, 34 of them EventSource threads, and all of them still alive when the next test class started.

After the fix (measured at @AfterClass with a settle delay, three runs): total ends at 33–34 instead of 65, okhttp-eventsource-events is 0, and okhttp-eventsource-stream is 4–5 instead of 17. See "Known remaining residue" below for those last few.

Did this actually cause a flake?

Yes — partially confirmed, and I want to be precise about what was and wasn't shown.

I ran the full module suite 8 times before the fix. 2 of 8 runs failed, each with a single different failure:

1. StreamingDataSourceTest.startWithHttp401PreventsSubsequentStart
AssertionError: Second start should not produce a callback expected null, but was:<LDInvalidResponseCodeFailure>
2. FDv2StreamingSynchronizerTest.streamRestartAfterInvalidDataRecordsMultipleDiagnostics
AssertionError: expected:<CHANGE_SET> but was:<STATUS>

The second one is a genuinely downstream class, which is the collateral damage the leak was suspected of causing.

After the fix I ran the full suite 8 more times: 8 of 8 green, 730 tests each.

Caveat: 8 runs before and 8 after is suggestive, not statistically conclusive for a failure rate around 25%. I am not claiming certainty that the leak is the sole cause of either failure. What is certain is the leak itself and its magnitude, and that both flakes disappeared.

I did work out a consistent mechanism for failure 1 that fits the leak: under load the first connection attempt can fail with a network error before the server's 401 is seen. StreamingDataSource.onError() reports that network error to the callback without setting any guard flags, so the test's awaitError() returns early. Meanwhile the retry gets the 401 and begins the non-retriable branch, which sets running = false before it sets connection401Error = true. If the test's second start() reads both fields inside that window, the guard passes and a second EventSource is created, which then reports the 401 to callback2. That is a pre-existing race in product code that the extra load makes reachable; this PR does not change it (see below).

The fix

• launchdarkly-android-client-sdk/src/test/java/android/os/Process.java (new) — a stub for android.os.Process, following the existing android.util.Base64 and android.util.Pair stubs in the same test source set. setThreadPriority becomes a no-op so stop()'s shutdown thread survives and actually closes the stream. android.os.Process is referenced from exactly one place in main source (StreamingDataSource.stop()), so the blast radius is that method.
• StreamingDataSourceTest.java — a startDataSource(...) helper that registers each started source in a list before starting it, and an @After that stops every registered source, awaiting the stop callback via the existing AwaitableCallback helper so teardown is deterministic. The 16 sds.start(callback) call sites now go through the helper. No test logic was changed.

Verification

• StreamingDataSourceTest alone, 5 consecutive runs: 5/5 green, 40 tests each.
• Full module suite, 8 consecutive runs: 8/8 green, 730 tests each. Verified both by Gradle exit code and by grepping the result XML for <failure/<error and counting <testcase entries.
• Thread census re-run after the fix, then all diagnostic instrumentation removed. Nothing temporary remains in the committed diff.

Known remaining residue, deliberately left alone

4–5 okhttp-eventsource-stream threads still survive the class (down from 34 EventSource threads). These come from the five tests whose data source is in a fast reconnect loop when we stop it (startWithHttp500/400/408/429ReportsRetriableError and startWithNetworkErrorReportsNetworkFailure), and the cause is upstream in okhttp-eventsource 4.2.0. In EventSource.tryStart() the loop does:

deliberatelyClosedConnection = calledStop = false;   // reset before every connect attempt
try { clientResult = client.connect(lastEventId); } catch (StreamException e) { exception = e; }

If close() lands inside that window, the flag it just set is wiped, the failed connect is treated as an ordinary retry, and the loop continues forever — it never consults readyState == SHUTDOWN. The surviving threads are much more benign than before (their OkHttp client is closed, so they just back off up to the 5 minute cap doing nothing), but they are still leaked. Fixing it belongs in okhttp-eventsource, not here.

The running / connection401Error ordering race described above is real product code behavior, not a test bug. Tightening it means making the guard-flag update and the callback atomic in StreamingDataSource. I left it out to keep this PR to the test-hygiene change.

Handlers.SSE.leaveOpen() leaves NanoHttpd request-processor threads sleeping after HttpServer.close() — about 9 threads by the end of this class. That is test-server side, inside com.launchdarkly.testhelpers, and unrelated to SDK behavior.

---

Note

Overview
Fixes unit-test thread leaks from StreamingDataSourceTest starting SSE connections without tearing them down, and makes stop() actually run in JVM unit tests.

Adds a test-only android.os.Process stub so setThreadPriority no longer throws under the mockable android.jar. Without it, StreamingDataSource.stop()’s shutdown thread dies before closing the EventSource, so stop was effectively a no-op in unit tests.

StreamingDataSourceTest now registers each started source via startDataSource(...) and an @After hook that stop()s every registered source and awaits the stop callback (5s timeout). All HttpServer-based tests that called sds.start(...) now go through the helper; assertions are unchanged.

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