Skip to content
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

threading Thread.join should call the OS join API #110829

Closed
gpshead opened this issue Oct 13, 2023 · 5 comments
Closed

threading Thread.join should call the OS join API #110829

gpshead opened this issue Oct 13, 2023 · 5 comments
Labels
type-feature A feature request or enhancement

Comments

@gpshead
Copy link
Member

gpshead commented Oct 13, 2023

Feature or enhancement

Proposal:

threading.Thread.join() only waits for the CPython internals to wash its hands of the underlying thread. It doesn't actually wait for the OS thread itself to exit, which in theory happens rapidly as its final internal code completes quickly - but we have no good wait to determine.

Why finally do this now? Now that we're encouraging people to notice and avoid threading existing when os.fork is called in 3.12, a use case has come up for deterministically knowing when the thread is done at the OS level so that the code can proceed with os.fork.

#110510 could use this in an atfork before fork handler for example.

POSIX has pthread_join, we should be able to expose and use via _thread. Windows presumably has an equivalent concept API.

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

Linked PRs

@pitrou
Copy link
Member

pitrou commented Oct 13, 2023

On Windows, we start the thread with _beginthreadex, so we should be able to call WaitForSingleObject to ensure that the thread has properly terminated. See second example here: https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/beginthread-beginthreadex?view=msvc-170#examples

But, of course, on Windows the entire fork() problem doesn't exist, so it's less of a problem if we don't provide lesser guarantees.

Note that right now we detach the thread as soon as it is started, so we may want to expose a different internal API that doesn't detach the thread.

@ericsnowcurrently
Copy link
Member

FWIW, the gap between Python-thread-ends and OS-thread-exits bit me after I added the per-interpreter GIL (PEP 684). We ended up with crash due to that interval (a race in the GIL's drop_gil(), which I fixed in gh-105109. The linked PRs on gh-104341 include a couple other approaches I tried out to solve the race (with extra complexity), and IIRC at least one of them would have (partially?) dealt with closing the above gap.

@pitrou
Copy link
Member

pitrou commented Oct 25, 2023

The API added here is only for the threading module, though, because it's quite inflexible (see comments in the .h file).

pitrou added a commit to pitrou/cpython that referenced this issue Oct 27, 2023
pitrou added a commit to pitrou/cpython that referenced this issue Oct 30, 2023
pitrou added a commit to pitrou/cpython that referenced this issue Nov 4, 2023
pitrou added a commit that referenced this issue Nov 4, 2023
Joining a thread now ensures the underlying OS thread has exited. This is required for safer fork() in multi-threaded processes.

---------

Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
@hugovk
Copy link
Member

hugovk commented Nov 9, 2023

Closing as the PR has been merged.

@hugovk hugovk closed this as completed Nov 9, 2023
@pitrou
Copy link
Member

pitrou commented Nov 9, 2023

Oops, sorry for forgetting to close this issue!

aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
)

Joining a thread now ensures the underlying OS thread has exited. This is required for safer fork() in multi-threaded processes.

---------

Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
Development

No branches or pull requests

4 participants