-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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: Debugger #495
Fix: Debugger #495
Conversation
…warning about it.
@@ -145,6 +146,7 @@ public void execute(@NonNull final ReportBuilder reportBuilder) { | |||
|| (config.resToastText() != 0 && (reportingInteractionMode == ReportingInteractionMode.NOTIFICATION || reportingInteractionMode == ReportingInteractionMode.DIALOG)); | |||
|
|||
final TimeHelper sentToastTimeMillis = new TimeHelper(); | |||
boolean displayingToast = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why introduce the 'displayingToast' variable?
Surely you just want to make sure you don't kill the process if the debugger is connected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a development artifact. I first tried to kill the process without triggering the kill on all other processes, which is why this was required. Now it is not and I'll remove it really quick.
…k when debugger is connected
android.os.Process.killProcess(android.os.Process.myPid()); | ||
System.exit(10); | ||
//prevent process kill if a debugger is attached, as this would kill the whole application | ||
if (!Debug.isDebuggerConnected()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should only be checking for !Debug.isDebuggerConnected() once, one either line#266 or line#296, not both.
If the Debugger is connected, do you just want to not kill the process?
Or do you also not want it to call the defaultExceptionHandler and clear the last Activity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can not hand the exception to the defaultExceptionHandler. It will also kill the process. Clearing the last activity is okay though (and as this is the only thing we can do to minimize inconsistent states I think this should definetly be done).
I finally found out what causes #478. The ActivityManagerService kills all processes of a package when a debugger is detached from one of them due to a crash.
The best solution I could come up with was to not kill the process in this situation.
However, this might have unknown side-effects, which is why it displays (and logs) a warning about it.