search

MozillaSecurity / lithium

Line-based testcase reducer
Mozilla Public License 2.0
95 stars 25 forks source link

interestingness.timed_run uses SIGKILL to terminate process #60

Open jschwartzentruber opened 6 years ago

jschwartzentruber commented 6 years ago

Many of the interestingness scripts use timed_run to implement timeout support. When timeout occurs, subprocess.Popen.kill() is called, which uses SIGKILL. If the subprocess is a wrapper script, no cleanup can occur since SIGKILL cannot be handled. We should try SIGINT or SIGTERM first to give the child a chance to cleanup.

Concretely, if run with ffpuppet as a subprocess, this leaves orphaned Firefox (and Xvfb if --xvfb is used) processes on every timeout.

nth10sd commented 6 years ago

I've been thinking, whether it is possible to replace some usages of the timed_run function with subprocess.run() or its subprocess32 PyPI counterpart...

nth10sd commented 6 years ago

That said, I didn't actually answer you. Yeah, as far as I can recall, SIGINT/SIGTERM didn't clean up properly historically, hence SIGKILL was used. I think ample years have passed, for us now to try SIGINT/SIGTERM first before falling back to SIGKILL.

nth10sd commented 6 years ago

Last week, we spoke concretely about moving to subprocess32 possibly while fixing this, so yes we should do this. Moreover, the Python 2.7.15 release notes now mention that users are "strongly encouraged" to use subprocess32 too.