Closed
Bug 1421295
Opened 6 years ago
Closed 6 years ago
After output timeout, mochitest may fail to kill some browser children
Categories
(Testing :: Mochitest, defect)
Testing
Mochitest
Tracking
(firefox58 fixed, firefox59 fixed)
RESOLVED
FIXED
mozilla59
People
(Reporter: gbrown, Assigned: gbrown)
References
Details
Attachments
(1 file)
1.95 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
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).
Assignee | ||
Comment 1•6 years ago
|
||
btw, this code was added in bug 1143547. https://hg.mozilla.org/mozilla-central/rev/0983990d3768
Assignee | ||
Comment 2•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9dc1ab433a66bd1a4032a2036b92710bca120c3b
Assignee | ||
Comment 3•6 years ago
|
||
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.
Attachment #8932569 -
Flags: review?(jmaher)
Comment 4•6 years ago
|
||
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.
Attachment #8932569 -
Flags: review?(jmaher) → review+
Pushed by gbrown@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/135c1eb226df Always use process log to find children; r=jmaher
Comment 6•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/135c1eb226df
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment 7•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/fb9ff0d537b9
status-firefox58:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•