Closed Bug 1603499 Opened 4 years ago Closed 4 years ago

Update mozcrash's handling of java exceptions to work with JSON-formatted .extra files

Categories

(Testing :: Mozbase, task, P1)

Version 3
Unspecified
Android
task

Tracking

(firefox74 fixed)

RESOLVED FIXED
mozilla74
Tracking Status
firefox74 --- fixed

People

(Reporter: bugzilla, Assigned: gbrown)

References

Details

(Whiteboard: [geckoview] dev-prod-2020)

Attachments

(1 file)

It looks like check_for_java_exception needs to be updated to work with the new JSON-formatted .extra file.

Whiteboard: [geckoview]
Priority: -- → P3

Geoff, this is hurting us because crashes in automation are lacking important info. Any chance you could get to this soon?

Flags: needinfo?(gbrown)

No problem -- will have a look next week (I am pto tomorrow).

Assignee: nobody → gbrown
Flags: needinfo?(gbrown)

Geoff, please note that bug 1572355 hasn't been fixed yet.

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #3)

Geoff, please note that bug 1572355 hasn't been fixed yet.

Thanks. But there's a reviewed patch there now; I'll work with that.

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #1)

Geoff, this is hurting us because crashes in automation are lacking important info.

Can you point out some examples? I am having trouble forcing a crash by throwing my own exception.

Also, do we know how we want to report the .extra info? Which fields are of interest?

Flags: needinfo?(snorp)

I can kind of help with this:

If native code calls into java through JNI, and an exception was thrown in the java code, we get a crash in Accessor::EndAccess once the thread returns back to native. The .extra file contains additional data about the java exception via the JavaStackTrace annotation.

I'm not sure if snorp has any other comments, so I'll leave his needinfo.

Thanks Aaron.

So, I should report the JavaStackTrace from the .extra file instead of scraping the logcat for the stack dumped at https://searchfox.org/mozilla-central/rev/cfd1cc461f1efe0d66c2fdc17c024a203d5a2fd8/mobile/android/geckoview/src/main/java/org/mozilla/gecko/CrashHandler.java#163; that will be cleaner, but will it produce different results?

btw, currently, callers of check_for_java_exception() bail out of crash reporting after reporting the Java exception:
https://searchfox.org/mozilla-central/rev/cfd1cc461f1efe0d66c2fdc17c024a203d5a2fd8/build/mobile/remoteautomation.py#147

That's intentional and advantageous in that it prevents .dmp file crash reporting, which is voluminous and rarely relevant. But, it also prevents inclusion of the .dmp and .extra files in the task artifacts, which seems bad. I'll also try to change that so that the .extra and .dmp files, whenever present, are provided as artifacts.

(clearing NI, as :klotz has provided the info)

Flags: needinfo?(snorp)

Although not directly blocking, I intend to land on top of bug 1572244.

Depends on: 1572244

(In reply to Geoff Brown [:gbrown] (less available until Feb 5) from comment #7)

So, I should report the JavaStackTrace from the .extra file instead of scraping the logcat for the stack dumped at https://searchfox.org/mozilla-central/rev/cfd1cc461f1efe0d66c2fdc17c024a203d5a2fd8/mobile/android/geckoview/src/main/java/org/mozilla/gecko/CrashHandler.java#163; that will be cleaner, but will it produce different results?

I see now: Yes! In fact, for all the cases I have tested, the expected logcat message is not dumped, so check_for_java_exception() finds nothing, and normal minidump processing reports the native stack:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d45e4cff4186fe211e9b9f92618d99fb8c37c5f9

In contrast, my wip .extra processing dumps the Java exception and Java stack correctly:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=bd656f949d9cd0b19fe97ca26fde847720ac0eff

I need to sort out the python test failures and test more scenarios.

(In reply to Geoff Brown [:gbrown] (less available until Feb 5) from comment #11)

I need to sort out the python test failures and test more scenarios.

That's sorted out now.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=104e721e77f4006fe1245e05de8a26689fbbcad3

  • python tests pass
  • desktop tasks are of course unaffected by my java exception, and continue to pass
  • android compiled code tests (gtest, cppunit) are unaffected by the java exception and continue to pass
  • most geckoview tests including junit, mochitest, reftest, wpt crash with the forced java exception:
    • .dmp and .extra files are available as artifacts;
    • the classification line is like: PROCESS-CRASH | org.mozilla.geckoview.test.AutofillDelegateTest.autofillActiveChange | java.lang.NullPointerException: demo an exception at org.mozilla.geckoview.PanZoomController$NativeProvider.setAttached(PanZoomController.java:142)
  • the PROCESS-CRASH message is immediately followed by the full java stack
  • the full crash report is not displayed
  • sadly, raptor hangs in this scenario: like bug 1600193

:aklotz, :snorp -- Please have a look at this try push and let me know if the reporting of the java exception is optimal.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff98c9726477c31f5e09e34f7a9db08b90cda3ff

Flags: needinfo?(snorp)
Flags: needinfo?(aklotz)
Priority: P3 → P1

Stop using mozcrash.check_for_java_exception(), which scans logcat for uncaught
exceptions. Instead, check for java_stack in the extra dump information during
normal crash reporting; if java_stack is present, display the java exception and
stack instead of dumping a full native crash report.

I'm digging it! \o/ Thank you!

Flags: needinfo?(aklotz)
Pushed by gbrown@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bef203601b7f
Report java exceptions from minidump .extra files; r=bc
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla74

lgtm thanks!

Flags: needinfo?(snorp)
Whiteboard: [geckoview] → [geckoview] dev-prod-2020
See Also: → 1624211
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: