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

Issues with MacOS Python 3.8 #4

Open
mikenac opened this issue Oct 27, 2020 · 2 comments
Open

Issues with MacOS Python 3.8 #4

mikenac opened this issue Oct 27, 2020 · 2 comments

Comments

@mikenac
Copy link

mikenac commented Oct 27, 2020

I am seeing some errors from this library inside of the 'kinesis-python' library, as described here: NerdWalletOSS/kinesis-python#14

This appears to be an issue with the way that Python 3.8 by default creates sub processes with "spawn" and not "fork", as referenced in that issue.

I've seen a workaround suggestion of setting:

multiprocessing.set_start_method("fork")

Is this an issue that has been seen before?

Maybe this could be done under a:

platform == "darwin":

@mikenac
Copy link
Author

mikenac commented Oct 27, 2020

I have no push rights so I cannot submit a PR, but here is a diff that makes all tests pass:

index e517769..cdea569 100644
--- a/src/offspring/process.py
+++ b/src/offspring/process.py
@@ -4,6 +4,7 @@ import multiprocessing
 import signal
 import sys
 import time
+import platform

 log = logging.getLogger(__name__)

@@ -16,6 +17,9 @@ class Subprocess(object):

     The run() method runs in the child process and implements whatever logic this process should execute.
     """
+    if platform.system() == "Darwin":
+        multiprocessing.set_start_method("fork")
+
     _INSTANCES = None

     WAIT_FOR_CHILD = False````

@borgstrom
Copy link
Owner

Hi @mikenac

Per the docs: https://docs.python.org/3/library/multiprocessing.html#multiprocessing.set_start_method

Note that this should be called at most once, and it should be protected inside the if __name__ == '__main__' clause of the main module.

So, this doesn't makes sense to set here since it could be called more than once. You should call multiprocessing.set_start_method("fork") in your own script that uses the kinesis-python library.

However, based on this comment: https://docs.python.org/3/library/multiprocessing.html#contexts-and-start-methods

Changed in version 3.8: On macOS, the spawn start method is now the default. The fork start method should be considered unsafe as it can lead to crashes of the subprocess.

I guess we should try to make this library work with spawn mode. TBH @NerdWallet hasn't moved from 3.7 -> 3.8/3.9 yet so we haven't run into this yet, and also we don't do too much Kinesis development on macOS.

We'll likely get this fixed once we switch to Py3.8 later this year. If you want to try and figure out how to work around this in the mean time I would be happy to participate in PRs and Issues.

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

2 participants