materialsproject / fireworks

The Fireworks Workflow Management Repo.
https://materialsproject.github.io/fireworks
Other
361 stars 185 forks source link

default multiprocessing change #416

Closed mamachra closed 4 years ago

mamachra commented 4 years ago

As of now running the following command: "lpad -l my_launchpad.yaml webgui" will result in the following error: File "/Users/me/miniconda3/bin/lpad", line 11, in load_entry_point('FireWorks', 'console_scripts', 'lpad')() File "/Users/me/repos/fireworks/fireworks/scripts/lpad_run.py", line 1434, in lpad args.func(args) File "/Users/me/repos/fireworks/fireworks/scripts/lpad_run.py", line 686, in webgui p1.start() File "/Users/me/miniconda3/lib/python3.8/multiprocessing/process.py", line 121, in start self._popen = self._Popen(self) File "/Users/me/miniconda3/lib/python3.8/multiprocessing/context.py", line 224, in _Popen return _default_context.get_context().Process._Popen(process_obj) File "/Users/me/miniconda3/lib/python3.8/multiprocessing/context.py", line 283, in _Popen return Popen(process_obj) File "/Users/me/miniconda3/lib/python3.8/multiprocessing/popen_spawn_posix.py", line 32, in init super().init(process_obj) File "/Users/me/miniconda3/lib/python3.8/multiprocessing/popen_fork.py", line 19, in init self._launch(process_obj) File "/Users/me/miniconda3/lib/python3.8/multiprocessing/popen_spawn_posix.py", line 47, in _launch reduction.dump(process_obj, fp) File "/Users/me/miniconda3/lib/python3.8/multiprocessing/reduction.py", line 60, in dump ForkingPickler(file, protocol).dump(obj) TypeError: cannot pickle '_thread.lock' object

This beavior is observed under python >= 3.8.X and a macOS Catalina environment. I suspect that the recent feature changes in python 3.8, more precisely, on MacOS, the spawn start method is not used by default for multiprocessing. I believe a straightforward fix would be to explicitly define the start method as 'fork' using: from multiprocessing import set_start_method set_start_method('fork')

This has fixed the problem. However, note that the following UserWarning will appear: /Users/me/miniconda3/lib/python3.8/site-packages/pymongo-3.11.0-py3.8-macosx-10.9-x86_64.egg/pymongo/topology.py:161: UserWarning: MongoClient opened before fork. Create MongoClient only after forking. See PyMongo's documentation for details: https://pymongo.readthedocs.io/en/stable/faq.html#is-pymongo-fork-safe

I believe this is coming from "app.lp = get_lp(args)" Line 662 of the lpad_run.py. On unix machines, the multiprocessing spawns processes using fork, where instances of MongoClient should not be copied from a parent process to a child process. While, as defined in lpad_run.py the MOngoClient instance is not created by the process, and the global MongoClient is being copied by the process instead.

1fish2 commented 4 years ago

I'll send you a PR to fix this Issue.

The idea to use 'fork' instead of 'spawn' is risky because the Python 3.8 multiprocessing docs say:

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.

An easy fix is to fork a thread to open the web browser (which of course opens or communicates with another process) rather than forking a process to run the web server. Actually, the thread is just a way to delay to give the web server more startup time in case that's needed. It works for me without that delay but I didn't test across a range of OSs and machines. (FYI another way to delay would be to construct a javascript: URL that delays then goes to the desired page.)