Skip to content

Commit 349566b

Browse files
0xadam-brownlazerg
andauthored
fix(android): Don't block callers on network status registration (JAVA-828) (#6537)
AndroidConnectionStatusProvider no longer makes callers wait while it registers its network and telephony callbacks with system services. That's good because one of those callers is Sentry.init. THE PROBLEM Before this commit, AndroidConnectionStatusProvider used one lock to guard three code paths: callback registration, connection cache refreshes, and adding or removing observers. The SDK executor held that lock across ConnectivityManager.registerDefaultNetworkCallback and, on API 31+, TelephonyManager.registerTelephonyCallback. Both are calls into system services that can occasionally block for seconds. The main thread waited on the same lock in two places: Session Replay adding its observer during Sentry.init. Every captured event reading the connection type or status. If the system service registrations took a while, the main thread would stall. Scenario (1) in particular could lead to ANRs during app startup. Oof. A ROAD NOT TAKEN Any fix has to isolate the slow part (registration) so that the main thread can return quickly when reading cached network info or adding observers. The narrowest approach would be to give registration its own lock and keep a second lock for the cache and observers. But a refresh would then either hold the cache lock while querying Android, making readers wait on that query, or need a version counter that every write must remember to update. Even worse, two locks would need to document order they're taken in to avoid deadlock, which would be easy to get wrong. A BETTER APPROACH The approach we ultimately landed on was to avoid having the main thread hold a lock entirely, and instead rely on atomically publishing cached network info (see commit 2). We did so by introducing a new ConnectionSnapshot class, which is immutable and can be safely shared across threads. We stored observers in copy-on-write lists rather than having them be explicitly locked, and we confined registration to the the SDK executor (it's still defensively guarded by its own lock). Callers that need to access cached network info or register observers can now return quickly. Either of approach would have allowed as much, of course, but the benefit of introducing the snapshot it that it also: - lets us consolidate a lot of related state that the provider previously had to handle separately - avoids the pain of managing (yet another) lock - lays the groundwork for future PRs that confine refreshes to the SDK executor and let snapshots tell a lost network from capabilities that are simply unknown. Co-authored-by: Lazizbek Ergashev <20501725+lazerg@users.noreply.github.com>
1 parent bcc4ebe commit 349566b

4 files changed

Lines changed: 1430 additions & 225 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131

3232
### Fixes
3333

34+
- Prevent the main thread from potentially hanging during app start if Android is slow to register the SDK's network or telephony listeners. `Sentry.init`, Session Replay, and capturing events no longer wait for that registration ([#6537](https://github.com/getsentry/sentry-java/pull/6537))
3435
- Add opt-in support for matching GraphQL ignored error types against classification specifications via `sentry.graphql.enable-specification-error-type-filtering` ([#6092](https://github.com/getsentry/sentry-java/pull/6092))
3536
- Session Replay: Fix `SentryOkHttpInterceptor` hanging forever on responses whose body has no known length, such as Server-Sent Events or a gzipped response, when network details are enabled for the URL ([#6231](https://github.com/getsentry/sentry-java/pull/6231))
3637
- Session Replay: `SentryOkHttpInterceptor` now captures network request and response details only while Session Replay is recording or buffering ([#6486](https://github.com/getsentry/sentry-java/pull/6486))

0 commit comments

Comments
 (0)