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

RESOLVED FIXED in mozilla30


6 years ago
6 years ago


(Reporter: jgriffin, Assigned: ahal)


Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(2 attachments)

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.
We can make use of the existing LogcatProc, which basically does the same thing.
Assignee: nobody → ahalberstadt
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.
Attachment #8375844 - Flags: review?(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.
Attachment #8375844 - Flags: review?(jgriffin) → review+
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.
Depends on: 949600
Blocks: 974093
Yeah, it depends on the mozbase sync which got backed out after being merged to m-c.
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.
Attachment #8378319 - Flags: review?(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.
Attachment #8378319 - Flags: review?(jgriffin) → review+
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.