| msg281565 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2016-11-23 15:26 |
| The following script: import multiprocessing import os def f(): pass multiprocessing.Lock() if __name__ == "__main__": ctx = multiprocessing.get_context('forkserver') # modname is the script's importable name (not "__main__") modname = os.path.basename(__file__).split(".")[0] ctx.set_forkserver_preload([modname]) proc = ctx.Process(target=f) proc.start() proc.join() Fails with the following error: Traceback (most recent call last): File "/home/antoine/miniconda3/envs/dask35/lib/python3.5/multiprocessing/forkserver.py", line 178, in main _serve_one(s, listener, alive_r, handler) File "/home/antoine/miniconda3/envs/dask35/lib/python3.5/multiprocessing/forkserver.py", line 212, in _serve_one code = spawn._main(child_r) File "/home/antoine/miniconda3/envs/dask35/lib/python3.5/multiprocessing/spawn.py", line 115, in _main prepare(preparation_data) File "/home/antoine/miniconda3/envs/dask35/lib/python3.5/multiprocessing/spawn.py", line 221, in prepare set_start_method(data['start_method']) File "/home/antoine/miniconda3/envs/dask35/lib/python3.5/multiprocessing/context.py", line 231, in set_start_method raise RuntimeError('context has already been set') RuntimeError: context has already been set This makes set_forkserver_preload() quite fragile if you preload any library that may create multiprocessing resources (such as locks) at the top level. |
|
|
| msg281566 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2016-11-23 15:27 |
| The example above works if you comment out either the "Lock()" line or the "set_forkserver_preload()" line. |
|
|
| msg281570 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2016-11-23 16:33 |
| Here is a patch. |
|
|
| msg281838 - (view) |
Author: Davin Potts (davin) * |
Date: 2016-11-28 00:25 |
| Antoine: I'm still unclear on what's going on here but I noticed something by accident when trying your example script. Specifically, when I ran your script from the directory where it exists on disk, I could successfully reproduce what you described but if I ran your script from a different working directory (e.g. 'python3.5 ../tmp/issue_28779_repro.py') then I did not see the misbehavior and it instead ran through to completion without any exceptions reported. I saw this phenomenon (all that I described above) on both OSX and Ubuntu 16.04 systems. Secondly, looking at the file you attached, did everything make it into the patch? I ask because unless I'm missing something it looks like the patch adds arguments to the function signature but does not act upon them in any new way? |
|
|
| msg281840 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2016-11-28 00:52 |
| > I noticed something by accident when trying your example script That's due to the way I wrote the script (the use of __file__ to deduce the module name to preload), not anything inherent in the bug described here. > looking at the file you attached, did everything make it into the patch? Yes. > I ask because unless I'm missing something it looks like the patch adds arguments to the function signature but does not act upon them in any new way? It does not. It's only fixing the signature of the method in the base class (which raises NotImplementedError) to match the signature of the method in the derived class (which is the only one actually called). |
|
|
| msg281841 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2016-11-28 00:56 |
| Slight mistake in my explanation: the method in the base class raises ValueError, not NotImplementedError. Still, the basic explanation stands. (that said, if you think this is out of scope for this issue, I can revert that part of the patch) |
|
|
| msg281948 - (view) |
Author: Davin Potts (davin) * |
Date: 2016-11-29 05:21 |
| I don't see any negative consequences for the helpers if the `force=True` is made in spawn.prepare's invocation of set_start_method(). In tracing backwards to figure out why this wasn't done already, it seems unchanged since sbt's patch in . This change may impact users of execution-bundling tools like cx-freeze -- suggests this could make freezing things easier for some. I admit I don't fully appreciate the details of how these tools are implemented. > (that said, if you think this is out of scope for this issue, I can revert that part of the patch) Given the nature of BaseContext's implementation, I don't see a problem with keeping your change. In case I was missing something, I spent some time searching for code possibly depending upon BaseContext.set_start_method's calling arguments but turned up nothing (no surprises). It feels cleaner to update it to be in sync with DefaultContext. Overall, LGTM the way you have it. |
|
|
| msg282663 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2016-12-07 20:35 |
| I will probably commit this in the coming days. |
|
|
| msg282857 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2016-12-10 16:22 |
| New changeset 1a955981b263 by Antoine Pitrou in branch '3.5': Issue #28779: multiprocessing.set_forkserver_preload() would crash the forkserver process if a preloaded module instantiated some multiprocessing objects such as locks. https://hg.python.org/cpython/rev/1a955981b263 New changeset f3b9fd41b5cb by Antoine Pitrou in branch '3.6': Issue #28779: multiprocessing.set_forkserver_preload() would crash the forkserver process if a preloaded module instantiated some multiprocessing objects such as locks. https://hg.python.org/cpython/rev/f3b9fd41b5cb New changeset 5456b699788f by Antoine Pitrou in branch 'default': Issue #28779: multiprocessing.set_forkserver_preload() would crash the forkserver process if a preloaded module instantiated some multiprocessing objects such as locks. https://hg.python.org/cpython/rev/5456b699788f |
|
|
| msg282858 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2016-12-10 16:22 |
| Now fixed, closing. |
|
|