[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[STINNER Victor]

STINNER Victor  added the comment:

Thanks Pablo for the fix ;-)

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:


New changeset 3766f18c524c57784eea7c0001602017d2122156 by Pablo Galindo in 
branch 'master':
bpo-35378: Fix multiprocessing.Pool references (GH-11627)
https://github.com/python/cpython/commit/3766f18c524c57784eea7c0001602017d2122156


--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
pull_requests: +11392, 11393, 11394

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
pull_requests: +11392

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
pull_requests: +11392, 11393

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[STINNER Victor]

STINNER Victor  added the comment:

According to all previous discussions, I now agree with having a strong 
reference.

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Raymond Hettinger]

Change by Raymond Hettinger :


--
nosy: +davin

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[tzickel]

tzickel  added the comment:

+1

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

I have been playing with possible solutions for a while and the weak-reference 
solution seems not robust enough as there are too potential race conditions 
between the destruction of the weakreferences (the pool) and the handling code.

I would advocate again for using a strong reference. The reasons are:

* The rest of the stdlib is using this solution to link iterator objects and 
similar to their parents (lists, dicts, sets...etc all have strong references 
back to their parents).

* This solution is backwards compatible with the current behaviour.

* We have the new ResourceWarnigns to make clear that this behaviour is not 
supported.

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[STINNER Victor]

STINNER Victor  added the comment:

bpo-35629 has been marked as a duplicate of this issue.

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[STINNER Victor]

STINNER Victor  added the comment:

See also bpo-35478: multiprocessing: ApplyResult.get() hangs if the pool is 
terminated.

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[tzickel]

tzickel  added the comment:

https://bugs.python.org/issue35267

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

Also, making tests that do not leak threads for this case is very difficult as 
if we go with weakrefs, the test *has* to leak a pool (the pool is dead but 
never calls join/close) to check that when you use the iterator the exception 
happens. Also, getting such test race-free is going to be challenging.

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

I am playing with weakreferences inside the iterator objects, but this may not 
be enough. For example, take the code of ApplyResult.get:

def get(self, timeout=None):
if self._pool() is None:
raise RuntimeError("The pool is dead!") <--- new code
self.wait(timeout)

It can be that the pool is alive when we check for it (self._pool() is None) 
but while the code is waiting with no timeout, the pool dies, effectively 
leaving the program deadlocked with no error.

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

I am playing with passing weakreferences into the iterator objects, but this 
may not be enough. For example, take the code of ApplyResult.get:

def get(self, timeout=None):
if self._pool() is None:
raise RuntimeError("The pool is dead!") <--- new code
self.wait(timeout)

It can be that the pool is alive when we check for it (self._pool() is None) 
but while the code is waiting with no timeout, the pool dies, effectively 
leaving the program deadlocked with no error.

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
Removed message: https://bugs.python.org/msg331441

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Antoine Pitrou]

Antoine Pitrou  added the comment:

I agree about making the multiprocessing API safer to use, but please let's 
have this discussion on a dedicated bug entry.

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

Ok, I will modify my PR to pass a weak reference and raise.

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[STINNER Victor]

STINNER Victor  added the comment:

See https://bugs.python.org/issue34172#msg331216 for a more general discussion 
about how the multiprocessing API is supposed tobe used and multiprocessing 
objects lifetime.

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Antoine Pitrou]

Antoine Pitrou  added the comment:

I think a weakref is fine.  You don't have to *pass* a weakref: just pass a 
normal ref and let the Result object take a weakref (and a reference to the 
cache perhaps).

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[STINNER Victor]

STINNER Victor  added the comment:

In general, I'm fine with adding a *weak* reference, especially if it helps to 
detect bugs.

--
nosy: +vstinner

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

Although I think keeping the iterator is not a bad solution if done correctly, 
I think more and more that is not the best solution.

@Antoine, would you be ok passing a weak reference to the iterator and raising 
if the pool is dead?

I still think we should try to avoid hanging.

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Let's step back a bit here.  This kind of code has never been supported.  As 
Victor says, we should be careful not to add any potential sources of reference 
cycles.

The reason the code originally "worked" is that it actually leaked the Pool 
object (and presumably its worker threads and processes) until the end of the 
application.

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
keywords: +patch
pull_requests: +10087
stage:  -> patch review

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

I think the code is almost ok (I also prefer to also use the cache as an excuse 
to maintain the pool alive) but the test needs to be done a bit more carefully 
to avoid hanging the test suite in case of failure and to avoid leaking threads 
or processes.

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[tzickel]

tzickel  added the comment:

I dont mind, I think my code is ready for review, but I'm not versed in this, 
so if you think you have something better, feel free to open a PR or tell me if 
I should submit mine, and you can comment on it:

https://github.com/python/cpython/compare/master...tzickel:fix34172

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

I was working already on a PR. Do you prefer to wait for yours instead?

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Eric N. Vander Weele]

Change by Eric N. Vander Weele :


--
nosy: +ericvw

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[tzickel]

tzickel  added the comment:

It's important to note that before those PR, that code would leak the Pool 
instance until the process ends (once per call).

https://github.com/python/cpython/compare/master...tzickel:fix34172

Is my proposed fix (till I get it to a PR).

--

___
Python tracker 

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

[issue35378] multiprocessing.Pool.imaps iterators do not maintain alive the multiprocessing.Pool objects

[Pablo Galindo Salgado]

New submission from Pablo Galindo Salgado :

After applying the PRs in issue34172, multiprocessing.Pool.imap hangs on MacOs 
and Linux. This is a simple reproducer:

import multiprocessing

def the_test():
print("Begin")
for x in multiprocessing.Pool().imap(int,
["4", "3"]):
print(x)
print("End")

the_test()

This happens because the IMapIterator does not maintain alive the 
multiprocessing.Pool object while it is still alive.

--
components: Library (Lib)
messages: 330890
nosy: pablogsal, pitrou, tzickel
priority: normal
severity: normal
status: open
title: multiprocessing.Pool.imaps iterators do not maintain alive the 
multiprocessing.Pool objects
versions: Python 3.7, Python 3.8

___
Python tracker 

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