Skip to content

Commit 14ad433

Browse files
committed
fix(android): Run the telephony callback on its own thread
The provider submitted the display info update to the SDK executor it got at construction. That reference goes stale twice over. The options hold a no-op executor until `SentryOptions.activate` replaces it, and `AndroidOptionsInitializer` builds this provider from the options configuration callback, which `Sentry.init` runs before `activate`. A second `SentryAndroid.init` then shuts the previous executor down while the provider keeps the dead one. Either way the submitted work never runs, and from API 31 on the listener is the only source of the value. Pass `Runnable::run` instead. The callback maps the display info to a generation and stores it, so the binder thread the framework calls it on is the right place for it. The executor, its field and the `RejectedExecutionException` guard all go away.
1 parent f295ed1 commit 14ad433

3 files changed

Lines changed: 11 additions & 33 deletions

File tree

‎sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,7 @@ public AndroidConnectionStatusProvider(
9696
this.handler = handler;
9797
this.connectionStatusObservers = new ArrayList<>();
9898
this.cellularNetworkTechnologyProvider =
99-
new CellularNetworkTechnologyProvider(
100-
this.context, options.getLogger(), buildInfoProvider, options.getExecutorService());
99+
new CellularNetworkTechnologyProvider(this.context, options.getLogger(), buildInfoProvider);
101100

102101
capabilities[0] = NetworkCapabilities.NET_CAPABILITY_INTERNET;
103102
if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.M) {

‎sentry-android-core/src/main/java/io/sentry/android/core/internal/util/CellularNetworkTechnologyProvider.java‎

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,9 @@
1010
import androidx.annotation.NonNull;
1111
import androidx.annotation.RequiresApi;
1212
import io.sentry.ILogger;
13-
import io.sentry.ISentryExecutorService;
1413
import io.sentry.SentryLevel;
1514
import io.sentry.android.core.BuildInfoProvider;
1615
import io.sentry.android.core.ContextUtils;
17-
import java.util.concurrent.Executor;
18-
import java.util.concurrent.RejectedExecutionException;
1916
import org.jetbrains.annotations.ApiStatus;
2017
import org.jetbrains.annotations.NotNull;
2118
import org.jetbrains.annotations.Nullable;
@@ -45,13 +42,6 @@ public final class CellularNetworkTechnologyProvider {
4542
/** Guards the registration of the display info listener. */
4643
private final @NotNull Object registrationLock = new Object();
4744

48-
/**
49-
* The executor the telephony framework calls. The framework calls it on a thread of its own, so a
50-
* rejection must not escape: the SDK's executor rejects work once it is shut down, which can
51-
* happen before the listener is unregistered.
52-
*/
53-
private final @NotNull Executor executor;
54-
5545
/**
5646
* Set from {@link TelephonyCallback.DisplayInfoListener} on API 31 and above. Declared as {@link
5747
* Object} so that loading this class on older devices never has to resolve a type that does not
@@ -64,22 +54,10 @@ public final class CellularNetworkTechnologyProvider {
6454
public CellularNetworkTechnologyProvider(
6555
final @NotNull Context context,
6656
final @NotNull ILogger logger,
67-
final @NotNull BuildInfoProvider buildInfoProvider,
68-
final @NotNull ISentryExecutorService executorService) {
57+
final @NotNull BuildInfoProvider buildInfoProvider) {
6958
this.context = ContextUtils.getApplicationContext(context);
7059
this.logger = logger;
7160
this.buildInfoProvider = buildInfoProvider;
72-
this.executor =
73-
runnable -> {
74-
try {
75-
executorService.submit(runnable);
76-
} catch (RejectedExecutionException e) {
77-
logger.log(
78-
SentryLevel.DEBUG,
79-
"Dropping a cellular network technology update, the executor rejected it.",
80-
e);
81-
}
82-
};
8361
}
8462

8563
/**
@@ -114,7 +92,12 @@ public void register() {
11492
}
11593
try {
11694
final @NotNull DisplayInfoCallback callback = new DisplayInfoCallback(this);
117-
telephonyManager.registerTelephonyCallback(executor, callback);
95+
// The callback maps the display info to a generation and stores it, so it runs on the
96+
// binder thread the telephony framework calls it on. An executor of the SDK would add a
97+
// reference that goes stale: the options hold a no-op executor until
98+
// SentryOptions.activate replaces it, and a second SentryAndroid.init shuts the previous
99+
// one down.
100+
telephonyManager.registerTelephonyCallback(Runnable::run, callback);
118101
displayInfoCallback = callback;
119102
logger.log(SentryLevel.DEBUG, "Started listening for cellular network technology changes.");
120103
} catch (SecurityException | IllegalStateException | UnsupportedOperationException e) {

‎sentry-android-core/src/test/java/io/sentry/android/core/internal/util/CellularNetworkTechnologyProviderTest.kt‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
1111
import com.google.common.truth.Truth.assertThat
1212
import io.sentry.ILogger
1313
import io.sentry.android.core.BuildInfoProvider
14-
import io.sentry.test.ImmediateExecutorService
1514
import java.util.concurrent.CountDownLatch
1615
import java.util.concurrent.TimeUnit
1716
import kotlin.test.BeforeTest
@@ -36,9 +35,6 @@ class CellularNetworkTechnologyProviderTest {
3635
val buildInfo = mock<BuildInfoProvider>()
3736
val telephonyManager = mock<TelephonyManager>()
3837

39-
/** Runs the display info callback inline, so tests don't have to wait for another thread. */
40-
val executorService = ImmediateExecutorService()
41-
4238
fun getSut(
4339
sdkVersion: Int = Build.VERSION_CODES.S,
4440
hasPermission: Boolean = false,
@@ -49,7 +45,7 @@ class CellularNetworkTechnologyProviderTest {
4945
.thenReturn(if (hasTelephonyManager) telephonyManager else null)
5046
whenever(context.checkPermission(any(), any(), any()))
5147
.thenReturn(if (hasPermission) PERMISSION_GRANTED else PERMISSION_DENIED)
52-
return CellularNetworkTechnologyProvider(context, logger, buildInfo, executorService)
48+
return CellularNetworkTechnologyProvider(context, logger, buildInfo)
5349
}
5450

5551
fun displayInfo(networkType: Int, overrideNetworkType: Int): TelephonyDisplayInfo {
@@ -65,8 +61,8 @@ class CellularNetworkTechnologyProviderTest {
6561
): TelephonyCallback.DisplayInfoListener {
6662
provider.register()
6763
val captor = argumentCaptor<TelephonyCallback>()
68-
// The provider wraps the executor service, so the executor it hands to the telephony
69-
// manager is not an object this test holds.
64+
// The provider runs the callback on the thread the framework calls, so the executor it
65+
// hands over only calls `run`.
7066
verify(telephonyManager).registerTelephonyCallback(any(), captor.capture())
7167
return captor.firstValue as TelephonyCallback.DisplayInfoListener
7268
}

0 commit comments

Comments
 (0)