[issue28779] set_forkserver_preload() can crash the forkserver if preloaded module instantiate multiprocessing classes

[Donald Stufft]

Changes by Donald Stufft :


--
pull_requests: +1031

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28779] set_forkserver_preload() can crash the forkserver if preloaded module instantiate multiprocessing classes

[Antoine Pitrou]

Antoine Pitrou added the comment:

Now fixed, closing.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28779] set_forkserver_preload() can crash the forkserver if preloaded module instantiate multiprocessing classes

[Roundup Robot]

Roundup Robot added the comment:

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

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28779] set_forkserver_preload() can crash the forkserver if preloaded module instantiate multiprocessing classes

[Antoine Pitrou]

Antoine Pitrou added the comment:

I will probably commit this in the coming days.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28779] set_forkserver_preload() can crash the forkserver if preloaded module instantiate multiprocessing classes

[Davin Potts]

Davin Potts added the comment:

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 issue18999.


This change may impact users of execution-bundling tools like cx-freeze -- 
issue22255 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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28779] set_forkserver_preload() can crash the forkserver if preloaded module instantiate multiprocessing classes

[Antoine Pitrou]

Antoine Pitrou added the comment:

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)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28779] set_forkserver_preload() can crash the forkserver if preloaded module instantiate multiprocessing classes

[Antoine Pitrou]

Antoine Pitrou added the comment:

> 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).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28779] set_forkserver_preload() can crash the forkserver if preloaded module instantiate multiprocessing classes

[Davin Potts]

Davin Potts added the comment:

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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28779] set_forkserver_preload() can crash the forkserver if preloaded module instantiate multiprocessing classes

[Antoine Pitrou]

Changes by Antoine Pitrou :


Added file: http://bugs.python.org/file45611/forkserver_preload.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28779] set_forkserver_preload() can crash the forkserver if preloaded module instantiate multiprocessing classes

[Antoine Pitrou]

Changes by Antoine Pitrou :


Removed file: http://bugs.python.org/file45610/forkserver_preload.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28779] set_forkserver_preload() can crash the forkserver if preloaded module instantiate multiprocessing classes

[Antoine Pitrou]

Antoine Pitrou added the comment:

Here is a patch.

--
keywords: +patch
Added file: http://bugs.python.org/file45610/forkserver_preload.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28779] set_forkserver_preload() can crash the forkserver if preloaded module instantiate multiprocessing classes

[Antoine Pitrou]

Changes by Antoine Pitrou :


--
stage: needs patch -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28779] set_forkserver_preload() can crash the forkserver if preloaded module instantiate multiprocessing classes

[Antoine Pitrou]

Antoine Pitrou added the comment:

The example above works if you comment out either the "Lock()" line or the 
"set_forkserver_preload()" line.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue28779] set_forkserver_preload() can crash the forkserver if preloaded module instantiate multiprocessing classes

[Antoine Pitrou]

New submission from Antoine Pitrou:

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.

--
components: Library (Lib)
messages: 281565
nosy: davin, pitrou, sbt
priority: normal
severity: normal
stage: needs patch
status: open
title: set_forkserver_preload() can crash the forkserver if preloaded module 
instantiate multiprocessing classes
type: behavior
versions: Python 3.5, Python 3.6, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com