[issue23051] multiprocessing.pool methods imap()[_unordered()] deadlock

[Gregory Szorc]

Gregory Szorc added the comment:

For posterity, I think we ran into a similar problem in 
https://bugzilla.mozilla.org/show_bug.cgi?id=1191877, where our code uses 
apply_async():

11:09:47 INFO -  Exception in thread Thread-2:
11:09:47 INFO -  Traceback (most recent call last):
11:09:47 INFO -File "/tools/python/lib/python2.7/threading.py", line 
551, in __bootstrap_inner
11:09:47 INFO -  self.run()
11:09:47 INFO -File "/tools/python/lib/python2.7/threading.py", line 
504, in run
11:09:47 INFO -  self.__target(*self.__args, **self.__kwargs)
11:09:47 INFO -File 
"/tools/python/lib/python2.7/multiprocessing/pool.py", line 319, in 
_handle_tasks
11:09:47 INFO -  put(task)
11:09:47 INFO -  RuntimeError: dictionary changed size during iteration

This resulted in deadlock, just like this issue.

The added try..except around the iteration of taskseq likely fixes this as well.

--
nosy: +Gregory.Szorc

___
Python tracker 

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

[issue23051] multiprocessing.pool methods imap()[_unordered()] deadlock

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Thank you for your contribution Alon and Davin.

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

[issue23051] multiprocessing.pool methods imap()[_unordered()] deadlock

[Roundup Robot]

Roundup Robot added the comment:

New changeset 525ccfcc55f7 by Serhiy Storchaka in branch '3.4':
Issue #23051: multiprocessing.Pool methods imap() and imap_unordered() now
https://hg.python.org/cpython/rev/525ccfcc55f7

New changeset 7891d084a9ad by Serhiy Storchaka in branch 'default':
Issue #23051: multiprocessing.Pool methods imap() and imap_unordered() now
https://hg.python.org/cpython/rev/7891d084a9ad

New changeset 311d52878a65 by Serhiy Storchaka in branch '2.7':
Issue #23051: multiprocessing.Pool methods imap() and imap_unordered() now
https://hg.python.org/cpython/rev/311d52878a65

--
nosy: +python-dev

___
Python tracker 

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

[issue23051] multiprocessing.pool methods imap()[_unordered()] deadlock

[Davin Potts]

Davin Potts added the comment:

After pondering it for two days and coming back to it with hopefully "fresh 
eyes", I believe that changing the for-loop to a while-loop is not overall 
easier to understand -- I would lean towards keeping the for-loop.

I do think the change to the while-loop very much made the exception handling 
logic clearer but seeing the while-loop with the "manual" invocation of next 
and incrementing of the variable i and re-use of i as a signal to break out of 
a loop (setting i = None) made other things less clear.  My belief is that 
someone reading this method's code for the first time will read the for-loop 
version as, "try to loop through the enumerated tasks and if anything goes 
wrong then set the next position in the cache to 'failed'".  That top-level 
reading is, I think, not quite as easy with the while-loop.  Without the 
exception handling that we add in this patch, the original code used the 
for-loop and would, I think, have looked weird if it had tried to use a 
while-loop -- I think that's a sign that the for-loop is likely to be more 
easily understood by a first-time reader.

Though I am not sure it really matters, the while-loop version would only help 
end the processing of further jobs if an exception occurs in the iterator 
whereas the for-loop version might help if exceptions occur in a couple of 
other places.  We do not have a clear motivation for needing that however.

--

___
Python tracker 

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

[issue23051] multiprocessing.pool methods imap()[_unordered()] deadlock

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

May be the code would cleaner when convert the "for" loop to the "while" loop 
and wrap in try/except only next()?

--
Added file: http://bugs.python.org/file38377/issue_23051_4-3.4.patch

___
Python tracker 

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

[issue23051] multiprocessing.pool methods imap()[_unordered()] deadlock

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
assignee:  -> serhiy.storchaka

___
Python tracker 

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

[issue23051] multiprocessing.pool methods imap()[_unordered()] deadlock

[Davin Potts]

Changes by Davin Potts :


Added file: 
http://bugs.python.org/file38363/issue_23051_revised_fix_and_tests_v27.patch

___
Python tracker 

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

[issue23051] multiprocessing.pool methods imap()[_unordered()] deadlock

[Davin Potts]

Davin Potts added the comment:

Updated (1) the patch for default/3.5 and 3.4 and (2) the patch for 2.7 to 
reflect recommendations from the review.

Thanks goes to Serhiy for the helpful review and especially the suggestion on 
better future-proofing in the tests.

--
Added file: 
http://bugs.python.org/file38362/issue_23051_revised_fix_and_tests_v35_and_v34.patch

___
Python tracker 

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

[issue23051] multiprocessing.pool methods imap()[_unordered()] deadlock

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Added comments on Rietveld.

--
nosy: +serhiy.storchaka
stage: commit review -> patch review

___
Python tracker 

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

[issue23051] multiprocessing.pool methods imap()[_unordered()] deadlock

[Davin Potts]

Davin Potts added the comment:

For my part, I'm now good with all aspects of the patch supplied by Alon.

I have a working set of tests that will be attached in the next day or two 
after seeing them behave on more than one platform.

--
stage:  -> test needed

___
Python tracker 

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

[issue23051] multiprocessing.pool methods imap()[_unordered()] deadlock

[Antoine Pitrou]

Antoine Pitrou added the comment:

The patch would at least need to add a unit test in order to avoid regressions.

--
nosy: +pitrou

___
Python tracker 

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

[issue23051] multiprocessing.pool methods imap()[_unordered()] deadlock

[Davin Potts]

Davin Potts added the comment:

Successfully reproduced the behavior playing through variations with the 
supplied examples (very helpful) on OS X v10.9.5 both in 2.7.9 and the default 
branch -- adding version 3.5 to issue.

Also successfully verified that the supplied patches do just as advertised on 
same: OS X v10.9.5, both 2.7.9 and default branch.

The patches' addition of a try around that for block looks like a sensible 
choice.  At the moment, I do not yet fully understand what the patches' 
exception block is doing with the 'cache[job]._set' call the way it is, but I 
would like to spend more time to digest it better -- I hope to do so later this 
week.

--
versions: +Python 3.5

___
Python tracker 

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

[issue23051] multiprocessing.pool methods imap()[_unordered()] deadlock

[Davin Potts]

Changes by Davin Potts :


--
nosy: +davin

___
Python tracker 

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

[issue23051] multiprocessing.pool methods imap()[_unordered()] deadlock

[Josh Rosenberg]

Changes by Josh Rosenberg :


--
nosy: +josh.r

___
Python tracker 

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

[issue23051] multiprocessing.pool methods imap()[_unordered()] deadlock

[Terry J. Reedy]

Changes by Terry J. Reedy :


--
title: multiprocessing.pool methods imap() and imap_unordered() cause deadlock 
-> multiprocessing.pool methods imap()[_unordered()] deadlock

___
Python tracker 

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