[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Josh Rosenberg]

Josh Rosenberg  added the comment:

It should probably be backport to all supported 3.x branches though, so people 
aren't required to move to 3.8 to benefit from it.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Josh Rosenberg]

Josh Rosenberg  added the comment:

Pablo's fix looks like a superset of the original fix applied here, so I'm 
assuming it fixes this issue as well.

--
nosy: +josh.r

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:

What's the status of this issue?

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Vy Nguyen]

Change by Vy Nguyen :


--
nosy: +Vy Nguyen

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:

Pablo fixed bpo-35378 with:

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

Does this change also fix this issue? If not, can we attempt again to fix this 
issue?

Moreover, should we do something in Python 3.7? Sadly, I don't think that we 
can do anything for 3.7 and 2.7.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:

multiprocessing.Pool destructor now emits a ResourceWarning if it is still 
running: if .close() nor .terminate() have been called, see bpo- 35424.

It is a first alarm that the problematic example is wrong.

Should reconsider to fix this bug in the master branch? If yes, we should 
carefully document this backward incompatible change.

--

__
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:

The new test_del_pool() test of the fix failed on a buildbot: bpo-35413 
"test_multiprocessing_fork: test_del_pool() leaks dangling threads and 
processes on AMD64 FreeBSD CURRENT Shared 3.x".

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:

I started a thread on python-dev to discuss these issues:
https://mail.python.org/pipermail/python-dev/2018-December/155946.html

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:

See also bpo-35424: "multiprocessing.Pool: emit ResourceWarning".

I wrote 10986 to fix 2 tests which leak resources.

I have a question. Why do tests have to call "pool.join()" after "with pool:"? 
When I use a file, I know that the resources are released after "with file:".

Should Pool.__exit__() call Pool.join()?

This question reminds me my fix in socketserver (bpo-31151 and bpo-31233) which 
leaked processes and threads, and my bug bpo-34037 (asyncio leaks threads).

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Antoine Pitrou]

Antoine Pitrou  added the comment:

I agree that reverting in bugfix branches was the right thing to do.  I think 
the fix should have remained in master, though.

--
stage: patch review -> 
versions:  -Python 2.7, 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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:

I reverted the change in 2.7, 3.6, 3.7 and master branches because it 
introduces a regression and we are very close to a release:
https://mail.python.org/pipermail/python-dev/2018-December/155920.html

I don't want to have the pressure to push a quick fix. I would like to make 
sure that we have enough time to design a proper fix. I'm not saying that 
Pablo's fix is not correct, it's just bad timing.

This bug is likely here for a long time, so I think that it's ok to still have 
it in the next 3.6 and 3.7 bugfix releases.

I suggest to open a discussion on the python-dev mailing list about 
multiprocessing relying on the garbage collector and lifetime of 
multiprocessing objects (Pool, Process, result, etc.). It seems like I disagree 
with Pablo and tzickel, whereas Armin Rigo (PyPy which has a different GC) is 
more on my side (release explicitly resources) :-) I would prefer to move 
towards explicit resource managment instead of relying on destructors and the 
garbage collector. For example, it's a bad practice to rely on these when using 
PyPy.

See my previous comments about issues related to multiprocessing objects 
lifetime.

--
priority: release blocker -> 

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 9dfc754d61c55a526304e10a328bad448efa9ee9 by Victor Stinner in 
branch 'master':
Revert "bpo-34172: multiprocessing.Pool leaks resources after being deleted 
(GH-8450)" (GH-10971)
https://github.com/python/cpython/commit/9dfc754d61c55a526304e10a328bad448efa9ee9


--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 358fc87f53cf97a1768d5b1ded08f2a564f9fd85 by Victor Stinner in 
branch '2.7':
Revert "[2.7] bpo-34172: multiprocessing.Pool leaks resources after being 
deleted (GH-9686)" (GH-10970)
https://github.com/python/cpython/commit/358fc87f53cf97a1768d5b1ded08f2a564f9fd85


--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:


New changeset eb38ee052e2273568d0041e969aa851ee44e43ce by Victor Stinner in 
branch '3.6':
[3.6] Revert "bpo-34172: multiprocessing.Pool leaks resources after being 
deleted (GH-8450) (GH-9677)" (GH-10969)
https://github.com/python/cpython/commit/eb38ee052e2273568d0041e969aa851ee44e43ce


--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 3c6b0d967eb4c95e06c4f1beddfca4f6300d92ce by Victor Stinner in 
branch '3.7':
[3.7] Revert "bpo-34172: multiprocessing.Pool leaks resources after being 
deleted (GH-8450) (GH-9676)" (#10968)
https://github.com/python/cpython/commit/3c6b0d967eb4c95e06c4f1beddfca4f6300d92ce


--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +10213

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +10212

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +10211

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +10210
stage: resolved -> patch review

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:

Another example of complex issue related to object lifetime, resources (file 
descriptors) and multiprocessing: bpo-30966, add SimpleQueue.close().

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:

multiprocessing should help the developer to detect when the API is misused. 
For example, emit a ResourceWarning if a pool is not released explicitly. It 
might help to detect such bugs:

* bpo-33676
* bpo-35413

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

> that there is code in the stdlib that holdes reference between child and 
> parent

Just to clarify: is not that is just code in the stdlib that keeps a reference 
between child and parent. The examples I have given are the exact same 
situation that we have here: the iterator object associated with another needs 
to keep its parent alive to work correctly.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[tzickel]

tzickel  added the comment:

Reverting the code will cause another class of problems, like the reason I 
fixed it. Programs written such as the example that Pablo gave (and what I've 
seen) will quietly leak child processes, file descriptors (for the pipes) and 
memory to a variety degree might not be detected, or in the end detected in a 
big error or crash.

Also in some ResourceWarnings (if not all), the resources are closed in the end 
(like in sockets), here without this code patch you cannot implicitly reclaim 
the resources (because there is a Thread involved here), which I think is a 
high bar for the user to think about.

You can also enable multiprocessing's debug logging to see how the code behaves 
with and without the fix:
https://stackoverflow.com/a/1353037

I also agree with Pablo that there is code in the stdlib that holdes reference 
between child and parent. There is also code that has circular reference (for 
example Python 2's OrderedDict) and that is ok as well (not that this is the 
situation here).

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

> I disagree that a child should keep its parent alive.

But this is normal across the standard library. For example, here is how a 
deque iterator keeps the deque alive:

>>> x = deque([1,2,3])
>>> deque_iter = iter(x)
>>> deque_weakref = weakref.ref(x)
>>> del x
>>> gc.collect()
>>> gc.get_referrers(deque_weakref())
[<_collections._deque_iterator object at 0x024447ED6EA8>]

Here, the deque iterator is the *only* reference to the deque. When we destroy 
it, the deque is destroyed:
>>> del deque_iter
>>> gc.collect()
>>> deque_weakref()
None

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:

> I found the weird code in the example in several projects. I have corrected 
> it to use the pool as a context manager or to call close(), but this means 
> that users are doing this and it used to work and not it does not: 
> technically is a regression.

That's why I'm asking for a revert :-) I suggest to revert this change 
immediately from 2.7, 3.6 and 3.7, and later see what can be done for master.

Even if we design carefully an API, there will be always someone to misuse it 
:-) I would prefer to stop promoting such bad code and find a transition to 
more correct code.

I disagree that a child should keep its parent alive.

I would be ok to use a *weak reference* from the child to the parent to detect 
when the parent goes away, and maybe trigger an action in that case. For 
example, raise an exception or log a warning.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

> I'm not comfortable with the fix. I cannot explain why but I feel like adding 
> a strong dependency from a child to its parent is going to lead to more bugs, 
> not less. It sounds like a recipe for reference cycles. Maybe I'm just plain 
> wrong.

The pool child objects (imap iterators, async results...etc) need to keep a 
reference to the parent because if not, the caller is in charge of doing that 
and that may lead to bugs. Is the same scenario as if I get a dictionary 
iterator and then I destroy my reference to the dictionary: if the iterator 
does not keep a reference to the parent (the dictionary) it will not be 
possible to be used in the future. Indeed, we can see that this is what happens:

I'm not comfortable with the fix. I cannot explain why but I feel like adding a 
strong dependency from a child to its parent is going to lead to more bugs, not 
less. It sounds like a recipe for reference cycles. Maybe I'm just plain wrong.

>>> x = {1:2}
>>> y = iter(x)
>>> gc.get_referrers(x)
[, 
{'__name__': '__main__', '__doc__': None, '__package__': None, '__loader__': 
, '__spec__': None, 
'__annotations__': {}, '__builtins__': , 'y': 
, 'gc': , 'x': {1: 2}}
]

We can see that the dict_keyiterator refers to the dictionary, keeping it alive.

Here we have the same situation: if we do not keep the pool alive, the iterator 
will hang when iterating because the jobs won't get finished.

>At this point, I would like that someone explains to me what the problem is. 
>https://github.com/python/cpython/pull/10852 is a solution, ok, but what is 
>the problem? What does the code hangs, whereas >previously it was fine? Is the 
>example code really correct? Do we want to support such usage?

The code hangs because the pool was not being destroyed before due to the 
reference cycle between the pool and an internal object (a Thread). Now it 
hangs because the worker thread is destroyed with the pool as no references are 
kept to it and the job that the iterator is waiting on is never finished.

>I understand that msg330864 rely on black magic to expect that it's going to 
>be fine. The lifetime of the pool is implicit and it sounds like a bad design. 
>I don't want to endorse that.

I found the weird code in the example in several projects. I have corrected it 
to use the pool as a context manager or to call close(), but this means that 
users are doing this and it used to work and not it does not: technically is a 
regression.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


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

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

> I'm not comfortable with the fix. I cannot explain why but I feel like adding 
> a strong dependency from a child to its parent is going to lead to more bugs, 
> not less. It sounds like a recipe for reference cycles. Maybe I'm just plain 
> wrong.

The pool child objects (imap iterators, async results...etc) need to keep a 
reference to the parent because if not, the caller is in charge of doing that 
and that may lead to bugs. Is the same scenario as if I get a dictionary 
iterator and then I destroy my reference to the dictionary: if the iterator 
does not keep a reference to the parent (the dictionary) it will not be 
possible to be used in the future. Indeed, we can see that this is what happens:

>>> x = {1:2}
>>> y = iter(x)
>>> gc.get_referrents(x)
Traceback (most recent call last):
  File "", line 1, in 
AttributeError: module 'gc' has no attribute 'get_referrents'
>>> gc.get_referrers(x)
[, 
{'__name__': '__main__', '__doc__': None, '__package__': None, '__loader__': 
, '__spec__': None, 
'__annotations__': {}, '__builtins__': , 'y': 
, 'gc': , 'x': {1: 2}}
]

We can see that the dict_keyiterator refers to the dictionary, keeping it alive.

Here we have the same situation: if we do not keep the pool alive, the iterator 
will hang when iterating because the jobs won't get finished.

>At this point, I would like that someone explains to me what the problem is. 
>https://github.com/python/cpython/pull/10852 is a solution, ok, but what is 
>the problem? What does the code hangs, whereas >previously it was fine? Is the 
>example code really correct? Do we want to support such usage?

The code hangs because the pool was not being destroyed before due to the 
reference cycle between the pool and an internal object (a Thread). Now it 
hangs because the worker thread is destroyed with the pool as no references are 
kept to it and the job that the iterator is waiting on is never finished.

>I understand that msg330864 rely on black magic to expect that it's going to 
>be fine. The lifetime of the pool is implicit and it sounds like a bad design. 
>I don't want to endorse that.

I found the weird code in the example in several projects. I have corrected it 
to use the pool as a context manager or to call close(), but this means that 
users are doing this and it used to work and not it does not: technically is a 
regression.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


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

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

> I'm not comfortable with the fix. I cannot explain why but I feel like adding 
> a strong dependency from a child to its parent is going to lead to more bugs, 
> not less. It sounds like a recipe for reference cycles. Maybe I'm just plain 
> wrong.

The pool child objects (imap iterators, async results...etc) need to keep a 
reference to the parent because if not, the caller is in charge of doing that 
and that may lead to bugs. Is the same scenario as if I get a dictionary 
iterator and then I destroy my reference to the dictionary: if the iterator 
does not keep a reference to the parent (the dictionary) it will not be 
possible to be used in the future. Indeed, we can see that this is what happens:

I'm not comfortable with the fix. I cannot explain why but I feel like adding a 
strong dependency from a child to its parent is going to lead to more bugs, not 
less. It sounds like a recipe for reference cycles. Maybe I'm just plain wrong.

>>> x = {1:2}
>>> y = iter(x)
>>> gc.get_referrents(x)
Traceback (most recent call last):
  File "", line 1, in 
AttributeError: module 'gc' has no attribute 'get_referrents'
>>> gc.get_referrers(x)
[, 
{'__name__': '__main__', '__doc__': None, '__package__': None, '__loader__': 
, '__spec__': None, 
'__annotations__': {}, '__builtins__': , 'y': 
, 'gc': , 'x': {1: 2}}
]

We can see that the dict_keyiterator refers to the dictionary, keeping it alive.

Here we have the same situation: if we do not keep the pool alive, the iterator 
will hang when iterating because the jobs won't get finished.

>At this point, I would like that someone explains to me what the problem is. 
>https://github.com/python/cpython/pull/10852 is a solution, ok, but what is 
>the problem? What does the code hangs, whereas >previously it was fine? Is the 
>example code really correct? Do we want to support such usage?

The code hangs because the pool was not being destroyed before due to the 
reference cycle between the pool and an internal object (a Thread). Now it 
hangs because the worker thread is destroyed with the pool as no references are 
kept to it and the job that the iterator is waiting on is never finished.

>I understand that msg330864 rely on black magic to expect that it's going to 
>be fine. The lifetime of the pool is implicit and it sounds like a bad design. 
>I don't want to endorse that.

I found the weird code in the example in several projects. I have corrected it 
to use the pool as a context manager or to call close(), but this means that 
users are doing this and it used to work and not it does not: technically is a 
regression.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:

tzickel:
> A. The documentation explicitly says: "When the pool object is garbage 
> collected terminate() will be called immediately." (Happened till a code 
> refactor 9 years ago introduced this bug).

It is a *very bad* practice to rely on __del__. Don't do that. That's why we 
introduced ResourceWarning.


tzickel:
> https://github.com/python/cpython/blob/master/Lib/multiprocessing/util.py#L147
>  (Till the end of the file almost)

Is this API *incompatible* with pool.close()? Explicitly release resources?


Pablo:
> Víctor, I have a PR fixing this in: (...) The fix is really simple, so I 
> think we should fix it and not revert the change on this case.

I'm not comfortable with the fix. I cannot explain why but I feel like adding a 
strong dependency from a child to its parent is going to lead to more bugs, not 
less. It sounds like a recipe for reference cycles. Maybe I'm just plain wrong.

At this point, I would like that someone explains me what the problem is. 
https://github.com/python/cpython/pull/10852 is a solution, ok, but what is the 
problem? What does the code hangs, whereas previously it was fine? Is the 
example code really correct? Do we want to support such usage?

I understand that msg330864 rely on black magic to expect that it's going to be 
fine. The lifetime of the pool is implicit and it sounds like a bad design. I 
don't want to endorse that.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[tzickel]

tzickel  added the comment:

The previous posts here touch all this subjects:
A. The documentation explicitly says: "When the pool object is garbage 
collected terminate() will be called immediately." (Happened till a code 
refactor 9 years ago introduced this bug).

B. Large amount of code was developed for this technique:
https://github.com/python/cpython/blob/master/Lib/multiprocessing/util.py#L147 
(Till the end of the file almost)

C. The reason I opened this bug was because I was called to see why a long 
running process crashes after a while, and found out it leaked tons of 
subprocesses / pool._cache memory.

D. The quoted code, will currently simply leak each invocation lots of 
subprocesses... 

I too, think we should push for the said fix.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

Víctor, I have a PR fixing this in:

issue35378

Even if is not correct to not call close or join on the Pool, this behaviour 
was working before while now it hangs. The fix is really simple, si I think we 
should fix it and not revert the change on this case.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:

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

Side-note: Is it correct to use a pool without calling terminate() nor close()? 
Should we start to emit a ResourceWarning if a pool is not closed explicitly, 
as we did for files, sockets, asyncio event loops and subprocess.Popen objects?

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[STINNER Victor]

STINNER Victor  added the comment:

I suggest to revert this change, since it caused a regression: 
"multiprocessing.Pool.imap hangs in MacOs after applying this commit: (...)"

--
nosy: +benjamin.peterson, ned.deily, vstinner
priority: normal -> release blocker
resolution: fixed -> 
status: closed -> open

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

As this issue was somehow still present previous to this commit, I am going to 
track this problem in a new issue:

issue35378

--
status: open -> closed

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[tzickel]

tzickel  added the comment:

here is something quick I did to check if it works (it works) but I'm not 
fluent in multiprocessing code, so If i'm missing something or doing something 
wrong feel free to tell me:

https://github.com/tzickel/cpython/commit/ec63a43706f3bf615ab7ed30fb095607f6101e26

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[tzickel]

tzickel  added the comment:

A. It would be nice to add a test that tests this.
B. Now that Pool is cleaning up properly, any of it's functions which return 
another object (like imap's IMapIterator) need to hold a reference to the Pool, 
so it won't get cleanedup before computing.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

This also happens on Linux, hanging in different moments:

❯ ./python test.py
Begin
4
[hangs]

❯ ./python test.py
Begin
4
3
[hangs]

❯ ./python test.py
[hangs]

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

Bisecting between 3.7.0(good) and 3.7.1(bad) with the code in my previous 
commit points to:

97f998a4dfd6db6d867f446daa62445d0782bf39 is the first bad commit
commit 97f998a4dfd6db6d867f446daa62445d0782bf39
Author: Miss Islington (bot) <31488909+miss-isling...@users.noreply.github.com>
Date:   Tue Oct 2 14:17:04 2018 -0700

bpo-34172: multiprocessing.Pool leaks resources after being deleted 
(GH-8450) (GH-9676)

Fix a reference issue inside multiprocessing.Pool that caused the pool to 
remain alive if it was deleted without being closed or terminated explicitly.
(cherry picked from commit 97bfe8d3ebb0a54c8798f57555cb4152f9b2e1d0)

Co-authored-by: tzickel 

:04 04 90e0af29e82d0fcc1c1d7e19b3659f9602596e3e 
d997cfb00c1c44bbd87ce15edfd391203362a1d7 M  Lib
:04 04 6aa4273821c2c563ea69370a59284ed48576416f 
b6d46f14b6bb36cc5ae62b3ca74025c24d683bb5 M  Misc
bisect run success

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Pablo Galindo Salgado]

Pablo Galindo Salgado  added the comment:

multiprocessing.Pool.imap hangs in MacOs after applying this commit:

import multiprocessing

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

the_test()

This also happens in the backported branches.

--
nosy: +pablogsal
status: closed -> open

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Antoine Pitrou]

Change by Antoine Pitrou :


--
status: open -> closed

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Antoine Pitrou]

Antoine Pitrou  added the comment:


New changeset 4a7dd30f5810e8861a3834159a222ab32d5c97d0 by Antoine Pitrou 
(tzickel) in branch '2.7':
[2.7] bpo-34172: multiprocessing.Pool leaks resources after being deleted 
(GH-9686)
https://github.com/python/cpython/commit/4a7dd30f5810e8861a3834159a222ab32d5c97d0


--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Antoine Pitrou]

Change by Antoine Pitrou :


--
versions: +Python 2.7

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Antoine Pitrou]

Change by Antoine Pitrou :


--
status: closed -> open

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[tzickel]

tzickel  added the comment:

Its ok, you only did it twice :) I've submitted a manual 2.7 fix on GH.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[tzickel]

Change by tzickel :


--
pull_requests: +9072

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Antoine Pitrou]

Antoine Pitrou  added the comment:

(tzickel, sorry for mistyping your handle :-/)

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Thanks tzickler for the report and pull request, and sorry for the delay.

This is now fixed in all 3.x branches.  I will close this now as 
multiprocessing in 2.7 diverges quite a bit from 3.x.  If you want to fix the 
issue in 2.7 as well, please say so and I'll reopen.

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

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Antoine Pitrou]

Antoine Pitrou  added the comment:


New changeset 07b96a95db78eff3557d1bfed1df9ebecc40815b by Antoine Pitrou (Miss 
Islington (bot)) in branch '3.6':
bpo-34172: multiprocessing.Pool leaks resources after being deleted (GH-8450) 
(GH-9677)
https://github.com/python/cpython/commit/07b96a95db78eff3557d1bfed1df9ebecc40815b


--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Antoine Pitrou]

Antoine Pitrou  added the comment:


New changeset 97f998a4dfd6db6d867f446daa62445d0782bf39 by Antoine Pitrou (Miss 
Islington (bot)) in branch '3.7':
bpo-34172: multiprocessing.Pool leaks resources after being deleted (GH-8450) 
(GH-9676)
https://github.com/python/cpython/commit/97f998a4dfd6db6d867f446daa62445d0782bf39


--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[miss-islington]

Change by miss-islington :


--
pull_requests: +9064

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Antoine Pitrou]

Antoine Pitrou  added the comment:


New changeset 97bfe8d3ebb0a54c8798f57555cb4152f9b2e1d0 by Antoine Pitrou 
(tzickel) in branch 'master':
bpo-34172: multiprocessing.Pool leaks resources after being deleted (GH-8450)
https://github.com/python/cpython/commit/97bfe8d3ebb0a54c8798f57555cb4152f9b2e1d0


--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[miss-islington]

Change by miss-islington :


--
pull_requests: +9065

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Thanks a lot tzickle, I'll take a look.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[tzickel]

tzickel  added the comment:

It actually makes tons of sense that while the thread is running, that the 
object representing it is alive. After the thread finishes its work, the object 
dies.

>>> import time, threading, weakref, gc
>>> t = threading.Thread(target=time.sleep, args=(10,))
>>> wr = weakref.ref(t)
>>> t.start()
>>> del t
>>> gc.collect()
>>> wr()

Wait 10 seconds...
>>> gc.collect()
>>> wr()

The thread is gone (which doesn't happen with the pool).

Anyhow, I've submitted a patch to fix the bug that was introduced 9 years ago 
on GH, feel free to check it.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[tzickel]

Change by tzickel :


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

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Antoine Pitrou]

Antoine Pitrou  added the comment:

> What other object in the standard lib, leaks resources when deleted in 
> CPython ?

Threads come to mind, for example:

>>> import time, threading, weakref
>>> t = threading.Thread(target=time.sleep, args=(10,))
>>> t.start()
>>> wr = weakref.ref(t)
>>> del t
>>> wr()


Note I'm not against fixing this issue, just saying it's not that surprising 
for Pool to keep lingering around when you lost any user-visible reference to 
it.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[tzickel]

tzickel  added the comment:

I think I've found the code bug causing the leak:

https://github.com/python/cpython/blob/caa331d492acc67d8f4edd16542cebfabbbe1e79/Lib/multiprocessing/pool.py#L180

There is a circular reference between the Pool object, and the 
self._worker_handler Thread object (and it's also saved in the frame locals for 
the thread object, which prevents it from being garbage collected).

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[tzickel]

tzickel  added the comment:

What other object in the standard lib, leaks resources when deleted in CPython 
? Even that documentation says the garbage collector will eventually destroy 
it, just like here... I think there is an implementation bug.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Indeed, I think this simply needs a documentation fix.

--
assignee:  -> docs@python
components: +Documentation
nosy: +docs@python
versions: +Python 3.6, Python 3.8

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[mattip]

mattip  added the comment:

It would be sufficient to modify the documentation to reflect the code. 

There are other objects like `file` that recommend[0] calling a method to 
release resources without depending on implementation-specific details like 
garbage collection.

[0] 
https://docs.python.org/3/tutorial/inputoutput.html#reading-and-writing-files

--
nosy: +mattip

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Windson Yang]

Windson Yang  added the comment:

Add a __del__ method in the Pool class should work. But I'm not sure we should 
do this.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Windson Yang]

Change by Windson Yang :


--
nosy: +zach.ware

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[tzickel]

tzickel  added the comment:

But alas that does not work...

--
nosy: +davin, pitrou

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Windson Yang]

Windson Yang  added the comment:

A patch would just add 

def __del__(self):
self.terminate()

in the Pool object.

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[tzickel]

tzickel  added the comment:

>>> from multiprocessing import Pool
>>> import gc
>>> a = Pool(10)
>>> del a
>>> gc.collect()
0
>>>

After this, there are still left behind Process (Pool) or Dummy (ThreadPool) 
and big _cache data (If you did something with it) which lingers till the 
process dies.

You are correct on the other issue (I'm using and reading the Python 2 
documentation which does not have that...).

--

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[Windson Yang]

Windson Yang  added the comment:

> A. This does not happen, creating a Pool, deleting it and collecting the 
> garbage, does not call terminate.

Would you give me an example how you delete the Pool and collecting the 
garbage? If you use context manager, It will call terminate() function.

> B. The documentation for Pool itself does not specify it has a context 
> manager (but the examples show it).

You can find this info on the same page:

New in version 3.3: Pool objects now support the context management protocol – 
see Context Manager Types. __enter__() returns the pool object, and __exit__() 
calls terminate().

--
nosy: +Windson Yang

___
Python tracker 

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

[issue34172] multiprocessing.Pool and ThreadPool leak resources after being deleted

[tzickel]

New submission from tzickel :

In multiprocessing.Pool documentation it's written "When the pool object is 
garbage collected terminate() will be called immediately.":

https://docs.python.org/3.7/library/multiprocessing.html#multiprocessing.pool.Pool.terminate

A. This does not happen, creating a Pool, deleting it and collecting the 
garbage, does not call terminate.

B. The documentation for Pool itself does not specify it has a context manager 
(but the examples show it).

C. This bug is both in Python 3 & 2.

--
components: Library (Lib)
messages: 322028
nosy: tzickel
priority: normal
severity: normal
status: open
title: multiprocessing.Pool and ThreadPool leak resources after being deleted
type: behavior
versions: Python 2.7, Python 3.7

___
Python tracker 

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