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

Fix potential deadlock when accessing a shared pasteboard from a background thread #310

Closed

Conversation

sanekgusev
Copy link
Contributor

Hello there.

We're using Adjust here at Duolingo and recently discovered an interesting deadlock scenario on one of our employees' devices. Please find an example watchdog termination crash report here: https://gist.github.com/sanekgusev/d5472726ac577032526799df37da532a.

Adjust is accessing a shared pasteboard fb_app_attribution to retrieve Facebook attribution ID. It does so by creating a UIPasteboard instance and reading the string data out of it. In the current implementation this appears to always happen from a background thread, shortly after initializing the SDK.

The deadlock occurs when Facebook tries to create an UIPasteboard instance on its own, at the exact same time, but from the main thread. This could be because it is actually accessing the exact same fb_app_attribution pasteboard, but this might be an issue for any concurrent shared pasteboard access.

Looking at the crashlog, the deadlock scenario can be outlined as follows:

  1. On a background thread Adjust SDK successfully initiates shared pasteboard creation by calling -[UIPasteboard pasteboardWithName: create:] (which tail calls into a private -[UIPasteboard _pasteboardWithName: create:] that we can see in the crashlog).
  2. As part of its implementation, -[UIPasteboard pasteboardWithName: create:] performs a barrier sync dispatch to an internal com.apple.UIKit.pasteboard.cache-queue, apparently to check if the pasteboard has been previously accessed and cached.
  3. Meanwhile, on the main thread, Facebook SDK, through calling +[FBSDKAppEventsUtility attributionID], attempts to get access to the same shared pasteboard and also calls -[UIPasteboard pasteboardWithName: create:]
  4. The same barrier sync dispatch occurs, but this time it blocks the calling main thread, because a background thread is already executing code on the com.apple.UIKit.pasteboard.cache-queue queue. Main thread is blocked at this point.
  5. Background thread where Adjust SDK is accessing the pasteboard, while still running as a barrier block on the com.apple.UIKit.pasteboard.cache-queue, attempts to do another sync dispatch, this time to the main queue, from the _pasteboardCacheQueue_existingItemCollectionWithName function. Deadlock.

While one can find evidence of people successfully using UIPasteboard class from threads other than main online, there is no explicit statement from Apple declaring that the class is thread-safe. As such, just as with any other UIKit API, we should assume that it may only be safely used from the main thread. The provided deadlock scenario above is another evidence to this.

If we wrap the UIPasteboard creation code in Adjust SDK into a dispatch_sync call and guarantee that it always runs on the main queue, we'd prevent the deadlock from happening. This PR does exactly that.

@uerceg
Copy link
Contributor

uerceg commented Oct 31, 2017

@sanekgusev

Thank you very much for the super detailed and helpful explanation and PR. Having in mind that new iOS SDK update will come soon, we will definitely consider adding change from this PR to it.

Cheers! 🍺

@uerceg uerceg mentioned this pull request Dec 12, 2017
@uerceg
Copy link
Contributor

uerceg commented Dec 13, 2017

@sanekgusev

Thank you one more time for this advice, we have included your commits in our official PR and made them part of our iOS SDK v4.12.0 release (https://github.com/adjust/ios_sdk/releases/tag/v4.12.0).

Cheers! 🍺

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants