Skip to content

Commit d653590

Browse files
markushimarkushi
authored
Move to a single NetworkCallback listener to reduce number of IPC calls (#4164)
* Move to a single NetworkCallback listener to reduce number of IPC calls * Update Changelog * Cleanup return handling
1 parent 55c9166 commit d653590

File tree

3 files changed

+79
-44
lines changed

3 files changed

+79
-44
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
- The `ignoredErrors` option is now configurable via the manifest property `io.sentry.traces.ignored-errors` ([#4178](https://github.com/getsentry/sentry-java/pull/4178))
88
- A list of active Spring profiles is attached to payloads sent to Sentry (errors, traces, etc.) and displayed in the UI when using our Spring or Spring Boot integrations ([#4147](https://github.com/getsentry/sentry-java/pull/4147))
99
- This consists of an empty list when only the default profile is active
10+
- Move to a single NetworkCallback listener to reduce number of IPC calls on Android ([#4164](https://github.com/getsentry/sentry-java/pull/4164))
1011

1112
### Fixes
1213

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

Lines changed: 71 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,19 @@
44
import android.annotation.SuppressLint;
55
import android.content.Context;
66
import android.net.ConnectivityManager;
7+
import android.net.ConnectivityManager.NetworkCallback;
78
import android.net.Network;
89
import android.net.NetworkCapabilities;
910
import android.os.Build;
10-
import androidx.annotation.NonNull;
1111
import io.sentry.IConnectionStatusProvider;
1212
import io.sentry.ILogger;
13+
import io.sentry.ISentryLifecycleToken;
1314
import io.sentry.SentryLevel;
1415
import io.sentry.android.core.BuildInfoProvider;
1516
import io.sentry.android.core.ContextUtils;
16-
import java.util.HashMap;
17-
import java.util.Map;
17+
import io.sentry.util.AutoClosableReentrantLock;
18+
import java.util.ArrayList;
19+
import java.util.List;
1820
import org.jetbrains.annotations.ApiStatus;
1921
import org.jetbrains.annotations.NotNull;
2022
import org.jetbrains.annotations.Nullable;
@@ -31,8 +33,9 @@ public final class AndroidConnectionStatusProvider implements IConnectionStatusP
3133
private final @NotNull Context context;
3234
private final @NotNull ILogger logger;
3335
private final @NotNull BuildInfoProvider buildInfoProvider;
34-
private final @NotNull Map<IConnectionStatusObserver, ConnectivityManager.NetworkCallback>
35-
registeredCallbacks;
36+
private final @NotNull List<IConnectionStatusObserver> connectionStatusObservers;
37+
private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock();
38+
private volatile @Nullable NetworkCallback networkCallback;
3639

3740
public AndroidConnectionStatusProvider(
3841
@NotNull Context context,
@@ -41,7 +44,7 @@ public AndroidConnectionStatusProvider(
4144
this.context = ContextUtils.getApplicationContext(context);
4245
this.logger = logger;
4346
this.buildInfoProvider = buildInfoProvider;
44-
this.registeredCallbacks = new HashMap<>();
47+
this.connectionStatusObservers = new ArrayList<>();
4548
}
4649

4750
@Override
@@ -65,40 +68,64 @@ public AndroidConnectionStatusProvider(
6568

6669
@Override
6770
public boolean addConnectionStatusObserver(final @NotNull IConnectionStatusObserver observer) {
71+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
72+
connectionStatusObservers.add(observer);
73+
}
6874

69-
final ConnectivityManager.NetworkCallback callback =
70-
new ConnectivityManager.NetworkCallback() {
71-
@Override
72-
public void onAvailable(@NonNull Network network) {
73-
observer.onConnectionStatusChanged(getConnectionStatus());
74-
}
75-
76-
@Override
77-
public void onLosing(@NonNull Network network, int maxMsToLive) {
78-
observer.onConnectionStatusChanged(getConnectionStatus());
79-
}
80-
81-
@Override
82-
public void onLost(@NonNull Network network) {
83-
observer.onConnectionStatusChanged(getConnectionStatus());
84-
}
85-
86-
@Override
87-
public void onUnavailable() {
88-
observer.onConnectionStatusChanged(getConnectionStatus());
75+
if (networkCallback == null) {
76+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
77+
if (networkCallback == null) {
78+
final @NotNull NetworkCallback newNetworkCallback =
79+
new NetworkCallback() {
80+
@Override
81+
public void onAvailable(final @NotNull Network network) {
82+
updateObservers();
83+
}
84+
85+
@Override
86+
public void onUnavailable() {
87+
updateObservers();
88+
}
89+
90+
@Override
91+
public void onLost(final @NotNull Network network) {
92+
updateObservers();
93+
}
94+
95+
public void updateObservers() {
96+
final @NotNull ConnectionStatus status = getConnectionStatus();
97+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
98+
for (final @NotNull IConnectionStatusObserver observer :
99+
connectionStatusObservers) {
100+
observer.onConnectionStatusChanged(status);
101+
}
102+
}
103+
}
104+
};
105+
106+
if (registerNetworkCallback(context, logger, buildInfoProvider, newNetworkCallback)) {
107+
networkCallback = newNetworkCallback;
108+
return true;
109+
} else {
110+
return false;
89111
}
90-
};
91-
92-
registeredCallbacks.put(observer, callback);
93-
return registerNetworkCallback(context, logger, buildInfoProvider, callback);
112+
}
113+
}
114+
}
115+
// networkCallback is already registered, so we can safely return true
116+
return true;
94117
}
95118

96119
@Override
97120
public void removeConnectionStatusObserver(final @NotNull IConnectionStatusObserver observer) {
98-
final @Nullable ConnectivityManager.NetworkCallback callback =
99-
registeredCallbacks.remove(observer);
100-
if (callback != null) {
101-
unregisterNetworkCallback(context, logger, callback);
121+
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
122+
connectionStatusObservers.remove(observer);
123+
if (connectionStatusObservers.isEmpty()) {
124+
if (networkCallback != null) {
125+
unregisterNetworkCallback(context, logger, networkCallback);
126+
networkCallback = null;
127+
}
128+
}
102129
}
103130
}
104131

@@ -281,7 +308,7 @@ public static boolean registerNetworkCallback(
281308
final @NotNull Context context,
282309
final @NotNull ILogger logger,
283310
final @NotNull BuildInfoProvider buildInfoProvider,
284-
final @NotNull ConnectivityManager.NetworkCallback networkCallback) {
311+
final @NotNull NetworkCallback networkCallback) {
285312
if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.N) {
286313
logger.log(SentryLevel.DEBUG, "NetworkCallbacks need Android N+.");
287314
return false;
@@ -307,7 +334,7 @@ public static boolean registerNetworkCallback(
307334
public static void unregisterNetworkCallback(
308335
final @NotNull Context context,
309336
final @NotNull ILogger logger,
310-
final @NotNull ConnectivityManager.NetworkCallback networkCallback) {
337+
final @NotNull NetworkCallback networkCallback) {
311338

312339
final ConnectivityManager connectivityManager = getConnectivityManager(context, logger);
313340
if (connectivityManager == null) {
@@ -322,8 +349,13 @@ public static void unregisterNetworkCallback(
322349

323350
@TestOnly
324351
@NotNull
325-
public Map<IConnectionStatusObserver, ConnectivityManager.NetworkCallback>
326-
getRegisteredCallbacks() {
327-
return registeredCallbacks;
352+
public List<IConnectionStatusObserver> getStatusObservers() {
353+
return connectionStatusObservers;
354+
}
355+
356+
@TestOnly
357+
@Nullable
358+
public NetworkCallback getNetworkCallback() {
359+
return networkCallback;
328360
}
329361
}

‎sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import kotlin.test.BeforeTest
2424
import kotlin.test.Test
2525
import kotlin.test.assertEquals
2626
import kotlin.test.assertFalse
27+
import kotlin.test.assertNotNull
2728
import kotlin.test.assertNull
2829
import kotlin.test.assertTrue
2930

@@ -258,9 +259,12 @@ class AndroidConnectionStatusProviderTest {
258259
val observer = IConnectionStatusProvider.IConnectionStatusObserver { }
259260
val addResult = connectionStatusProvider.addConnectionStatusObserver(observer)
260261
assertTrue(addResult)
262+
assertEquals(1, connectionStatusProvider.statusObservers.size)
263+
assertNotNull(connectionStatusProvider.networkCallback)
261264

262265
connectionStatusProvider.removeConnectionStatusObserver(observer)
263-
assertTrue(connectionStatusProvider.registeredCallbacks.isEmpty())
266+
assertTrue(connectionStatusProvider.statusObservers.isEmpty())
267+
assertNull(connectionStatusProvider.networkCallback)
264268
}
265269

266270
@Test
@@ -269,18 +273,16 @@ class AndroidConnectionStatusProviderTest {
269273

270274
var callback: NetworkCallback? = null
271275
whenever(connectivityManager.registerDefaultNetworkCallback(any())).then { invocation ->
272-
callback = invocation.getArgument(0, NetworkCallback::class.java)
276+
callback = invocation.getArgument(0, NetworkCallback::class.java) as NetworkCallback
273277
Unit
274278
}
275279
val observer = mock<IConnectionStatusProvider.IConnectionStatusObserver>()
276280
connectionStatusProvider.addConnectionStatusObserver(observer)
277281
callback!!.onAvailable(mock<Network>())
278282
callback!!.onUnavailable()
279-
callback!!.onLosing(mock<Network>(), 0)
280283
callback!!.onLost(mock<Network>())
281-
callback!!.onUnavailable()
282284
connectionStatusProvider.removeConnectionStatusObserver(observer)
283285

284-
verify(observer, times(5)).onConnectionStatusChanged(any())
286+
verify(observer, times(3)).onConnectionStatusChanged(any())
285287
}
286288
}

0 commit comments

Comments
 (0)