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

Hang during initialization when SentrySDK.addBreadcrumb() is called from different thread #3899

Closed
juraj-blahunka opened this issue Apr 24, 2024 · 5 comments · Fixed by #3970

Comments

@juraj-blahunka
Copy link

Platform

iOS

Environment

TestFlight

Installed

Carthage

Version

8.24.0

Did it work on previous versions?

8.21.0

Steps to Reproduce

The issue was only reproducible in TestFlight / AppStore builds, not in DEBUG.
It was reproduced with 8.24.0 and 8.23.0. Didn't try 8.22.0. Not reproducible with 8.21.0.

  1. Launch the app
  2. Force-quit (kill) the app
  3. Run the app and wait as it hangs

The SDK is initialized through the usual Swift call:

SentrySDK.start { options in ... }

The breadcrumb is logged from a different thread (synchronous dispatch queue)

private lazy var cookiesSyncQueue = DispatchQueue(label: "CookieManagerSyncQueue")

cookiesChangeStream = NotificationCenter.default
    .publisher(for: .NSHTTPCookieManagerCookiesChanged, object: rawStorage)
    .throttle(for: 0.1, scheduler: cookiesSyncQueue, latest: true)
    .sink {  _ in
        let breadcrumb = ...
        SentrySDK.addBreadcrumb(breadcrumb)
    }

Other notes

Expected Result

The app launches without a hang.

Actual Result

The app hangs and gets killed by the watchdog after 20 seconds.

In the ips file, both threads hang with -[SentryCrashWrapper systemInfo] at the top of the trace. Here are the excerpts:

Main thread:

0   libsystem_kernel.dylib        	       0x1e266293c __ulock_wait2 + 8
1   libsystem_platform.dylib      	       0x1f6028ea8 _os_unfair_lock_lock_slow + 188
2   libobjc.A.dylib               	       0x1920138b8 objc_sync_enter + 20
3   kphone                        	       0x102ac5224 +[SentrySDK options] (in kphone) + 24 + 7918116
4   kphone                        	       0x102ad5508 -[SentryDependencyContainer crashReporter] (in kphone) + 88 + 7984392
5   kphone                        	       0x102a7e10c __32-[SentryCrashWrapper systemInfo]_block_invoke (in kphone) + 40 + 7627020
6   libdispatch.dylib             	       0x1a1ff6dd4 _dispatch_client_callout + 20
7   libdispatch.dylib             	       0x1a1ff8654 _dispatch_once_callout + 32
8   kphone                        	       0x102a7e0e0 -[SentryCrashWrapper systemInfo] (in kphone) + 60 + 7626976
9   kphone                        	       0x102a7e258 -[SentryCrashWrapper enrichScope:] (in kphone) + 164 + 7627352
10  kphone                        	       0x102a96f14 -[SentryHub initWithClient:andScope:] (in kphone) + 736 + 7728916
11  kphone                        	       0x102ac5768 +[SentrySDK startWithOptions:] (in kphone) + 640 + 7919464
12  kphone                        	       0x102ac5db4 +[SentrySDK startWithConfigureOptions:] (in kphone) + 88 + 7921076

Thread 2:

0   libsystem_kernel.dylib        	       0x1e2656578 __ulock_wait + 8
1   libdispatch.dylib             	       0x1a1ff77c4 _dlock_wait + 56
2   libdispatch.dylib             	       0x1a1ff76f4 _dispatch_once_wait + 112
3   kphone                        	       0x102a7e0e0 -[SentryCrashWrapper systemInfo] (in kphone) + 60 + 7626976
4   kphone                        	       0x102a7e258 -[SentryCrashWrapper enrichScope:] (in kphone) + 164 + 7627352
5   kphone                        	       0x102a96f14 -[SentryHub initWithClient:andScope:] (in kphone) + 736 + 7728916
6   kphone                        	       0x102ac51bc +[SentrySDK currentHub] (in kphone) + 60 + 7918012
7   kphone                        	       0x102ac6b3c +[SentrySDK addBreadcrumb:] (in kphone) + 40 + 7924540

Are you willing to submit a PR?

No response

@philipphofmann
Copy link
Member

philipphofmann commented Apr 25, 2024

I tried to reproduce this by adding breadcrumbs on a dispatch queue directly before starting the SDK but was unable to reproduce the issue:

let dispatchSerial = DispatchQueue(label: "queue")

for _ in 0..<100 {
    dispatchSerial.async {
        let crumb = Breadcrumb()
        crumb.message = "hello"
        SentrySDK.addBreadcrumb(crumb)
    }
}

SentrySDK.start(configureOptions: { options in
// ...

It looks like the locks of both threads end up in a deadlock. So both SentrySDK currentHub and [SentrySDK options] require a lock to self in SentrySDK.

+ (SentryHub *)currentHub
{
@synchronized(self) {
if (nil == currentHub) {
currentHub = [[SentryHub alloc] initWithClient:nil andScope:nil];
}
return currentHub;
}
}
+ (nullable SentryOptions *)options
{
@synchronized(self) {
return startOption;
}
}

That could be the culprit, but I would like to get a repro if possible so that we can fix it properly. @juraj-blahunka, how often does this happen? Every time or only from time to time? When do you call this code snipped?

private lazy var cookiesSyncQueue = DispatchQueue(label: "CookieManagerSyncQueue")

cookiesChangeStream = NotificationCenter.default
    .publisher(for: .NSHTTPCookieManagerCookiesChanged, object: rawStorage)
    .throttle(for: 0.1, scheduler: cookiesSyncQueue, latest: true)
    .sink {  _ in
        let breadcrumb = ...
        SentrySDK.addBreadcrumb(breadcrumb)
    }

@juraj-blahunka
Copy link
Author

@philipphofmann thank you for looking into it!

I've been trying to reproduce with a similar code in DEBUG but also failed. I was only able to reproduce when the debugger isn't attached (in TestFlight builds) and I get it to hang in ~70% of cases after force killing the app.

Everything happens from one method (simplified):

func application(_ application: UIApplication, willFinishLaunchingWithOptions ... {
    cookiesChangeStream = NotificationCenter.default.publisher(for: .NSHTTPCookieManagerCookiesChanged, object: rawStorage) ... // where the sink adds breadcrumbs
    SentrySDK.start { ... }

@philipphofmann
Copy link
Member

@juraj-blahunka, we will address the issues pointed out in #3899 (comment) because we should address them anyways. Maybe they fix your problem if not we need to investigate further.

@juraj-blahunka
Copy link
Author

@philipphofmann Great news, happy to test!

@philipphofmann
Copy link
Member

This is related to #3956.

philipphofmann added a commit that referenced this issue May 13, 2024
Fix a deadlock when two threads access SentrySDK.options and
SentrySDK.currentHub, which used the same object in synchronized.
This problem is fixed now by using two independent locks for
SentrySDK.options and SentrySDK.currentHub.

Fixes GH-3956, GH-3899
@philipphofmann philipphofmann linked a pull request May 13, 2024 that will close this issue
7 tasks
philipphofmann added a commit that referenced this issue May 13, 2024
Fix a deadlock when two threads access SentrySDK.options and
SentrySDK.currentHub, which used the same object in synchronized.
This problem is fixed now by using two independent locks for
SentrySDK.options and SentrySDK.currentHub.

Fixes GH-3956, Fixes GH-3899
threema-matteo pushed a commit to threema-ch/sentry-cocoa that referenced this issue May 21, 2024
Fix a deadlock when two threads access SentrySDK.options and
SentrySDK.currentHub, which used the same object in synchronized.
This problem is fixed now by using two independent locks for
SentrySDK.options and SentrySDK.currentHub.

Fixes getsentryGH-3956, Fixes getsentryGH-3899
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants