[Python-checkins] (no subject)

[Batuhan Taşkaya]

To: python-checkins at python.org
Subject: bpo-39360: Ensure all workers exit when finalizing a multiprocessing
 Pool (GH-19009)
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

https://github.com/python/cpython/commit/ac10e0c93218627d1a639db0b7b41714c5f6=
a883
commit: ac10e0c93218627d1a639db0b7b41714c5f6a883
branch: master
author: Batuhan Ta=C5=9Fkaya <47358913+isidentical at users.noreply.github.com>
committer: GitHub <noreply at github.com>
date: 2020-03-15T19:45:56Z
summary:

bpo-39360: Ensure all workers exit when finalizing a multiprocessing Pool (GH=
-19009)

When the pull is not used via the context manager or terminate() is called, t=
here is a system in multiprocessing.util that handles finalization of all poo=
ls via an atexit handler (the Finalize) class. This class registers the _term=
inate_pool handler in the registry of finalizers of the module, and that regi=
stry is called on interpreter exit via _exit_function. The problem is that th=
e "happy" path with the context manager or manual call to finalize() does som=
e extra steps that _terminate_pool does not. The step that is not executed wh=
en the atexit() handler calls _terminate_pool is pinging the _change_notifier=
 queue to unblock the maintenance threads.

This commit moves the notification to the _terminate_pool function so is call=
ed from both code paths.

Co-authored-by: Pablo Galindo <Pablogsal at gmail.com>

files:
A Misc/NEWS.d/next/Library/2020-03-15-05-41-05.bpo-39360.cmcU5p.rst
M Lib/multiprocessing/pool.py
M Lib/test/_test_multiprocessing.py

diff --git a/Lib/multiprocessing/pool.py b/Lib/multiprocessing/pool.py
index b223d6aa724bb..41dd923d4f974 100644
--- a/Lib/multiprocessing/pool.py
+++ b/Lib/multiprocessing/pool.py
@@ -651,8 +651,6 @@ def close(self):
     def terminate(self):
         util.debug('terminating pool')
         self._state =3D TERMINATE
-        self._worker_handler._state =3D TERMINATE
-        self._change_notifier.put(None)
         self._terminate()
=20
     def join(self):
@@ -682,7 +680,12 @@ def _terminate_pool(cls, taskqueue, inqueue, outqueue, p=
ool, change_notifier,
         # this is guaranteed to only be called once
         util.debug('finalizing pool')
=20
+        # Notify that the worker_handler state has been changed so the
+        # _handle_workers loop can be unblocked (and exited) in order to
+        # send the finalization sentinel all the workers.
         worker_handler._state =3D TERMINATE
+        change_notifier.put(None)
+
         task_handler._state =3D TERMINATE
=20
         util.debug('helping task handler/workers to finish')
diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessi=
ng.py
index b985d51508cb7..4a87b1761f9ef 100644
--- a/Lib/test/_test_multiprocessing.py
+++ b/Lib/test/_test_multiprocessing.py
@@ -2780,6 +2780,24 @@ def test_pool_worker_lifetime_early_close(self):
         for (j, res) in enumerate(results):
             self.assertEqual(res.get(), sqr(j))
=20
+    def test_worker_finalization_via_atexit_handler_of_multiprocessing(self):
+        # tests cases against bpo-38744 and bpo-39360
+        cmd =3D '''if 1:
+            from multiprocessing import Pool
+            problem =3D None
+            class A:
+                def __init__(self):
+                    self.pool =3D Pool(processes=3D1)
+            def test():
+                global problem
+                problem =3D A()
+                problem.pool.map(float, tuple(range(10)))
+            if __name__ =3D=3D "__main__":
+                test()
+        '''
+        rc, out, err =3D test.support.script_helper.assert_python_ok('-c', c=
md)
+        self.assertEqual(rc, 0)
+
 #
 # Test of creating a customized manager class
 #
diff --git a/Misc/NEWS.d/next/Library/2020-03-15-05-41-05.bpo-39360.cmcU5p.rs=
t b/Misc/NEWS.d/next/Library/2020-03-15-05-41-05.bpo-39360.cmcU5p.rst
new file mode 100644
index 0000000000000..148878886e6ee
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2020-03-15-05-41-05.bpo-39360.cmcU5p.rst
@@ -0,0 +1,4 @@
+Ensure all workers exit when finalizing a :class:`multiprocessing.Pool` impl=
icitly via the module finalization
+handlers of multiprocessing. This fixes a deadlock situation that can be exp=
erienced when the Pool is not
+properly finalized via the context manager or a call to ``multiprocessing.Po=
ol.terminate``. Patch by Batuhan Taskaya
+and Pablo Galindo.