972518 - Emulator can hang because it's stdout/stderr buffer fills up

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Jonathan Griffin (:jgriffin)]

When we launch the emulator via Marionette, we never read from its stdout/stderr pipes.  Normally, this output isn't interesting, but according to jmuizelaar, we can actually hang in some cases because these pipes fill up.

We should handle the output from the process and log it, so that we can see any errors that are being generated by it, and also prevent that kind of hang.

Comment 1

[Andrew Halberstadt [:ahal]]

We can make use of the existing LogcatProc, which basically does the same thing.

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: Patch 1.0 - log output of qemu

This works for me locally. I decided to just piggy back off of --logcat-dir for the location. If --logcat-dir isn't specified, no log will be created, but the output will still be consumed preventing overflows in the buffer.

I'll file a follow up bug to upload these to blobber. It would be interesting to see if they contain anything different on some of the runs that are timing out.

Comment 3

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8375844 [details] [diff] [review]
Patch 1.0 - log output of qemu

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

Thanks!  Yes, will be really interesting to see if this is the cause of some of our TBPL hangs.

Comment 4

[Andrew Halberstadt [:ahal]]

This patch depends on proc.poll() which just got added to mozprocess recently. I'll need to wait for the sync to finish before landing this.

Comment 5

[Andrew Halberstadt [:ahal]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/018ee8ea0c12

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back this out in http://hg.mozilla.org/integration/mozilla-inbound/rev/d769d6ca70b5 for breaking Mnw jobs:
https://tbpl.mozilla.org/php/getParsedLog.php?id=34874112&full=1&branch=mozilla-inbound#error0

Comment 7

[Andrew Halberstadt [:ahal]]

Yeah, it depends on the mozbase sync which got backed out after being merged to m-c.

Comment 8

[Andrew Halberstadt [:ahal]]

Attachment: Patch 1.1 - emulator.py fix

I was debating whether to just carry r+ forward or not, but figured maybe there was a reason we were calling Popen.terminate instead of Popen.kill. Jgriffin, do you remember if there was any particular reason?

I also got rid of the wait call because mozprocess.kill() calls wait implicitly.

Comment 9

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8378319 [details] [diff] [review]
Patch 1.1 - emulator.py fix

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

There wasn't a specific reason for choosing Popen.terminate(), AFAIK.

Comment 10

[Andrew Halberstadt [:ahal]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/957e1906ee1a

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/957e1906ee1a