From ef8bff34231bb9c74fe349905bc29b5be8a48325 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Batuhan=20Ta=C5=9Fkaya?= Date: Sun, 15 Mar 2020 22:45:56 +0300 Subject: [PATCH] bpo-39360: Ensure all workers exit when finalizing a multiprocessing Pool (GH-19009) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the pull is not used via the context manager or terminate() is called, there is a system in multiprocessing.util that handles finalization of all pools via an atexit handler (the Finalize) class. This class registers the _terminate_pool handler in the registry of finalizers of the module, and that registry is called on interpreter exit via _exit_function. The problem is that the "happy" path with the context manager or manual call to finalize() does some extra steps that _terminate_pool does not. The step that is not executed when 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 called from both code paths. Co-authored-by: Pablo Galindo (cherry picked from commit ac10e0c93218627d1a639db0b7b41714c5f6a883) Co-authored-by: Batuhan TaÅŸkaya --- Lib/multiprocessing/pool.py | 7 +++++-- Lib/test/_test_multiprocessing.py | 18 ++++++++++++++++++ .../2020-03-15-05-41-05.bpo-39360.cmcU5p.rst | 4 ++++ 3 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2020-03-15-05-41-05.bpo-39360.cmcU5p.rst diff --git a/Lib/multiprocessing/pool.py b/Lib/multiprocessing/pool.py index b223d6aa724bb6..41dd923d4f9740 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 = TERMINATE - self._worker_handler._state = TERMINATE - self._change_notifier.put(None) self._terminate() def join(self): @@ -682,7 +680,12 @@ def _terminate_pool(cls, taskqueue, inqueue, outqueue, pool, change_notifier, # this is guaranteed to only be called once util.debug('finalizing pool') + # 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 = TERMINATE + change_notifier.put(None) + task_handler._state = TERMINATE util.debug('helping task handler/workers to finish') diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index de912428737f9a..e64a8e9b3b8793 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -2778,6 +2778,24 @@ def test_pool_worker_lifetime_early_close(self): for (j, res) in enumerate(results): self.assertEqual(res.get(), sqr(j)) + def test_worker_finalization_via_atexit_handler_of_multiprocessing(self): + # tests cases against bpo-38744 and bpo-39360 + cmd = '''if 1: + from multiprocessing import Pool + problem = None + class A: + def __init__(self): + self.pool = Pool(processes=1) + def test(): + global problem + problem = A() + problem.pool.map(float, tuple(range(10))) + if __name__ == "__main__": + test() + ''' + rc, out, err = test.support.script_helper.assert_python_ok('-c', cmd) + 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.rst b/Misc/NEWS.d/next/Library/2020-03-15-05-41-05.bpo-39360.cmcU5p.rst new file mode 100644 index 00000000000000..148878886e6ee5 --- /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` implicitly via the module finalization +handlers of multiprocessing. This fixes a deadlock situation that can be experienced when the Pool is not +properly finalized via the context manager or a call to ``multiprocessing.Pool.terminate``. Patch by Batuhan Taskaya +and Pablo Galindo.