From 77af355b24f8cae1cb43e62d30c4447c6ba58f1c Mon Sep 17 00:00:00 2001 From: nullptr <3621629+0x0L@users.noreply.github.com> Date: Fri, 27 Aug 2021 23:13:41 +0200 Subject: [PATCH 1/4] bpo-45021: Fix a hang in forked children _global_shutdown_lock should be reinitialized in forked children --- Lib/concurrent/futures/thread.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Lib/concurrent/futures/thread.py b/Lib/concurrent/futures/thread.py index b7a2cac7f57015..80226b634ff0a3 100644 --- a/Lib/concurrent/futures/thread.py +++ b/Lib/concurrent/futures/thread.py @@ -36,6 +36,12 @@ def _python_exit(): # See bpo-39812 for context. threading._register_atexit(_python_exit) +# Make sure `_global_shutdown_lock` is reinit in forked children +if hasattr(os, 'register_at_fork'): + os.register_at_fork(before=_global_shutdown_lock.acquire, + after_in_child=_global_shutdown_lock._at_fork_reinit, + after_in_parent=_global_shutdown_lock.release) + class _WorkItem(object): def __init__(self, future, fn, args, kwargs): From fc006bf2a6570f587e379b9f7343959653f6c399 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sat, 28 Aug 2021 13:00:13 +0000 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../NEWS.d/next/Library/2021-08-28-13-00-12.bpo-45021.rReeaj.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2021-08-28-13-00-12.bpo-45021.rReeaj.rst diff --git a/Misc/NEWS.d/next/Library/2021-08-28-13-00-12.bpo-45021.rReeaj.rst b/Misc/NEWS.d/next/Library/2021-08-28-13-00-12.bpo-45021.rReeaj.rst new file mode 100644 index 00000000000000..54fd9109a9ae5f --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-08-28-13-00-12.bpo-45021.rReeaj.rst @@ -0,0 +1 @@ +Fix a potential deadlock at shutdown of forked children when using :mod:`concurrent.futures` module \ No newline at end of file From 75ac805d297ba340f44cb279686df41035af4ba0 Mon Sep 17 00:00:00 2001 From: nullptr <3621629+0x0L@users.noreply.github.com> Date: Tue, 31 Aug 2021 15:17:07 +0200 Subject: [PATCH 3/4] adding test --- Lib/test/test_concurrent_futures.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Lib/test/test_concurrent_futures.py b/Lib/test/test_concurrent_futures.py index 99651f5f4ed4da..4fce49cd74d22c 100644 --- a/Lib/test/test_concurrent_futures.py +++ b/Lib/test/test_concurrent_futures.py @@ -568,6 +568,19 @@ def test_shutdown_no_wait(self): # shutdown. assert all([r == abs(v) for r, v in zip(res, range(-5, 5))]) + def test_hang_issue45021(self): + """https://bugs.python.org/issue45021""" + def submit(pool): + pool.submit(submit, pool) + + if hasattr(os, 'register_at_fork'): + with futures.ThreadPoolExecutor(1) as pool: + pool.submit(submit, pool) + + for _ in range(50): + with futures.ProcessPoolExecutor(1, mp_context=get_context('fork')) as workers: + workers.submit(tuple) + create_executor_tests(ProcessPoolShutdownTest, executor_mixins=(ProcessPoolForkMixin, From 02891f98ddf8b36063f8b772e625433aa0517934 Mon Sep 17 00:00:00 2001 From: nullptr <3621629+0x0L@users.noreply.github.com> Date: Mon, 13 Sep 2021 19:17:51 +0200 Subject: [PATCH 4/4] tidy --- Lib/concurrent/futures/thread.py | 2 +- Lib/test/test_concurrent_futures.py | 27 ++++++++++++++------------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/Lib/concurrent/futures/thread.py b/Lib/concurrent/futures/thread.py index 80226b634ff0a3..51c942f51abd37 100644 --- a/Lib/concurrent/futures/thread.py +++ b/Lib/concurrent/futures/thread.py @@ -36,7 +36,7 @@ def _python_exit(): # See bpo-39812 for context. threading._register_atexit(_python_exit) -# Make sure `_global_shutdown_lock` is reinit in forked children +# At fork, reinitialize the `_global_shutdown_lock` lock in the child process if hasattr(os, 'register_at_fork'): os.register_at_fork(before=_global_shutdown_lock.acquire, after_in_child=_global_shutdown_lock._at_fork_reinit, diff --git a/Lib/test/test_concurrent_futures.py b/Lib/test/test_concurrent_futures.py index 4fce49cd74d22c..2703e1e8e6b046 100644 --- a/Lib/test/test_concurrent_futures.py +++ b/Lib/test/test_concurrent_futures.py @@ -568,19 +568,6 @@ def test_shutdown_no_wait(self): # shutdown. assert all([r == abs(v) for r, v in zip(res, range(-5, 5))]) - def test_hang_issue45021(self): - """https://bugs.python.org/issue45021""" - def submit(pool): - pool.submit(submit, pool) - - if hasattr(os, 'register_at_fork'): - with futures.ThreadPoolExecutor(1) as pool: - pool.submit(submit, pool) - - for _ in range(50): - with futures.ProcessPoolExecutor(1, mp_context=get_context('fork')) as workers: - workers.submit(tuple) - create_executor_tests(ProcessPoolShutdownTest, executor_mixins=(ProcessPoolForkMixin, @@ -918,6 +905,20 @@ def test_idle_thread_reuse(self): self.assertEqual(len(executor._threads), 1) executor.shutdown(wait=True) + @unittest.skipUnless(hasattr(os, 'register_at_fork'), 'need os.register_at_fork') + def test_hang_global_shutdown_lock(self): + # bpo-45021: _global_shutdown_lock should be reinitialized in the child + # process, otherwise it will never exit + def submit(pool): + pool.submit(submit, pool) + + with futures.ThreadPoolExecutor(1) as pool: + pool.submit(submit, pool) + + for _ in range(50): + with futures.ProcessPoolExecutor(1, mp_context=get_context('fork')) as workers: + workers.submit(tuple) + class ProcessPoolExecutorTest(ExecutorTest):