1421295 - After output timeout, mochitest may fail to kill some browser children

Status: RESOLVED FIXED

(Testing :: Mochitest, defect)

Description

[Geoff Brown [:gbrown]]

See https://bugzilla.mozilla.org/show_bug.cgi?id=1419121#c3: It is possible that the mochitest harness launches a browser and subsequently finds that some of the children of that browser are still running but now have a ppid of 1. In this case, the harness' attempt to kill off browser processes may not be fully successful because it may rely on the parent/child process relationship to determine which processes to kill. Consider particularly:

https://dxr.mozilla.org/mozilla-central/rev/cad9c9573579698c223b4b6cb53ca723cd930ad2/testing/mochitest/runtests.py#1998-2012

If psutil is available (it usually is), the harness checks for children at that point in time: If one is detached (ppid=1), it is missed.

In some cases, it is possible that not killing such a process may leave mozprocess hung, waiting for additional output, ultimately resulting in a test hang.


The alternate approach - using the process log - appears to be relied on by most other harnesses. However, that approach has at least one drawback: A child may have died and the pid may have been recycled by the OS (the harness may accidentally kill a process not related to the browser).

Comment 1

[Geoff Brown [:gbrown]]

btw, this code was added in bug 1143547. https://hg.mozilla.org/mozilla-central/rev/0983990d3768

Comment 2

[Geoff Brown [:gbrown]]

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

Comment 3

[Geoff Brown [:gbrown]]

Attachment: use process log as backup for psutil

In rare cases, I expect this to be helpful. It might also be harmful in rare cases (the pid recycling issue mentioned earlier). I'd like to take that risk and see how this goes. Try push (above) looks fine to me.

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8932569 [details] [diff] [review]
use process log as backup for psutil

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

I don't see anything dangerous here.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/135c1eb226df
Always use process log to find children; r=jmaher

Comment 6

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/135c1eb226df

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/fb9ff0d537b9