Joel Maher ( :jmaher ) (UTC -8) Reporter
	Description • 13 years ago

currently in our mochitest and talos harness we have a lot of code (partially duplicated) to handle checking for crashes and processing them.  We should include a large portion of this crash processing and checking in mozprocess.

Some functions from automation.py.in:
killAndGetStack: http://mxr.mozilla.org/mozilla-central/source/build/automation.py.in#709
checkForZombies: http://mxr.mozilla.org/mozilla-central/source/build/automation.py.in#823

from automationutils.py:
checkForCrashes: http://mxr.mozilla.org/mozilla-central/source/build/automationutils.py#103

from talos ttest.py:
cleanupAndCheckForCrashes: http://mxr.mozilla.org/build/source/talos/ttest.py#151

optionally we might want to include leak checking and analyzing (as found in automationutils.py).

	Jeff Hammel
	Comment 1 • 13 years ago

see also Bug 561235

	cmtalbert
	Comment 2 • 13 years ago

Yeah I think as a first step, we should combine the APIs mentioned in comment 0 and add them to the mozprocess object.

Looking over the APIs briefly, I think we can do the following:
* Combine checkForCrashes and cleanupAndCheckForCrashes into one API
* Add the killAndGetStack to mozprocess (perhaps by extending the current kill() method?)

It's not clear to me how we're going to deal with the checkForZombies API yet because I think that depends on the multi-process log file which is itself created by the mochitest harness.  So, to port that over, we probably want to add the ability to generate this log file to mozprocess as well.

	cmtalbert
	Comment 3 • 13 years ago

Sorry for the spam, realized something might not be clear...

(In reply to Clint Talbert ( :ctalbert ) from comment #2)
> Yeah I think as a first step, we should combine the APIs mentioned in
> comment 0 and add them to the mozprocess object.
> 
> Looking over the APIs briefly, I think we can do the following:
> * Combine checkForCrashes and cleanupAndCheckForCrashes into one API on the larger MozProcess API - just call it check_for_crashes and have it optionally perform cleanup steps, based on some parameter that it is passed.

	Jeff Hammel
	Comment 4 • 13 years ago

This might be better in mozrunner, as mozprocess is generic whereas mozrunner is specific to mozilla (XUL) programs

	Joel Maher ( :jmaher ) (UTC -8) Reporter
	Comment 5 • 13 years ago

can you describe what mozrunner is?  Is this something that makes sense to include in talos, mochitest,reftest and xpcshell?

Not sure if all our stuff is mozilla specific, or just a normal usage of breakpad.  If it is breakpad proper it would make sense to put in mozprocess.

	Jeff Hammel
	Comment 6 • 13 years ago

so mozprocess is really a generic utility for starting/stopping processes.  Ideally, if python had a robust process handling module we could use, mozprocess would not be necessary.

mozrunner is a wrapper around gecko/xul/xpcom/whatever applications that front-ends mozprocess (and mozprofile) for starting/stoppping these specific needs.  It sets up specific environment variables, a profile, preferences, command line flags that are only for mozilla code.  See: https://github.com/mozautomation/mozmill/blob/master/mozrunner/mozrunner/runner.py

As far as crash-handling goes, i'm not sure how mozilla-specific it is or not or how much can be done in a generic way.  If it can be done generically, mozprocess is the place to put it. If it cannot, mozrunner.

	Jeff Hammel
	Comment 7 • 13 years ago

see also https://bugzilla.mozilla.org/show_bug.cgi?id=679759

	Henrik Skupin [:whimboo][⌚️UTC+2]
	Comment 8 • 13 years ago

(In reply to Jeff Hammel [:jhammel] from comment #6)
> so mozprocess is really a generic utility for starting/stopping processes. 
> Ideally, if python had a robust process handling module we could use,
> mozprocess would not be necessary.

Partly off-topic, but how generic is it? I haven't taken a look at this module but I wonder if we could offer that code for the inclusion in Python proper? As long as it hasn't been done yet in the most recent 3.x branch.

	cmtalbert
	Comment 9 • 13 years ago

(In reply to Henrik Skupin (:whimboo) from comment #8)
> (In reply to Jeff Hammel [:jhammel] from comment #6)
> > so mozprocess is really a generic utility for starting/stopping processes. 
> > Ideally, if python had a robust process handling module we could use,
> > mozprocess would not be necessary.
> 
> Partly off-topic, but how generic is it? I haven't taken a look at this
> module but I wonder if we could offer that code for the inclusion in Python
> proper? As long as it hasn't been done yet in the most recent 3.x branch.

The internal Popen class is very generic, and it was written with an eye to including it into the standard library, but I (nor anyone else) has had time to try to upstream it yet.  Specifically it tracks child processes which the current stdlib Popen module doesn't do.  (This module expands the Popen class to watch and handle subprocesses).  The outer layer of mozprocess (the part that wraps the internal Popen class) is a customized process interaction layer and I don't think it really belongs in the stdlib.

	Jeff Hammel
	Comment 10 • 13 years ago

To my knowledge, mozprocess is completely generic.  I agree that it would be nice to upgrade subprocess.Popen to take in the mozprocess changes.

However, getting anything into the python standard lib is a huge deal and a matter of politics.  To my knowledge, no one has time to deal with this.  It won't be included in python 2, and I don't even know the state of subprocess on python 3 or if it has improved with killing processes.  Considerable research would need to be done to document mozprocess and its improvements over standard Popen (which I don't think are fully understood) and to reformat mozprocess into the subprocess API.

mozprocess is already available on pypi.  If people start using mozprocess because it is good then we can reasonably push on the python board that it is worth upstreaming.  Note again that this is a long and laborious process.

	Andrew Halberstadt [:ahal]
	Comment 11 • 13 years ago

I took the checkForCrashes method and stripped out the CGI script bits, downloading and extracting the symbols if a url is passed in:

https://github.com/ahal/peptest/blob/4ed9090aaee9cb80dda0cc09d2d5fed061c80de7/runpeptests.py#L276

	Jeff Hammel
	Comment 12 • 12 years ago

We now have mozcrash (thank you, Ted!): https://github.com/mozilla/mozbase/commit/6204991f24fa40452a464c9560b7e64c13b5c404 . We should make use of that (and peptest should be updated to use that, etc)

	Jeff Hammel
	Comment 13 • 12 years ago

So, is this done?