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

Fix process handling on Windows and Mac #120

Merged
merged 3 commits into from
Jul 14, 2023
Merged

Conversation

jdholtz
Copy link
Owner

@jdholtz jdholtz commented Jul 14, 2023

Fixes #118 and #119.

When multiprocessing uses the 'spawn' start method, the Process object cannot be an instance variable as it can't be pickled. This PR changes the previous implementation to only keep track of the Process ID. Then, the os module is used to kill the process when we need to.

Additionally, os.waitpid() is used (isn't needed in Windows) to ensure killed processes don't turn into zombie/defunct processes. Now, no zombie processes are left from this script.

jdholtz added 3 commits July 12, 2023 10:14
When using the 'spawn' method, each class is pickled. However, the
Process object cannot be pickled and therefore cannot be referenced as
an instance variable.

Due to these limitations, the Process ID is now kept track of and killed
using os.kill. os.waitpid is used as well to prevent zombie processes.
This functionality isn't in undetected_chromedriver yet (although I
might make a PR). Therefore, this has to be handled within this script.

There should no longer be any zombie processes left from this script.
terminate

Windows does not leave zombie processes after os.kill() is called.
Therefore, during waitpid() it will throw a ChildProcessError as the PID
is not a child of the parent process. To fix it, the exception can
simply be caught and passed.
@jdholtz jdholtz merged commit c24da51 into develop Jul 14, 2023
@jdholtz jdholtz deleted the fix-process-handling branch July 14, 2023 04:27
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

Successfully merging this pull request may close these issues.

1 participant