[issue31092] multiprocessing.Manager() race condition

2017-10-06 Thread Oren Milman

Oren Milman  added the comment:

Davin and Antoine, i added you to the nosy list because you are listed
as multiprocessing experts :)

--
nosy: +davin, pitrou

___
Python tracker 

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



[issue31092] multiprocessing.Manager() race condition

2017-10-05 Thread Prof Plum

Prof Plum  added the comment:

Oh I see, I thought getting an error that caused the python code execution to 
terminate was considered a "crash".

On the note of whether you should fix this I think the answer is yes. When I 
call pool.apply_async() I expect it only to return when the worker process has 
been started and finished it's initialization process (i.e. sending the 
incr-ref request). That being said I could see people wanting to utilize the 
minor performance gain of having the worker start AND run asynchronously so I 
think this option should be available via a boolean arg to apply_async() but it 
should be off by default because that is the safer and intuitive behavior of 
apply_async().

--

___
Python tracker 

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



[issue31092] multiprocessing.Manager() race condition

2017-10-05 Thread Oren Milman

Oren Milman  added the comment:

Prof Plum, i changed the type of the issue to 'behavior', because Lang and me
both got a KeyError. if your interpreter actually crashed, please change it
back to 'crash'.

--

___
Python tracker 

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



[issue31092] multiprocessing.Manager() race condition

2017-10-05 Thread Oren Milman

Oren Milman  added the comment:

IIUC:
In Lang's example, doing `queue = None` caused the destruction of the shared
queue, which caused a call to BaseProxy._decref() (in 
multiprocessing/managers.py),
which dispatched a decref request to the manager's server process.

Meanwhile, the pool's worker process (in function worker() in 
multiprocessing/pool.py)
tries to retrieve a task from its task queue, by calling inqueue.get().
The get() method unpickles the first pickled task in the queue, which is the
function and arguments that we passed to apply_async().
The unpickling of the shared queue causes creating a proxy object for the
shared queue, in which BaseProxy.__init__() is called, which calls
BaseProxy._incref(), which dispatches an incref request to the manager's server
process.

Unfortunately, the decref request gets to the server before the incref request.
So when the server receives the decref request (in Server.handle_request()),
and accordingly calls Server.decref(), the refcount of the shared queue in the
server is 1, so the refcount is decremented to 0, and the shared queue is
disposed.
Then, when the server receives the incref request, it tries to increment the
refcount of the shared queue (in Server.incref()), but can't find it in its
refcount dict, so it raises the KeyError.
(If, for example, you added a 'sleep(0.5)' before the call to dispatch() in
BaseProxy._decref(), the incref request would win the race, and the KeyError
wouldn't be raised.)


Should we fix this?
Or is it the responsibility of the user to not destroy shared objects too soon?
(In that case, maybe we should mention it in the docs?)


The situation in the example of Prof Plum is similar.
Also, note that this issue is not specific to using pool workers or to
Manager.Queue. For example, we get a similar error (for similar reasons) in
this code:

from multiprocessing import Process, Manager
from time import sleep
if __name__ == '__main__':
with Manager() as manager:
shared_list = manager.list()
p = Process(target=sorted, args=(shared_list,))
p.start()
# sleep(0.5)
shared_list = None
p.join()

--
components: +Library (Lib)
nosy: +Oren Milman
type: crash -> behavior
versions: +Python 3.7

___
Python tracker 

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



[issue31092] multiprocessing.Manager() race condition

2017-09-22 Thread Lang

Lang added the comment:

# code reproduce bug
# KeyError in lib\multiprocessing\managers.py in incref 

import multiprocessing as mp
from time import sleep

def func(queue):
pass

if __name__ == '__main__':
manager = mp.Manager()

pool = mp.Pool(1)

queue = manager.Queue()
r = pool.apply_async(func, args = [queue])
#sleep(1)
queue = None

pool.close()
pool.join()

--
nosy: +tlxxzj

___
Python tracker 

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



[issue31092] multiprocessing.Manager() race condition

2017-09-21 Thread Prof Plum

Changes by Prof Plum :


--
title: Potential multiprocessing.Manager() race condition -> 
multiprocessing.Manager() race condition

___
Python tracker 

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