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.

Sign up for GitHub

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

gh-103323: Get the "Current" Thread State from a Thread-Local Variable #103324

Merged
merged 12 commits into from
Apr 24, 2023
Merged

gh-103323: Get the "Current" Thread State from a Thread-Local Variable #103324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c4db85a
Add a thread_local macro.
ericsnowcurrently Jan 10, 2023
47a7094
tstate_current -> thread_local.
ericsnowcurrently Apr 6, 2023
cf22de1
Add _PyThraedState_GetCurrent().
ericsnowcurrently Apr 7, 2023
d4136d2
Add HAVE_THREAD_LOCAL.
ericsnowcurrently Apr 7, 2023
f8c6598
Support the faster approach, if available.
ericsnowcurrently Apr 7, 2023
9496df0
Do not fail if thread_local not supported.
ericsnowcurrently Apr 7, 2023
2c335a3
thread_local -> _Py_thread_local
ericsnowcurrently Apr 7, 2023
4af0ce7
Only define _Py_thread_local for the core runtime.
ericsnowcurrently Apr 7, 2023
3db4007
Fix pystate.c.
ericsnowcurrently Apr 7, 2023
d573053
Call _PyThreadState_GET() from _PyRuntimeState_GetThreadState().
ericsnowcurrently Apr 7, 2023
feb8ef5
Fix the error message.
ericsnowcurrently Apr 7, 2023
2332a2e
Add a NEWS entry.
ericsnowcurrently Apr 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions Include/internal/pycore_pystate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,27 +64,35 @@ _Py_ThreadCanHandlePendingCalls(void)
/* Variable and macro for in-line access to current thread
and interpreter state */

static inline PyThreadState*
_PyRuntimeState_GetThreadState(_PyRuntimeState *runtime)
{
return (PyThreadState*)_Py_atomic_load_relaxed(&runtime->tstate_current);
}
#if defined(HAVE_THREAD_LOCAL) && !defined(Py_BUILD_CORE_MODULE)
extern _Py_thread_local PyThreadState *_Py_tss_tstate;
#endif
PyAPI_DATA(PyThreadState *) _PyThreadState_GetCurrent(void);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the use case for this new _PyThreadState_GetCurrent() function? There is already PyThreadState_Get(). How is it different?

The API to get the current thread state is already complicated and has a complicated history: https://pythondev.readthedocs.io/pystate.html

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's there because I couldn't find a way to mix PyAPI_DATA with _Py_thread_local. Looks like that's the same issue you ran into in 2020.


/* Get the current Python thread state.

Efficient macro reading directly the 'tstate_current' atomic
variable. The macro is unsafe: it does not check for error and it can
return NULL.
This function is unsafe: it does not check for error and it can return NULL.

The caller must hold the GIL.

See also PyThreadState_Get() and _PyThreadState_UncheckedGet(). */
static inline PyThreadState*
_PyThreadState_GET(void)
{
return _PyRuntimeState_GetThreadState(&_PyRuntime);
#if defined(HAVE_THREAD_LOCAL) && !defined(Py_BUILD_CORE_MODULE)
return _Py_tss_tstate;
#else
return _PyThreadState_GetCurrent();
#endif
}

static inline PyThreadState*
_PyRuntimeState_GetThreadState(_PyRuntimeState *Py_UNUSED(runtime))
{
return _PyThreadState_GET();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function no longer makes sense: I wrote PR #104171 to remove it.

}


static inline void
_Py_EnsureFuncTstateNotNULL(const char *func, PyThreadState *tstate)
{
Expand Down
3 changes: 0 additions & 3 deletions Include/internal/pycore_runtime.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,6 @@ typedef struct pyruntimestate {

unsigned long main_thread;

/* Assuming the current thread holds the GIL, this is the
PyThreadState for the current thread. */
_Py_atomic_address tstate_current;
/* Used for the thread state bound to the current thread. */
Py_tss_t autoTSSkey;

Expand Down
21 changes: 21 additions & 0 deletions Include/pyport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,27 @@ extern char * _getpty(int *, int, mode_t, int);
# define WITH_THREAD
#endif

#ifdef WITH_THREAD
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is useless. This macro is now always defined. It's only kept for backward compatibility.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't it affect WASM builds?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the code 3 lines above:

#ifndef WITH_THREAD
#  define WITH_THREAD
#endif

# ifdef Py_BUILD_CORE
# ifdef HAVE_THREAD_LOCAL
# error "HAVE_THREAD_LOCAL is already defined"
# endif
# define HAVE_THREAD_LOCAL 1
# ifdef thread_local
# define _Py_thread_local thread_local
# elif __STDC_VERSION__ >= 201112L && !defined(__STDC_NO_THREADS__)
# define _Py_thread_local _Thread_local
# elif defined(_MSC_VER) /* AKA NT_THREADS */
# define _Py_thread_local __declspec(thread)
# elif defined(__GNUC__) /* includes clang */
# define _Py_thread_local __thread
# else
// fall back to the PyThread_tss_*() API, or ignore.
# undef HAVE_THREAD_LOCAL
# endif
# endif
#endif

/* Check that ALT_SOABI is consistent with Py_TRACE_REFS:
./configure --with-trace-refs should must be used to define Py_TRACE_REFS */
#if defined(ALT_SOABI) && defined(Py_TRACE_REFS)
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2023-04-07-12-18-41.gh-issue-103323.9802br.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
We've replaced our use of ``_PyRuntime.tstate_current`` with a thread-local
variable. This is a fairly low-level implementation detail, and there
should be no change in behavior.
38 changes: 32 additions & 6 deletions Python/pystate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,30 +60,56 @@ extern "C" {
For each of these functions, the GIL must be held by the current thread.
*/


#ifdef HAVE_THREAD_LOCAL
_Py_thread_local PyThreadState *_Py_tss_tstate = NULL;
#endif

static inline PyThreadState *
current_fast_get(_PyRuntimeState *runtime)
current_fast_get(_PyRuntimeState *Py_UNUSED(runtime))
{
return (PyThreadState*)_Py_atomic_load_relaxed(&runtime->tstate_current);
#ifdef HAVE_THREAD_LOCAL
return _Py_tss_tstate;
#else
// XXX Fall back to the PyThread_tss_*() API.
# error "no supported thread-local variable storage classifier"
#endif
}

static inline void
current_fast_set(_PyRuntimeState *runtime, PyThreadState *tstate)
current_fast_set(_PyRuntimeState *Py_UNUSED(runtime), PyThreadState *tstate)
{
assert(tstate != NULL);
_Py_atomic_store_relaxed(&runtime->tstate_current, (uintptr_t)tstate);
#ifdef HAVE_THREAD_LOCAL
_Py_tss_tstate = tstate;
#else
// XXX Fall back to the PyThread_tss_*() API.
# error "no supported thread-local variable storage classifier"
#endif
}

static inline void
current_fast_clear(_PyRuntimeState *runtime)
current_fast_clear(_PyRuntimeState *Py_UNUSED(runtime))
{
_Py_atomic_store_relaxed(&runtime->tstate_current, (uintptr_t)NULL);
#ifdef HAVE_THREAD_LOCAL
_Py_tss_tstate = NULL;
#else
// XXX Fall back to the PyThread_tss_*() API.
# error "no supported thread-local variable storage classifier"
#endif
}

#define tstate_verify_not_active(tstate) \
if (tstate == current_fast_get((tstate)->interp->runtime)) { \
_Py_FatalErrorFormat(__func__, "tstate %p is still current", tstate); \
}

PyThreadState *
_PyThreadState_GetCurrent(void)
{
return current_fast_get(&_PyRuntime);
}


//------------------------------------------------
// the thread state bound to the current OS thread
Expand Down