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

Status: RESOLVED FIXED

(Testing :: Mozbase, task, P1)

Description

[(No longer employed by Mozilla) Aaron Klotz]

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

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

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

Comment 2

[Geoff Brown [:gbrown]]

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

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

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

Comment 4

[Geoff Brown [:gbrown]]

(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.

Comment 5

[Geoff Brown [:gbrown]]

(In reply to James Willcox (:snorp) ([e-mail]) (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?

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

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.

Comment 7

[Geoff Brown [:gbrown]]

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?

Comment 8

[Geoff Brown [:gbrown]]

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.

Comment 9

[James Willcox (:snorp) ([e-mail]) (he/him)]

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

Comment 10

[Geoff Brown [:gbrown]]

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

Comment 11

[Geoff Brown [:gbrown]]

(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.

Comment 12

[Geoff Brown [:gbrown]]

(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

Comment 13

[Geoff Brown [:gbrown]]

: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

Comment 14

[Geoff Brown [:gbrown]]

Attachment: Bug 1603499 - Report java exceptions from minidump .extra files; r=

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.

Comment 15

[(No longer employed by Mozilla) Aaron Klotz]

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

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bef203601b7f
Report java exceptions from minidump .extra files; r=bc

Comment 17

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/bef203601b7f

Comment 18

[James Willcox (:snorp) ([e-mail]) (he/him)]

lgtm thanks!