os.fork() called from DummyThread confuses threading shutdown logic

[marmarek]

Bug report

threading._shutdown() relies on _main_thread having _tstate_lock not None (there is assert for that). When fork is called from a DummyThread (in my case, that's a thread created by (Py)Qt), it gets promoted to main thread, but remains very simplistic DummyThread. Especially, nobody initializes its _tstate_lock. threading._after_fork() handles the case of current thread not being in _active dict at all (by creating new MainThread object), but it doesn't handle the case of having DummyThread there already. This results in AssertionError in thread shutdown method - which for example confuses multiprocessing.Process (it gets exit code 1, even if the process function was successful).

Reproducer:

#!/usr/bin/python3

import threading
import multiprocessing
import _thread

class Bar(multiprocessing.Process):
    def run(self):
        print("process")

def run_thread(lock):
    # the call to current_thread() is crucial for reproducer - it allocates
    # DummyThread()
    print(f"thread: {threading.current_thread()}")
    p = Bar()
    p.start()
    p.join()
    print(f"proc exit code: {p.exitcode}")
    lock.release()


def main():
    lock = _thread.allocate_lock()
    lock.acquire()
    t = _thread.start_new_thread(run_thread, (lock,))
    # t.join
    lock.acquire()
    print(f"thread exit")

main()

It should print:

thread: <_DummyThread(Dummy-1, started daemon 135243893053120)>
process
proc exit code: 0
thread exit

but it prints:

thread: <_DummyThread(Dummy-1, started daemon 135243893053120)>
process
proc exit code: 1
thread exit

(see exit code difference)

multiprocessing.Process (or rather multiprocessing.popen_fork.Popen._launch() to be specific) swallows the exception, but adding some debug prints there I get:

Traceback (most recent call last):
  File "/usr/lib64/python3.11/multiprocessing/popen_fork.py", line 71, in _launch
    code = process_obj._bootstrap(parent_sentinel=child_r)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.11/multiprocessing/process.py", line 332, in _bootstrap
    threading._shutdown()
  File "/usr/lib64/python3.11/threading.py", line 1553, in _shutdown
    assert tlock is not None
           ^^^^^^^^^^^^^^^^^

Your environment

• CPython versions tested on: 3.11.2
• Operating system and architecture: Fedora 37, x86_64

Linked PRs

• gh-102512: Fix threading after os.fork() called from a foreign thread #102517
• gh-102512: Fix threading after os.fork() called from a foreign thread #113261
• [3.12] gh-102512: Turn _DummyThread into _MainThread after os.fork() called from a foreign thread (GH-113261) #114430
• [3.11] gh-102512: Turn _DummyThread into _MainThread after os.fork() called from a foreign thread (GH-113261) #114431

[marmarek]

Simplified reproducer (not importing multiprocessing, but doing what it does directly):

#!/usr/bin/python3

import threading
import _thread
import os
import traceback

def run_thread(lock):
    # the call to current_thread() is crucial for reproducer - it allocates
    # DummyThread()
    print(f"thread: {threading.current_thread()}")
    if os.fork() == 0:
        print("process")
        try:
            threading._shutdown()
            os._exit(0)
        except:
            traceback.print_exc()
            os._exit(1)
    else:
        retcode = os.wait()
    print(f"proc exit code: {os.WEXITSTATUS(retcode[1])}")
    lock.release()


def main():
    lock = _thread.allocate_lock()
    lock.acquire()
    t = _thread.start_new_thread(run_thread, (lock,))
    # t.join
    lock.acquire()
    print(f"thread exit")

main()

It fails this way:

thread: <_DummyThread(Dummy-1, started daemon 127509727442624)>
process
Traceback (most recent call last):
  File "/tmp/test.py", line 15, in run_thread
    threading._shutdown()
  File "/usr/lib64/python3.11/threading.py", line 1553, in _shutdown
    assert tlock is not None
           ^^^^^^^^^^^^^^^^^
AssertionError
proc exit code: 1
thread exit

[gvanrossum]

@marmarek Do you have enough of a handle on the problem to propose a PR that fixes it?

[marmarek]

@marmarek Do you have enough of a handle on the problem to propose a PR that fixes it?

I already did, over half a year ago (it's linked in the issue description). Should I rebase it to get it merged?

[gvanrossum]

My bad! I stumbled upon this thread and didn't notice that (I blame GitHub :-). It needs to be reviewed, perhaps by @serhiy-storchaka.

[serhiy-storchaka]

Would not be more correct to replace the current _DummyThread with a _MainThread in _after_fork()? Or maybe the current thread of any type with a new _MainThread?

[serhiy-storchaka]

Thank you for your report and PR @marmarek. I created a more radical PR based on it. It is not only create a lock for a main thread after fork, it makes it non-demonic and changes it type.

Perhaps a similar should be done for normal threads, but this is a different issue. It can be an instance of a custom Thread subclass, so changing its type may be not safe.