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

[PLAT-6800] App hang detection improvements #1122

Merged
merged 3 commits into from
Jun 18, 2021

Conversation

nickdowell
Copy link
Contributor

@nickdowell nickdowell commented Jun 17, 2021

Goal

To prevent false positive detection of app hangs and improve performance.

Changeset

False positive mitigation

App hangs will now be ignored if the detector thread wakes up too long after the specified deadline. This condition could occur if an app was suspended while the detector thread was waiting on the (processingFinished) semaphore and subsequently woke up before the main thread was able to signal.

The app hang deadline is now advanced whenever kCFRunLoopBeforeSources is encountered. This caters for cases where a busy run loop runs multiple timers / sources iteration before sleeping.

Performance improvement

The detector has been reworked to use a long-lived thread controlled by signalling two semaphores. This is more efficient - profiling revealed that dispatch_after is roughly 7x slower than dispatch_semaphore_signal.

Other improvements

App hangs are now symbolicated in-app (this was broken by the recent change to symbolicate asynchronously)

Testing

Automated testing via E2E tests running the app hang scenarios.

Updated the example apps to include a fatal app hang sample.

Tested manually using example apps.

@github-actions
Copy link

Infer: No issues found 🎉

OCLint: No issues found 🎉

Bugsnag.framework binary size increased by 2,136 bytes from 1,126,544 to 1,128,680

Generated by 🚫 Danger


// Start monitoring immediately so that app hangs during launch can be detected.
// Note that because scene-based apps start in UIApplicationStateBackground, hangs in
// -[AppDelegate application:didFinishLaunchingWithOptions:] will be ignored.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any way we can detect this in future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could temporarily allow background hangs to be acted upon, we'd need to find a suitable point at which to reenable the usual logic though.

Briefly discussed this with Tom and agreed it was better to address it as a separate issue to be discussed with the product team.

@nickdowell nickdowell merged commit 055b2d2 into next Jun 18, 2021
@nickdowell nickdowell deleted the nickdowell/app-hang-detection-improvements branch June 18, 2021 10:25
@nickdowell nickdowell mentioned this pull request Jun 23, 2021
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