Skip to content

Not able to start an instantiated Thread in a child process through multiprocessing.Process in python 3.13 #134381

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
anadrianmanrique opened this issue May 20, 2025 · 11 comments
Labels
3.13 bugs and security fixes 3.14 bugs and security fixes 3.15 new features, bugs and security fixes extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error

Comments

@anadrianmanrique
Copy link

anadrianmanrique commented May 20, 2025

Bug report

Bug description:

I'm experiencing an issue specifically in python 3.13 regarding Threads in the context of multiprocessing.
The following code is working in python < 3.13

from multiprocessing import Process
from threading import Thread
import time
class MyThread(Thread):
    def __init__(self):
        Thread.__init__(self)
        self.__data = ''

    def run(self):
        print("Hi from thread")
        print(self.__data)


class Aclass():
    def __init__(self):
        self._t = MyThread()
        self._t.daemon = True

    def start(self):
        self._t.start()
        print("thread started")

if __name__ == '__main__':
    t = Aclass()
    p = Process(target = t.start )
    p.start()
    time.sleep(2)

After executing the above in python 3.13 I get this output:

 File "/usr/lib/python3.13/multiprocessing/process.py", line 313, in _bootstrap
    self.run()
    ~~~~~~~~^^
  File "/usr/lib/python3.13/multiprocessing/process.py", line 108, in run
    self._target(*self._args, **self._kwargs)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/kali/impacket/master/impacket/tests/SMB_RPC/test_bug_process_thread.py", line 18, in start
    self._t.start()
    ~~~~~~~~~~~~~^^
  File "/usr/lib/python3.13/threading.py", line 973, in start
    _start_joinable_thread(self._bootstrap, handle=self._handle,
    ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                           daemon=self.daemon)
                           ^^^^^^^^^^^^^^^^^^^
RuntimeError: thread already started

I managed to workaround this by delaying the Thread instance initialization, moving the Thread.init() call to start() method in the derived class, so everything gets executed in the context of the child process.

CPython versions tested on:

3.13

Operating systems tested on:

Linux

Linked PRs

@anadrianmanrique anadrianmanrique added the type-bug An unexpected behavior, bug, or error label May 20, 2025
@zangjiucheng
Copy link
Contributor

Hi, I am a beginner contributor to the CPython project, I scan your issue and I believe this related to following function,

Modules/_threadmodule.c

void
_PyThread_AfterFork(struct _pythread_runtime_state *state)
{
    // gh-115035: We mark ThreadHandles as not joinable early in the child's
    // after-fork handler. We do this before calling any Python code to ensure
    // that it happens before any ThreadHandles are deallocated, such as by a
    // GC cycle.
    PyThread_ident_t current = PyThread_get_thread_ident_ex();

    struct llist_node *node;
    llist_for_each_safe(node, &state->handles) {
        ThreadHandle *handle = llist_data(node, ThreadHandle, node);
        if (handle->ident == current) {
            continue;
        }

        // Mark all threads as done. Any attempts to join or detach the
        // underlying OS thread (if any) could crash. We are the only thread;
        // it's safe to set this non-atomically.

        /* Issue Here, it will set `handle->state` to THREAD_HANDLE_DONE */
        // handle->state = THREAD_HANDLE_DONE;

        handle->once = (_PyOnceFlag){_Py_ONCE_INITIALIZED};
        handle->mutex = (PyMutex){_Py_UNLOCKED};
        _PyEvent_Notify(&handle->thread_is_exiting);
        llist_remove(node);
        remove_from_shutdown_handles(handle);
    }
}

There is a related PR to this topic: gh-114271

If I've made any mistakes, please point them out and I'll continue to explore the issue.

@ZeroIntensity
Copy link
Member

The problem is that _PyThread_AfterFork assumes all Thread objects have started so it can remove them. This line is the problem:

handle->state = THREAD_HANDLE_DONE;

We need to ignore threads that haven't started. That said, doing this sort of thing is incredibly weird. What's the use-case for creating a Thread in a parent process, but then starting it in a child process?

@ZeroIntensity ZeroIntensity added extension-modules C modules in the Modules dir 3.13 bugs and security fixes 3.14 bugs and security fixes 3.15 new features, bugs and security fixes labels May 21, 2025
@anadrianmanrique
Copy link
Author

anadrianmanrique commented May 21, 2025

In my particular scenario, I'm running tests in client/server project. Test code for the client executes server code in a Process instance, as a precondition. Some parts of the server are implemented as Thread subclasses.

@zangjiucheng
Copy link
Contributor

Hi everyone, I was thinking about this today. Based on an idea from @ZeroIntensity. (It's probably a very basic implement), since this is a thread that hasn't been started yet, I can't think of any harm in keeping it. (The only trouble could be we will need another GC cycle to cleanup the dead thread)

void
_PyThread_AfterFork(struct _pythread_runtime_state *state)
{
    // gh-115035: We mark ThreadHandles as not joinable early in the child's
    // after-fork handler. We do this before calling any Python code to ensure
    // that it happens before any ThreadHandles are deallocated, such as by a
    // GC cycle.
    PyThread_ident_t current = PyThread_get_thread_ident_ex();

    struct llist_node *node;
    llist_for_each_safe(node, &state->handles) {
        ThreadHandle *handle = llist_data(node, ThreadHandle, node);
        if (handle->ident == current) {
            continue;
        }

+      // keep not_start handles – they are safe to start in the child
+      if (handle->state == THREAD_HANDLE_NOT_STARTED){
+          continue;
+      }

        // Mark all threads as done. Any attempts to join or detach the
        // underlying OS thread (if any) could crash. We are the only thread;
        // it's safe to set this non-atomically.
        handle->state = THREAD_HANDLE_DONE;
        handle->once = (_PyOnceFlag){_Py_ONCE_INITIALIZED};
        handle->mutex = (PyMutex){_Py_UNLOCKED};
        _PyEvent_Notify(&handle->thread_is_exiting);
        llist_remove(node);
        remove_from_shutdown_handles(handle);
    }
}

@ZeroIntensity
Copy link
Member

Sounds reasonable to me.

@zangjiucheng
Copy link
Contributor

Sounds reasonable to me.

Hi there, I wish to start draft a PR for this issue. Should I target to branch main or 3.13, I did more test on 3.14 / main branch. It seems another issue pop up. TypeError: cannot pickle '_thread._ThreadHandle' object.

This only fix the issue on 3.13 now, but I will also keep observe more detail about the new issue on 3.14 and main.

Thanks for your help @ZeroIntensity @anadrianmanrique

@ZeroIntensity
Copy link
Member

Put up a PR for the main branch. If we decide to fix this for older versions, we can automatically backport the fix.

@zangjiucheng
Copy link
Contributor

Done, thanks so much :)

@anadrianmanrique
Copy link
Author

Thank you all for the quick response/fix

@colesbury
Copy link
Contributor

colesbury commented May 22, 2025

This pattern will only work with "fork", not "spawn" because you can't pickle a Thread. In 3.13 3.14, the default start method switched from "fork" to "spawn", so this test case won't work as is. You'd need something like multiprocessing.set_start_method("fork") in the if __name__ == '__main__' in addition to the changes proposed by @zangjiucheng. It also will not work on Windows, which doesn't support "fork".

@zangjiucheng
Copy link
Contributor

@colesbury Thanks a bunch! I was also wondering why this crashed. I learned something new, thanks!

zangjiucheng added a commit to zangjiucheng/cpython that referenced this issue May 23, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this issue May 23, 2025
…d after fork (pythongh-134514)

(cherry picked from commit 9a2346d)

Co-authored-by: Jiucheng(Oliver) <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue May 23, 2025
…d after fork (pythongh-134514)

(cherry picked from commit 9a2346d)

Co-authored-by: Jiucheng(Oliver) <[email protected]>
colesbury pushed a commit that referenced this issue May 23, 2025
…ad after fork (gh-134514) (gh-134597)

(cherry picked from commit 9a2346d)

Co-authored-by: Jiucheng(Oliver) <[email protected]>
colesbury pushed a commit that referenced this issue May 23, 2025
…ad after fork (gh-134514) (gh-134596)

(cherry picked from commit 9a2346d)

Co-authored-by: Jiucheng(Oliver) <[email protected]>
anadrianmanrique added a commit to fortra/impacket that referenced this issue May 23, 2025
* Update build_and_test.yml

added python 3.13
removed python 3.8

* workaround delying thread initialization to avoid python3.13 bug

* workaround delaying thread initialization to avoid python3.13 bug

* avoid running Process instances beacuse of a bug in python3.13 python/cpython#134381

* removed hack

* make helper servers run as daemon in order to make testecases not to get stuck
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes 3.14 bugs and security fixes 3.15 new features, bugs and security fixes extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants