Skip to content

Ensure current thread state is cleared before deleting it in _PyThreadState_DeleteCurrent #116515

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
colesbury opened this issue Mar 8, 2024 · 0 comments
Assignees
Labels
topic-C-API type-feature A feature request or enhancement

Comments

@colesbury
Copy link
Contributor

colesbury commented Mar 8, 2024

Feature or enhancement

In general, when _PyThreadState_GET() is non-NULL then the current thread is "attached", but there is a small window during PyThreadState_DeleteCurrent() where that's not true: tstate_delete_common() is called when the thread is detached, but before current_fast_clear():

cpython/Python/pystate.c

Lines 1689 to 1691 in 601f3a7

tstate_set_detached(tstate, _Py_THREAD_DETACHED);
tstate_delete_common(tstate);
current_fast_clear(tstate->interp->runtime);

I think it's worth swapping the order of the calls so that we call current_fast_clear before tstate_delete_common. This would also make the order of operations in PyThreadState_DeleteCurrent() more similar to the order used when calling PyThreadState_Delete().

See also: #116483

cc @ericsnowcurrently

Linked PRs

@colesbury colesbury added the type-feature A feature request or enhancement label Mar 8, 2024
@colesbury colesbury self-assigned this Mar 8, 2024
colesbury added a commit to colesbury/cpython that referenced this issue Mar 8, 2024
This moves current_fast_clear() up so that the current thread state is
NULL while running tstate_delete_common().
colesbury added a commit that referenced this issue Mar 11, 2024
…16517)

This moves `current_fast_clear()` up so that the current thread state is
`NULL` while running `tstate_delete_common()`.

This doesn't fix any bugs, but it means that we are more consistent that
`_PyThreadState_GET() != NULL` means that the thread is "attached".
adorilson pushed a commit to adorilson/cpython that referenced this issue Mar 25, 2024
…() (python#116517)

This moves `current_fast_clear()` up so that the current thread state is
`NULL` while running `tstate_delete_common()`.

This doesn't fix any bugs, but it means that we are more consistent that
`_PyThreadState_GET() != NULL` means that the thread is "attached".
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
…() (python#116517)

This moves `current_fast_clear()` up so that the current thread state is
`NULL` while running `tstate_delete_common()`.

This doesn't fix any bugs, but it means that we are more consistent that
`_PyThreadState_GET() != NULL` means that the thread is "attached".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-C-API type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

1 participant