If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

mozrunner kills processes it does not own (on non-win32)

RESOLVED DUPLICATE of bug 627794

Status

Testing
Mozbase
--
major
RESOLVED DUPLICATE of bug 627794
6 years ago
6 years ago

People

(Reporter: johns, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

6 years ago
areweslimyet.com's tests were randomly failing when run in parallel. After much hair-pulling, I tracked it down to this in mozrunner's __init__.py:

>        if sys.platform != 'win32':
>            self.process_handler.kill()
>            for name in self.names:
>                for pid in get_pids(name, self.process_handler.pid):
>                    self.process_handler.pid = pid  # bad touch!
>                    self.process_handler.kill()

The comment in get_pids:
>     """Get all the pids matching name, exclude any pids below minimum_pid."""

So if the platform is not win32, this effectively kils all processes named e.g. 'firefox' with a higher numeric pid. Not only will this kill potentially unrelated processes, but it's not guaranteed to kill all processes mozrunner started (pids do not work that way)

I'm not sure what problem this code was trying to solve (electrolysis weirdness maybe?) but I locally removed the non-win32 fallback, so the .kill()/.wait() is used everywhere. This has caused no issues with processes not dying in the last few hundred tests on the areweslimyet linux test box.
Clint and Jeff, can one of you please comment? For now raising severity to major and setting platform to all because it also affects OS X.
Severity: normal → major
OS: Linux → All

Comment 2

6 years ago
This is a known bug fixed with mozmill 2 and mozprocess. https://bugzilla.mozilla.org/show_bug.cgi?id=627794 .  Incidentally, I just fixed a similar bug in Talos.  Moral of the story: blindly grepping through ps to kill processes is just friggin stupid.  We may consider switching to http://psychofx.com/psi/ for talos at some point, though, iirc, we may not need this for mozmill
Closing as dupe of bug 627794 then. There are no plans to backport those changes to hotfix-1.5.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 627794
You need to log in before you can comment on or make changes to this bug.