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

STINNER Victor added the comment: Thanks Pablo for the fix ;-) -- ___ Python tracker ___ ___ Python-bugs-list mailing list

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

Change by Pablo Galindo Salgado : -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker ___

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

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

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

Change by Pablo Galindo Salgado : -- pull_requests: +11392, 11393, 11394 ___ Python tracker ___ ___ Python-bugs-list mailing list

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

Change by Pablo Galindo Salgado : -- pull_requests: +11392 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

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

Change by Pablo Galindo Salgado : -- pull_requests: +11392, 11393 ___ Python tracker ___ ___ Python-bugs-list mailing list

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

STINNER Victor added the comment: According to all previous discussions, I now agree with having a strong reference. -- ___ Python tracker ___

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

Change by Raymond Hettinger : -- nosy: +davin ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

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

tzickel added the comment: +1 -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

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

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

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

STINNER Victor added the comment: bpo-35629 has been marked as a duplicate of this issue. -- ___ Python tracker ___ ___

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

STINNER Victor added the comment: See also bpo-35478: multiprocessing: ApplyResult.get() hangs if the pool is terminated. -- ___ Python tracker ___

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

tzickel added the comment: https://bugs.python.org/issue35267 -- ___ Python tracker ___ ___ Python-bugs-list mailing list

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

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,

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

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

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

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!")

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

Change by Pablo Galindo Salgado : -- Removed message: https://bugs.python.org/msg331441 ___ Python tracker ___ ___ Python-bugs-list

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

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

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

Pablo Galindo Salgado added the comment: Ok, I will modify my PR to pass a weak reference and raise. -- ___ Python tracker ___ ___

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

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

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

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

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

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 ___

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

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

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

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

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

Change by Pablo Galindo Salgado : -- keywords: +patch pull_requests: +10087 stage: -> patch review ___ Python tracker ___ ___

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

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

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

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:

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

Pablo Galindo Salgado added the comment: I was working already on a PR. Do you prefer to wait for yours instead? -- ___ Python tracker ___

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

Change by Eric N. Vander Weele : -- nosy: +ericvw ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

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

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

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

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"]):