lithium icon indicating copy to clipboard operation
lithium copied to clipboard

interestingness.timed_run uses SIGKILL to terminate process

Open jschwartzentruber opened this issue 7 years ago • 3 comments

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.

jschwartzentruber avatar Apr 09 '18 14:04 jschwartzentruber

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 avatar Apr 09 '18 21:04 nth10sd

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 avatar Apr 09 '18 23:04 nth10sd

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.

nth10sd avatar May 01 '18 22:05 nth10sd