Closed Bug 529137 Opened 12 years ago Closed 12 years ago

Update Talos to cleanup child processes


(Testing :: General, defect)

Not set


(Not tracked)



(Reporter: jgriffin, Assigned: anodelman)


(Blocks 1 open bug)



(1 file)

Currently, Talos cleans up browser-related process before and after a test.  For e10s, this code needs to be updated to allow it to cleanup any orphaned child processes as well.
This patch causes Talos process management routines to be aware of the e10s child process ("mozilla-runtime") when cleaning up processes, and checking if any processes are running.  It defaults to "mozilla-runtime" but can be overridden by adding a "child_process" setting to a .config file.

This patch does not update some dead code, e.g., RunProcessAndWaitForOutput() in
Assignee: nobody → jgriffin
Attachment #412733 - Flags: review?(anodelman)
What platforms has this been tested on?
I've tested this on Windows 7 and Ubuntu 9.04; do you need it tested on other platforms?
We also run on mac 10.4/10.5 and windowsxp/vista.  I think that you probably have windows pretty decently covered, but a mac run would increase confidence.
Electrolysis is currently not supported on the Mac, but I will test it on the Mac to make sure I haven't broken anything there.
Blocks: 531142
I tested this on 10.6, which is the only mac OS I have easy access to, and it worked fine.
Attachment #412733 - Flags: review?(anodelman) → review+
ping? I want to turn on OOPP by default on the electrolysis tree this week, but I don't want to kill the Talos slave pool.
(In reply to comment #7)
> ping? I want to turn on OOPP by default on the electrolysis tree this week, but
> I don't want to kill the Talos slave pool.

bsmedberg: 1) what testing is needed to verify that OOPP-on-by-default is Talos-safe ? and 2) who's doing that?
jgriffin wrote the patch and tested it, as the many comments above attest, I think.
Comment on attachment 412733 [details] [diff] [review]
[checked in]patch

Checking in;
/cvsroot/mozilla/testing/performance/talos/,v  <--
new revision: 1.7; previous revision: 1.6
Checking in;
/cvsroot/mozilla/testing/performance/talos/,v  <--
new revision: 1.15; previous revision: 1.14
Checking in;
/cvsroot/mozilla/testing/performance/talos/,v  <--
new revision: 1.57; previous revision: 1.56
Checking in;
/cvsroot/mozilla/testing/performance/talos/,v  <--
new revision: 1.39; previous revision: 1.38
Attachment #412733 - Attachment description: patch → [checked in]patch
Back to alice for downtime/deployment, right? Can we get this done soon?
Assignee: jgriffin → anodelman
Completed by comment #10, I didn't close the bug when it landed as it wasn't assigned to me.
Closed: 12 years ago
Resolution: --- → FIXED
Hey thanks! I didn't realize that landing into CVS was the same as deployment.
Component: New Frameworks → General
You need to log in before you can comment on or make changes to this bug.