This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: multiprocessing Queue leaks a file descriptor associated with the pipe writer (#33081 still a problem)
Type: resource usage Stage:
Components: Library (Lib) Versions: Python 3.10, Python 3.9, Python 3.8, Python 3.7, Python 3.6
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: crazycasta, davin, graingert, mattip, pitrou
Priority: normal Keywords: patch

Created on 2020-12-26 23:03 by crazycasta, last changed 2022-04-11 14:59 by admin.

Files
File name Uploaded Description Edit
test_queue.py crazycasta, 2020-12-26 23:03
queue_close_write.patch crazycasta, 2021-01-01 02:36
Messages (5)
msg383832 - (view) Author: Alex Orange (crazycasta) Date: 2020-12-26 23:03
Didn't feel like necroing #33081, but this is basically that problem. The trouble is the cleanup that appeared to fix #33081 only kicks in once something has been put in the queue. So if for instance a Process function puts something in the queue and the parent gets it, then calls q.close() the writer on the parent side doesn't get culled until the object does. This is particularly a problem for PyPy and isn't exactly great for any weird corner cases if anyone holds onto Queue objects after they're closed for any reason (horders!).

Attached file test_queue.py is an example of how to trigger this problem. Run it without a command line argument "python test_queue.py" and it won't crash (though it will take a very long time to complete). Run with an argument "python test_queue.py fail" and it will fail once you run out of file descriptors (one leaked per queue).

My suggestion on how to handle this is to set self._close to something that will close self._writer. Then, when _start_thread is called, instead of directly passing the self._writer.close object, pass a small function that will switch out self._close to the Finalize method used later on and return self._writer. Finally, inside _feed, use this method to get the _writer object and wrap the outer while 1 with a contextlib.closer on this object.

This is a fair bit of stitching things together here and there so let me know if anyone has any suggestions on this before I get started.
msg384149 - (view) Author: Alex Orange (crazycasta) Date: 2021-01-01 02:36
Well, having not heard anything I decided to just make a patch and throw it up. Here it is. This includes a test that will fail with the old version and passes once patched as well as the patch to the queue code itself.

Worth noting, the CleanExchange class is used because simpler things like using a closure to pass the exchange mechanism hold a reference to the Queue one way or another that is difficult/impossible to kill. This is because the intermediate thread mechanisms hold a reference to all the arguments that are passed to the run function. CleanExchange allows an indirect reference to be passed and for the reference to Queue to be None'd out.
msg384151 - (view) Author: mattip (mattip) * Date: 2021-01-01 06:35
Just to be clear, here is the code from the test (how do you format a code block here?) that demonstrates the writer is not closed when nothing is put into the queue

```
$ python3
Python 3.8.6 | packaged by conda-forge | (default, Oct  7 2020, 19:08:05) 
[GCC 7.5.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import multiprocessing
>>> q = multiprocessing.Queue()
>>> q.close()
>>> q.join_thread()
>>> q._reader.closed
True
>>> q._writer.closed
False
>>> 

```
And the changed behaviour to close the writer if the queue is used

```
>>> q = multiprocessing.Queue()
>>> q.put(1)
>>> q.get()
1
>>> q.close()
>>> q.join_thread()
>>> q._reader.closed
True
>>> q._writer.closed
True

```
msg384737 - (view) Author: mattip (mattip) * Date: 2021-01-09 18:42
In the expert index https://devguide.python.org/experts/ it lists @davin, @pitrou as referrents for multiprocessing. Adding then to the Nosy list
msg388632 - (view) Author: mattip (mattip) * Date: 2021-03-13 18:20
@crazycasta could you turn your patches into a PR? I am not sure how to get some eyes on this, but certainly the test is useful to prove the problem still exits
History
Date User Action Args
2022-04-11 14:59:39adminsetgithub: 86918
2022-02-21 11:50:18vstinnersetnosy: - vstinner
2022-02-17 16:37:30graingertsetnosy: + vstinner, graingert
2021-03-13 18:20:37mattipsetmessages: + msg388632
2021-01-21 11:37:55mattipsetnosy: + pitrou, davin
2021-01-09 18:42:06mattipsetmessages: + msg384737
2021-01-01 06:35:59mattipsetnosy: + mattip
messages: + msg384151
2021-01-01 02:36:59crazycastasetfiles: + queue_close_write.patch
keywords: + patch
messages: + msg384149
2020-12-26 23:03:41crazycastacreate