Skip to content

bpo-38501: Add a warning section to multiprocessing.Pool docs about resource managing #19466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 11, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion Doc/library/multiprocessing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,8 @@ process which created it.
>>> def f(x):
... return x*x
...
>>> p.map(f, [1,2,3])
>>> with p:
... p.map(f, [1,2,3])
Process PoolWorker-1:
Process PoolWorker-2:
Process PoolWorker-3:
Expand Down Expand Up @@ -2127,6 +2128,16 @@ with the :class:`Pool` class.
Note that the methods of the pool object should only be called by
the process which created the pool.

.. warning::
:class:`multiprocessing.pool` objects have internal resources that need to be
properly managed (like any other resource) by using the pool as a context manager
or by calling :meth:`close` and :meth:`terminate` manually. Failure to do this
can lead to the process hanging on finalization.

Note that is **not correct** to rely on the garbage colletor to destroy the pool
as CPython does not assure that the finalizer of the pool will be called
(see :meth:`object.__del__` for more information).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My potential fix is to use atexit.register when we can't use the context manager. Worth suggesting here? (Also let me know if it's clearly a bad idea for some reason!)

Copy link
Member Author

@pablogsal pablogsal Apr 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My potential fix is to use atexit.register when we can't use the context manager. Worth suggesting here? (Also let me know if it's clearly a bad idea for some reason!)

That works, but the problem is that is still a lossy method of resource managing (you would not use that method for all your file objects for instance). Technically, someone that is not "the interpreter" should own the pool and should be in charge of destroying it correctly.

I understand that some architecture and designs make this challenging so things like atexit do work for those cases, but I think is better not to explicitly mention that here so users don't start to use that as a silver bullet and use more the context manager for instance.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that makes sense. Otherwise, text LGTM and reads nicely in the rendered artifact!


.. versionadded:: 3.2
*maxtasksperchild*

Expand Down