Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

interestingness.timed_run uses SIGKILL to terminate process #60

Open
jschwartzentruber opened this issue Apr 9, 2018 · 3 comments
Open

Comments

@jschwartzentruber
Copy link
Collaborator

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
Copy link
Contributor

nth10sd commented Apr 9, 2018

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
Copy link
Contributor

nth10sd commented Apr 9, 2018

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
Copy link
Contributor

nth10sd commented May 1, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants