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

subprocess not killed when timeout fires #159

Open
alexandruy2k opened this issue Dec 13, 2023 · 4 comments
Open

subprocess not killed when timeout fires #159

alexandruy2k opened this issue Dec 13, 2023 · 4 comments

Comments

@alexandruy2k
Copy link

I've been using pytest-timeout for some time to catch whenever we have bad tests that just hang for a long time.
This doesn't happen very often and most of the time it happens locally and pytest-timeout saves the day.

We've been having an issue for some time when we run our tests in GitLab CI and a test ends up running for too long and then it's killed by pytest-timeout, The GitLab job hangs and eventually the GitLab runner crashes.

I debugged it today both locally and on GitLab and the issue I found is that we have a fixture where we open a server application using subprocess.Popen().
In the case where one of our tests takes too long and pytest is killed by pytest-timeout the subprocess remains open in the background.
Locally this isn't an issue because we cleanup any hanging processes at the start of our tests so they don't affect subsequent tests.
However in GitLab the job doesn't finish until all the processes are closed. Because the parent process is closed the GitLab runner isn't able to find any of the child processes and close those and everything hangs.

I'm running on Windows by the way.

So to get to my question, is there a way I can ensure that when pytest-timeout fires and kills pytest that it also kills any open subprocesses?

Thank you,

Alex

@flub
Copy link
Member

flub commented Dec 13, 2023 via email

@RonnyPfannschmidt
Copy link
Member

a opt-in for ci to ensure all child processes are killed on messed up platforms may be a reasonable workaround

ultimately this looks like a platform bug in gitlab - i wonder if a container or a ci managed daemon process could be a better help than making timeout handle the platform bug

@alexandruy2k
Copy link
Author

@flub I just use a script to call pytest, so I might be able to do something there. However I looked into a similar issue in the past and the problem is if pytest is still running there are commands available to find the list of processes and any child processes, so I could search for all python child processes and kill them. However when pytest-timeout fires, it kills pytest/python and those subprocesses are orphaned.

@RonnyPfannschmidt to add to my message above I don't think it's a platform bug in GitLab, I'm happy to be convinced, but how could GitLab or myself know which processes are started by my pytest session once the pytest process is killed? Don't those subprocesses become orphaned?

Even when I run this testing locally, those extra processes are left open after pytest-timeout fires and one way I manage that is to open task manager and close them by name. Programatically if I don't know their name once pytest/python is closed is there another way to find them?

Thank you both for the comments, in terms of a PR I think unfortunately it's outside my skillset, if more people have this issue maybe someone else will do a PR, in the meantime I will try to come up with a workaround and try to kill things myself afterwards

@alexandruy2k
Copy link
Author

so I've come up with a workaround now that I understand what the issue is, but I do maintain that this is an issue with pytest-timeout.
It should be possible when using pytest-timeout for everything to be cleaned up from within pytest and not have to resort to external cleaning up.

My workaround for anyone else that comes across this issue is to run pytest from a script. I use bash but powershell or oven a batch script would work

`
python -m pytest "$@"
ret_code=$?

echo "Force close sub-process open by python just in case it's hanging as a result of pytest-timeout firing"
taskkill //f //im "my_sub_process.exe"

exit ${ret_code}
`

I do think that more people are likely coming across this but they're not noticing it.
For example when running locally if pytest is ran a bunch of times and pytest-timeout fires it will leave any subprocess opened in the background. Eventually those processes will use up loads of memory and they would need to be killed using task manager or a reboot.
I think that this hasn't been noticed by others as in general I don't think pytest-timeout ends up firing for people that often, and if it does then they'd investigate the issue and end up with just 1 or 2 processes left behind which in most cases wouldn't cause issues and those processes would be killed on the next reboot.

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

No branches or pull requests

3 participants