Assume the following code:

from multiprocessing.pool import ThreadPool
class A(object):
    def __init__(self):
        self.pool = ThreadPool()
    def __del__(self):
        self.pool.close()
        self.pool.join()
a = A()
print(a)

The code snippet above hangs forever on Python 3.8+ (works ok on Python 3.7 and earlier). An example output where I've added some extra prints on to the thread joins:

<__main__.A object at 0x1104d6070>
join thread <Thread(Thread-13, started daemon 123145699672064)> None
done
join thread <Thread(Thread-14, stopped daemon 123145716461568)> None
done
join thread <Thread(Thread-15, stopped daemon 123145733251072)> None
done
join thread <Thread(Thread-13, stopped daemon 123145699672064)> None
done
join thread <Thread(Thread-14, stopped daemon 123145716461568)> None
done
join thread <Thread(Thread-15, stopped daemon 123145733251072)> None
done
join thread <DummyProcess(Thread-1, stopped daemon 123145498198016)> None
done
join thread <DummyProcess(Thread-2, stopped daemon 123145514987520)> None
done
join thread <DummyProcess(Thread-3, stopped daemon 123145531777024)> None
done
join thread <DummyProcess(Thread-4, stopped daemon 123145548566528)> None
done
join thread <DummyProcess(Thread-5, started daemon 123145565356032)> None

I've tested on MacOs, but could replicate on Linux too within the CI.

An FYI update, the code is used by swagger-codegen to generate HTTP clients (https://github.com/swagger-api/swagger-codegen/blob/master/modules/swagger-codegen/src/main/resources/python/api_client.mustache#L73). The example below basically replicates creating such client as a global.

This code snippet seems to trigger a crash in the _struct module:
https://bugs.python.org/issue38076#msg360127

Reported the issue downstream too under swagger-api/swagger-codegen#9991

This is not a regression: the code only worked on 3.7 because there is a sleep in the pool maintainance loop that makes the pool more likely being more consistent, but is the same problem as there is a race condition. For example, reducing the sleep in the loop (notice 3.8 does not use a sleep but instead uses a more resilient system of sentinels) for the 3.7 version reproduces the problem:

diff --git a/Lib/multiprocessing/pool.py b/Lib/multiprocessing/pool.py
index 3e9a0d6b48..f8d438d87c 100644
--- a/Lib/multiprocessing/pool.py
+++ b/Lib/multiprocessing/pool.py
@@ -410,7 +410,7 @@ class Pool(object):
         # is terminated.
         while thread._state == RUN or (pool._cache and thread._state != TERMINATE):
             pool._maintain_pool()
-            time.sleep(0.1)
+            time.sleep(0.00000001)
         # send sentinel to stop workers
         pool._taskqueue.put(None)
         util.debug('worker handler exiting')

With that patch, 3.7 hangs as well. The problem here is that something regarding the locks inside the SimpleQueue of inbound tasks is not in a consistent state when Python finalization calls __del__.

Check out also https://bugs.python.org/issue38744

I've applied pull request patch (which looks a bit weird to me, removes something then adds the same thing back etc https://patch-diff.githubusercontent.com/raw/python/cpython/pull/19009.patch). Essentially the change is:

diff --git a/Lib/multiprocessing/pool.py b/Lib/multiprocessing/pool.py
index b223d6aa72..8bd9608b0e 100644
--- a/Lib/multiprocessing/pool.py
+++ b/Lib/multiprocessing/pool.py
@@ -682,7 +682,15 @@ class Pool(object):
# this is guaranteed to only be called once
util.debug('finalizing pool')

-        worker_handler._state = TERMINATE
+        # Explicitly do the cleanup here if it didn't come from terminate()
+        # (required for if the queue will block, if it is already closed)
+        if worker_handler._state != TERMINATE:
+            # 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')

Unfortunately test case from first message in this bug report still hangs for me with this applied.

(note, testing on 3.8 branch + pull request patch)

which looks a bit weird to me

Is explained in the message in https://bugs.python.org/msg364211:

What happens is that is moving that code so is executed in both code paths: explicit termination and multiprocessing finalization.

Unfortunately test case from first message in this bug report still hangs for me with this applied.

Hummm, the test case from this bug is precisely the test case in PR 19009 so it should not hang.

New changeset ac10e0c by Batuhan Taşkaya in branch 'master':
bpo-39360: Ensure all workers exit when finalizing a multiprocessing Pool (GH-19009)
ac10e0c

Hummm, the test case from this bug is precisely the test case in PR 19009 so it should not hang.

Oh, actually I am wrong as this is a different issue I had in mind. PR 19009 should fix the case reported in https://bugs.python.org/issue38744 and this equivalent:

          from multiprocessing import Pool
            class A:
                def __init__(self):
                    self.pool = Pool(processes=1)
            def test():
                problem = A()
                problem.pool.map(float, tuple(range(10)))
            if __name__ == "__main__":
                test()

Notice that in this code there is no __del__ call.

I tried and I cannot reproduce the hang with the code provided in the first message here after PR 19009. (Tested on Linux, Windows and MacOS)

Ok, so two test cases

from multiprocessing.pool import ThreadPool
class A(object):
    def __init__(self):
        self.pool = ThreadPool()
    def __del__(self):
        self.pool.close()
        self.pool.join()
a = A()
print(a)

from multiprocessing import Pool
class A:
    def __init__(self):
        self.pool = Pool(processes=1)
def test():
    problem = A()
    problem.pool.map(float, tuple(range(10)))
if __name__ == "__main__":
    test()

On two my test environments with Linux, 5.5.2 and 4.9.208 kernel, glibc 2.31 (userspace is mostly the same on both) test 1) hangs, 2) doesn't hang.

Testing done on 3.8 branch + https://patch-diff.githubusercontent.com/raw/python/cpython/pull/19023.patch

For test 1) no traceback on ctrl+c and also no trace logged with faulthandler installed, so no idea where it hangt.

strace shows
[...]
[pid 232031] munmap(0x7f1baaffe000, 8392704 <unfinished ...>
[pid 232017] write(5, "\0\0\0\4\200\4N.", 8 <unfinished ...>
[pid 232031] <... munmap resumed>) = 0
[pid 232017] <... write resumed>) = 8
[pid 232031] exit(0) = ?
[pid 232017] futex(0x7f1b94000b60, FUTEX_WAIT_BITSET_PRIVATE|FUTEX_CLOCK_REALTIME, 0, NULL, FUTEX_BITSET_MATCH_ANY <unfinished ...>
[pid 232031] +++ exited with 0 +++
^C ************************* ctrl + c pressed
[pid 232017] <... futex resumed>) = ? ERESTARTSYS (To be restarted if SA_RESTART is set)
strace: Process 232017 detached
strace: Process 232032 detached
strace: Process 232033 detached
strace: Process 232034 detached
strace: Process 232035 detached

Sadly, I am unable to reproduce, so without more clarification or a reliable reproducer, we cannot start debugging the issue.

For test 1) no traceback on ctrl+c and also no trace logged with faulthandler installed, so no idea where it hangt.

Can you attach with gdb and tell the C trace and the python trace (check the gdb helpers on the CPython repo for the later).

New changeset 7f5302f by Miss Islington (bot) in branch '3.8':
bpo-39360: Ensure all workers exit when finalizing a multiprocessing Pool (GH-19009)
7f5302f

http://ixion.pld-linux.org/~arekm/python-gdb1.txt

it's a result of thread apply all bt, thread apply all bt full, where

pystack command shows nothing.

ps. I'm arekm@freenode and can give ssh account for playing with the issue

ps. I'm arekm@freenode and can give ssh account for playing with the issue

Can you send me an email to pablogsal@gmail.com to set up ssh access?

I still cannot reproduce it in your system:

[test@ixion-pld cpython]$ hostname
ixion-pld
[test@ixion-pld cpython]$ uname -a
Linux ixion-pld 5.5.2-1 #1 SMP Wed Feb 5 19:26:43 CET 2020 x86_64 Common_KVM_processor PLD Linux

[test@ixion-pld cpython]$ cat lel.py
from multiprocessing.pool import ThreadPool
class A(object):
def __init__(self):
self.pool = ThreadPool()
def __del__(self):
self.pool.close()
self.pool.join()
a = A()
print(a)
[test@ixion-pld cpython]$ time ./python lel.py
<main.A object at 0x7f9130becd20>

real 0m0.124s
user 0m0.105s
sys 0m0.021s

I tested it on the master branch with commit 5b66ec1

master works fine
3.8 branch hangs

so there is some other/related issue.

On master test 1) hangs before commit below and works after commit below. Unfortunately applying that commit to 3.8 branch doesn't help - 3.8 still hangs. Some other fix is also needed I guess

commit 9ad58ac
Author: Victor Stinner <vstinner@python.org>
Date: Mon Mar 9 23:37:49 2020 +0100

bpo-19466: Py_Finalize() clears daemon threads earlier (GH-18848)

Clear the frames of daemon threads earlier during the Python shutdown to
call objects destructors. So "unclosed file" resource warnings are now
emitted for daemon threads in a more reliable way.

Cleanup _PyThreadState_DeleteExcept() code: rename "garbage" to
"list".

This one is also needed on 3.8 to get it not hang with 1) test case.

So 3.8 branch + 9ad58ac + 4d96b46 is working for me.

commit 4d96b46
Author: Victor Stinner <vstinner@python.org>
Date: Sat Feb 1 02:30:25 2020 +0100

bpo-39511: PyThreadState_Clear() calls on_delete (GH-18296)

PyThreadState.on_delete is a callback used to notify Python when a
thread completes. _thread._set_sentinel() function creates a lock
which is released when the thread completes. It sets on_delete
callback to the internal release_sentinel() function. This lock is
known as Threading._tstate_lock in the threading module.

The release_sentinel() function uses the Python C API. The problem is
that on_delete is called late in the Python finalization, when the C
API is no longer fully working.

The PyThreadState_Clear() function now calls the
PyThreadState.on_delete callback. Previously, that happened in
PyThreadState_Delete().

The release_sentinel() function is now called when the C API is still
fully working.

Can you try applying 4f384af and 4d96b46 together?

Oh, sorry, I missed your last message.

So 3.8 branch + 9ad58ac + 4d96b46 is working for me.

I can confirm that these commits together fix the problem.

Victor, are you OK if we backport both changes to 3.8?

Victor, are you OK if we backport both changes to 3.8?

Let me look at commit 9ad58ac:
"bpo-19466: Py_Finalize() clears daemon threads earlier (GH-18848)"

Calling _PyThreadState_DeleteExcept() in Py_FinalizeEx() is really dangerous. It frees PyThreadState memory of daemon threads. Daemon threads continue to run while Py_FinalizeEx() is running (which takes an unknown amount of time, we only know that it's larger than 0 seconds). When a daemon thread attempts to acquire the GIL, it will likely crash if its PyThreadState memory is freed. This memory can be overriden by another memory allocation, or dereferencing the pointer can trigger a segmentation fault.

This change caused multiple regressions in the master branch. I had hard time to fix all crashes: I modified take_gil() 4 times, and I'm still not sure that my fix is correct. I had to modify take_gil() function which acquire the GIL: this function is really fragile and I would prefer to not touch it in a stable branch. See bpo-39877 changes to have an idea of the complexity of the problem.

Python finalization is really fragile: https://pythondev.readthedocs.io/finalization.html

commit 4d96b46
"bpo-39511: PyThreadState_Clear() calls on_delete (GH-18296)"

"(...) The release_sentinel() function is now called when the C API is still
fully working."

This change is "safer" than the other one, but there is still a risk of introducing a regression.

The problem is that this commit is part of a larger effort to make the Python finalization more reliable. The commit makes sense in the serie of commits pushed into the master branch, but I'm not sure that it makes sense if it's taken alone.

I don't have the bandwidth right now to investigate this issue, to identify the root issue, and suggest which commits should be backported or not.

Ok, so sadly it seems that we need to close this issue as is fixed in master but the backport is risky.

If you want to get a reliable behavior, don't rely on destructors. Python finalization is not determistic and destructors can be called while Python is not longer fully functional.

Release ressources explicitly. For example, use multiprocessing.Pool with a context manager, or even call close() and join() methods explicitly.

So only this test (from first commit) should be added on master to check for further regressions in the area, right?