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

marmarek · 2023-03-07T21:40:28Z

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

The text was updated successfully, but these errors were encountered:

marmarek · 2023-03-07T21:53:40Z

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

If os.fork() is called from a DummyThread (a thread started not by the threading module, in which somebody called threading.current_thread()), it becomes main thread. Threading shutdown logic relies on the main thread having tstate_lock, so initialize it for the DummyThread too. Fixes python#102512

python/cpython#102512 Fixes QubesOS/qubes-issues#8078

The bug causes occasional issues with multi-process pipeline executions with pytest using concurrent running. This is a known issue (python/cpython#102512) which will eventually be fixed in Python, at that time the patch should become no-op and can be removed.

gvanrossum · 2023-12-17T20:19:37Z

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

marmarek · 2023-12-17T21:00:17Z

@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 · 2023-12-17T21:32:15Z

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 · 2023-12-18T11:05:02Z

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?

…from a foreign thread (GH-113261) Always set a _MainThread as a main thread after os.fork() is called from a thread started not by the threading module. A new _MainThread was already set as a new main thread after fork if threading.current_thread() was not called for a foreign thread before fork. Now, if it was called before fork, the implicitly created _DummyThread will be turned into _MainThread after fork. It fixes, in particularly, an incompatibility of _DummyThread with the threading shutdown logic which relies on the main thread having tstate_lock. Co-authored-by: Marek Marczykowski-Górecki <[email protected]>

…alled from a foreign thread (pythonGH-113261) Always set a _MainThread as a main thread after os.fork() is called from a thread started not by the threading module. A new _MainThread was already set as a new main thread after fork if threading.current_thread() was not called for a foreign thread before fork. Now, if it was called before fork, the implicitly created _DummyThread will be turned into _MainThread after fork. It fixes, in particularly, an incompatibility of _DummyThread with the threading shutdown logic which relies on the main thread having tstate_lock. (cherry picked from commit 49785b0) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Marek Marczykowski-Górecki <[email protected]>

…ork() called from a foreign thread (pythonGH-113261) Always set a _MainThread as a main thread after os.fork() is called from a thread started not by the threading module. A new _MainThread was already set as a new main thread after fork if threading.current_thread() was not called for a foreign thread before fork. Now, if it was called before fork, the implicitly created _DummyThread will be turned into _MainThread after fork. It fixes, in particularly, an incompatibility of _DummyThread with the threading shutdown logic which relies on the main thread having tstate_lock. (cherry picked from commit 49785b0) Co-authored-by: Marek Marczykowski-Górecki <[email protected]>

…ork() called from a foreign thread (pythonGH-113261) Always set a _MainThread as a main thread after os.fork() is called from a thread started not by the threading module. A new _MainThread was already set as a new main thread after fork if threading.current_thread() was not called for a foreign thread before fork. Now, if it was called before fork, the implicitly created _DummyThread will be turned into _MainThread after fork. It fixes, in particularly, an incompatibility of _DummyThread with the threading shutdown logic which relies on the main thread having tstate_lock. (cherry picked from commit 49785b0) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Marek Marczykowski-Górecki <[email protected]>

serhiy-storchaka · 2024-01-22T14:56:09Z

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.

…called from a foreign thread (GH-113261) (GH-114431) Always set a _MainThread as a main thread after os.fork() is called from a thread started not by the threading module. A new _MainThread was already set as a new main thread after fork if threading.current_thread() was not called for a foreign thread before fork. Now, if it was called before fork, the implicitly created _DummyThread will be turned into _MainThread after fork. It fixes, in particularly, an incompatibility of _DummyThread with the threading shutdown logic which relies on the main thread having tstate_lock. (cherry picked from commit 49785b0) Co-authored-by: Marek Marczykowski-Górecki <[email protected]>

…alled from a foreign thread (pythonGH-113261) Always set a _MainThread as a main thread after os.fork() is called from a thread started not by the threading module. A new _MainThread was already set as a new main thread after fork if threading.current_thread() was not called for a foreign thread before fork. Now, if it was called before fork, the implicitly created _DummyThread will be turned into _MainThread after fork. It fixes, in particularly, an incompatibility of _DummyThread with the threading shutdown logic which relies on the main thread having tstate_lock. (cherry picked from commit 49785b0) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Marek Marczykowski-Górecki <[email protected]>

…called from a foreign thread (GH-113261) (GH-114430) Always set a _MainThread as a main thread after os.fork() is called from a thread started not by the threading module. A new _MainThread was already set as a new main thread after fork if threading.current_thread() was not called for a foreign thread before fork. Now, if it was called before fork, the implicitly created _DummyThread will be turned into _MainThread after fork. It fixes, in particularly, an incompatibility of _DummyThread with the threading shutdown logic which relies on the main thread having tstate_lock. (cherry picked from commit 49785b0) Co-authored-by: Serhiy Storchaka <[email protected]> Co-authored-by: Marek Marczykowski-Górecki <[email protected]>

…alled from a foreign thread (pythonGH-113261) Always set a _MainThread as a main thread after os.fork() is called from a thread started not by the threading module. A new _MainThread was already set as a new main thread after fork if threading.current_thread() was not called for a foreign thread before fork. Now, if it was called before fork, the implicitly created _DummyThread will be turned into _MainThread after fork. It fixes, in particularly, an incompatibility of _DummyThread with the threading shutdown logic which relies on the main thread having tstate_lock. Co-authored-by: Marek Marczykowski-Górecki <[email protected]>

marmarek added the type-bug An unexpected behavior, bug, or error label Mar 7, 2023

marmarek mentioned this issue Mar 7, 2023

Restoring/verifying a backup in GUI fails with "check extracting process errors" QubesOS/qubes-issues#8078

Closed

marmarek added a commit to marmarek/cpython that referenced this issue Mar 7, 2023

Add regression test for pythongh-102512

6874b6a

marmarek added a commit to marmarek/cpython that referenced this issue Mar 7, 2023

Add regression test for pythongh-102512

59ace50

bedevere-bot mentioned this issue Mar 7, 2023

gh-102512: Fix threading after os.fork() called from a foreign thread #102517

Closed

marmarek added a commit to marmarek/qubes-core-admin-client that referenced this issue Mar 7, 2023

Yet another disgusting hack for python 3.11.2 bug

9f73cd5

python/cpython#102512 Fixes QubesOS/qubes-issues#8078

marmarek mentioned this issue Mar 7, 2023

Yet another disgusting hack for python 3.11.2 bug QubesOS/qubes-core-admin-client#236

Merged

marmarek added a commit to marmarek/qubes-core-admin-client that referenced this issue Mar 7, 2023

Yet another disgusting hack for python 3.11.2 bug

179bc4b

python/cpython#102512 Fixes QubesOS/qubes-issues#8078

marmarek added a commit to marmarek/qubes-core-admin-client that referenced this issue Mar 8, 2023

Yet another disgusting hack for python 3.11.2 bug

a9b17d9

python/cpython#102512 Fixes QubesOS/qubes-issues#8078

arhadthedev added the topic-multiprocessing label Apr 2, 2023

andy-slac mentioned this issue Apr 12, 2023

DM-38669: Add workaround for Python threading bug lsst/ctrl_mpexec#231

Merged

2 tasks

serhiy-storchaka added this to Multiprocessing issues Sep 10, 2023

serhiy-storchaka added 3.11 only security fixes 3.12 bugs and security fixes 3.13 bugs and security fixes labels Dec 18, 2023

bedevere-app bot mentioned this issue Dec 18, 2023

gh-102512: Fix threading after os.fork() called from a foreign thread #113261

Merged

bedevere-app bot mentioned this issue Jan 22, 2024

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

Merged

bedevere-app bot mentioned this issue Jan 22, 2024

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

Merged

serhiy-storchaka closed this as completed Jan 22, 2024

github-project-automation bot moved this to Done in Multiprocessing issues Jan 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

marmarek commented Mar 7, 2023 •

edited by bedevere-app bot

Loading

marmarek commented Mar 7, 2023 •

edited

Loading

gvanrossum commented Dec 17, 2023

marmarek commented Dec 17, 2023

gvanrossum commented Dec 17, 2023

serhiy-storchaka commented Dec 18, 2023

serhiy-storchaka commented Jan 22, 2024

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

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

Comments

marmarek commented Mar 7, 2023 • edited by bedevere-app bot Loading

Bug report

Your environment

Linked PRs

marmarek commented Mar 7, 2023 • edited Loading

gvanrossum commented Dec 17, 2023

marmarek commented Dec 17, 2023

gvanrossum commented Dec 17, 2023

serhiy-storchaka commented Dec 18, 2023

serhiy-storchaka commented Jan 22, 2024

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

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

marmarek commented Mar 7, 2023 •

edited by bedevere-app bot

Loading

marmarek commented Mar 7, 2023 •

edited

Loading