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

Avoid polling when detecting ANRs by invoking JNI from SIGQUIT handler #741

Merged
merged 1 commit into from Jan 8, 2020

Conversation

fractalwrench
Copy link
Contributor

@fractalwrench fractalwrench commented Jan 7, 2020

Goal

The previous implementation of ANR detection used a SIGQUIT handler. When SIGQUIT was raised, a shared ByteBuffer was modified, and the JVM layer polled every 5ms to check whether an ANR had occurred.

Polling every 5ms is inefficient and risks an app being killed if it is in the background due to excessive work. This alternative approach invokes the JNI from the SIGQUIT handler and avoids the need for polling entirely.

Changeset

  • Added an AnrInterface class which is responsible for all communication from the ANR layer to the JVM layer
  • Update AnrPlugin initialisation so that a ByteBuffer is no longer required
  • Made collectAnrErrorDetails() accessible from AnrInterface (the containing class is still non-public)
  • Removed obsolete AppNotRespondingMonitor class

ANR handler

  • Cache references to the JVM + AnrInterface in Jni_On_Load, as JNIEnv is not thread safe and we need to obtain a reference each time the ANR handler is invoked
  • Remove SA_ONSTACK flag from SIGQUIT handler installation as we don't require information about the signal; we just need to know that it happened
  • Invoke AnrInterface through JNI in the SIGQUIT handler

Tests

I have verified that ANRs are detected on the following devices, and that changing releaseStage alters whether the error is detected or not:

  • Pixel 3a API 29
  • (Emulator) Nexus 5x API 26
  • (Emulator) Nexus 5x API 24
  • (Emulator) Nexus 5x API 21
  • (Emulator) Nexus 5 API 19

@fractalwrench fractalwrench changed the title Invoke JNI from SIGQUIT handler when detecting ANRs [WIP] Invoke JNI from SIGQUIT handler when detecting ANRs Jan 7, 2020
@bugsnagbot
Copy link
Collaborator

bugsnagbot commented Jan 7, 2020

Android notifier sizes

Format Size impact of Bugsnag (kB) Size impact of Bugsnag when Minified (kB)
APK 1386.98 1345.51
arm64_v8a 331.14 289.73
armeabi 311.33 269.93
armeabi_v7a 293.92 252.54
x86 369.26 327.87
x86_64 352.96 311.58

Generated by 🚫 Danger

@fractalwrench fractalwrench changed the title [WIP] Invoke JNI from SIGQUIT handler when detecting ANRs Invoke JNI from SIGQUIT handler when detecting ANRs Jan 7, 2020
@fractalwrench fractalwrench changed the title Invoke JNI from SIGQUIT handler when detecting ANRs Avoid polling when detecting ANRs by invoking JNI from SIGQUIT handler Jan 7, 2020
@fractalwrench fractalwrench requested review from tomlongridge and kattrali and removed request for tomlongridge January 7, 2020 14:46
Copy link
Contributor

@kattrali kattrali left a comment

Choose a reason for hiding this comment

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

Stopped early on obj_plugin

@fractalwrench
Copy link
Contributor Author

fractalwrench commented Jan 8, 2020

Apologies - it seems as those an old stale artefact must have been lying around the system, which meant ANRs appeared to work fine in the example app.

I've altered obj_plugin to be assigned to a global reference on installation and have verified that this works on my machine on a clean build now.

Copy link
Contributor

@kattrali kattrali left a comment

Choose a reason for hiding this comment

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

Nice! Very streamlined implementation. Noted a few smaller things but looks just about there.

The previous implementation of ANR detection used a SIGQUIT handler. When SIGQUIT was raised, a shared ByteBuffer was modified, and the JVM layer polled every 5ms to check whether an ANR had occurred.

Polling every 5ms is inefficient and risks an app being killed if it is in the background due to excessive work. This alternative approach invokes the JNI from the SIGQUIT handler and avoids the need for polling entirely.
Copy link
Contributor

@kattrali kattrali left a comment

Choose a reason for hiding this comment

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

NOICE

@fractalwrench fractalwrench merged commit 796c621 into next Jan 8, 2020
@fractalwrench fractalwrench deleted the anr-non-polling branch January 8, 2020 14:13
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.

None yet

3 participants