1503587 - Detect and cleanup crashreporter in Android tests

Status: RESOLVED FIXED

(Testing :: General, defect, P1)

Description

[Geoff Brown [:gbrown]]

Bug 1480786 showed that, in a multiprocess world, it is possible to launch the crashreporter UI even when the test harness has requested MOZ_CRASHREPORTER_NO_REPORT (buggy code might not propagate important environment to child processes). Currently this condition goes undetected: the crashreporter remains active and in the foreground, prompting for user input, after completion of the test run. I worry that it may affect test results on the next test run (eg. next manifest/directory of a mochitest job).

Comment 1

[Geoff Brown [:gbrown]]

Picking up from https://bugzilla.mozilla.org/show_bug.cgi?id=1480786#c5, recall that such a crash results in:

u0_a64    3329  1343  1263584 43072    ep_poll 00ffffe430 S org.mozilla.fennec_gbrown:crashreporter
u0_a64    26672 1343  1267508 50576          0 00e89cdf60 R org.mozilla.fennec_gbrown.CrashReporter

'adb shell am force-stop org.mozilla.fennec_gbrown' ends org.mozilla.fennec_gbrown:crashreporter but leaves org.mozilla.fennec_gbrown.CrashReporter running.

'adb shell am force-stop org.mozilla.fennec_gbrown.CrashReporter' is ineffective; 'adb shell kill <pid of org.mozilla.fennec_gbrown.CrashReporter>' is required.

Comment 2

[Geoff Brown [:gbrown]]

Attachment: check for and kill crashreporter

I forced a crashreporter ui locally and this worked just as I wanted:

 7:26.72 TEST_START: dom/media/tests/mochitest/test_dataChannel_basicAudio.html
wait for org.mozilla.fennec_gbrown complete; top activity=org.mozilla.fennec_gbrown
Browser unexpectedly found running. Killing...
Warning: found org.mozilla.fennec_gbrown.CrashReporter running; trying to kill...
TEST-UNEXPECTED-FAIL | dom/media/tests/mochitest/test_dataChannel_basicAudio.html | application timed out after 370 seconds with no output
INFO | automation.py | Application ran for: 0:07:00.655063
INFO | zombiecheck | Reading PID log: /tmp/tmpT3YLX5pidlog
MOZ_UPLOAD_DIR not defined; tombstone check skipped
PROCESS-CRASH | dom/media/tests/mochitest/test_dataChannel_basicAudio.html | java-exception java.lang.IllegalArgumentException: hi at org.mozilla.gecko.media.MediaControlService.onStartCommand(MediaControlService.java:45)
14:15.84 INFO Stopping web server


In non-exceptional circumstances, this seems to do no harm:

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=42cfa4bec92719140bd65f1d1fe4c5f0787cf35c

Comment 3

[Bob Clary [:bc] (inactive)]

Comment on attachment 9021561 [details] [diff] [review]
check for and kill crashreporter

Review of attachment 9021561 [details] [diff] [review]:
-----------------------------------------------------------------

Please fix the root=True issue and do a try run with android-hw as well before landing. Maybe a full ./mach try fuzzy --full --query 'android-hw'

These are all of the device kill/pkills in this file.

::: build/mobile/remoteautomation.py
@@ +451,4 @@
>                      retries += 1
> +                if self.device.process_exist(self.procName):
> +                    try:
> +                        self.device.pkill(self.procName, sig=9, attempts=1)

We'll need root=True for devices. Please also fix 

https://searchfox.org/mozilla-central/source/build/mobile/remoteautomation.py#430
https://searchfox.org/mozilla-central/source/build/mobile/remoteautomation.py#438

@@ +466,5 @@
> +            crashreporter = "%s.CrashReporter" % self.procName
> +            if self.device.process_exist(crashreporter):
> +                print("Warning: found %s running; trying to kill..." % crashreporter)
> +                try:
> +                    self.device.pkill(crashreporter)

ditto.

Comment 4

[Geoff Brown [:gbrown]]

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=13c14c12bea90b6457fabe800c7e4d0b5b34df58
https://treeherder.mozilla.org/#/jobs?repo=try&tier=1%2C2%2C3&revision=3cb6dff7629bbb2f6c2df0698b0e18bf255d1d2e

I worry that we are making bug 1486004 more and more difficult, but I'm not sure we have a good alternative.

Comment 5

[Bob Clary [:bc] (inactive)]

If we can get rid of kill and pkill for the app and any thing running on a non shell account by using stop_application or a similar approach we can roll back the need for root. But as long as we use kill on a process running under a different user account we'll need root for devices.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/97d1d375d25b
Check for and kill errant crashreporter after Android browser tests; r=bc

Comment 7

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/97d1d375d25b