Skip to content

gh-109793: Allow Switching Interpreters During Finalization #109794

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

Merged
ericsnowcurrently merged 9 commits into from
Sep 27, 2023
Merged

gh-109793: Allow Switching Interpreters During Finalization #109794

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5e2224f
Add a test.
ericsnowcurrently Sep 23, 2023
a1014a9
Also check the thread ID in _PyThreadState_MustExit().
ericsnowcurrently Sep 23, 2023
d46103d
Use _PyRuntime directly in _Py_IsMainInterpreterFinalizing().
ericsnowcurrently Sep 23, 2023
2e13a75
Add _PyRuntimeState._finalizing_id and _PyInterpreterState._finalizin…
ericsnowcurrently Sep 24, 2023
756cc3b
Add verbose output to the test.
ericsnowcurrently Sep 25, 2023
ad312b9
Add a NEWS entry.
ericsnowcurrently Sep 25, 2023
8feeb4c
Skip the assert for now.
ericsnowcurrently Sep 25, 2023
9e589a7
Use the new _Py_atomic API.
ericsnowcurrently Sep 27, 2023
9829315
Fix the NEWS entry.
ericsnowcurrently Sep 27, 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
Prev Previous commit
Next Next commit
Use the new _Py_atomic API.
  • Loading branch information
@ericsnowcurrently
ericsnowcurrently committed Sep 27, 2023
commit 9e589a789ab49e78d764a669c88906b10ece441d
17 changes: 17 additions & 0 deletions Include/cpython/pyatomic.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,3 +501,20 @@ static inline void _Py_atomic_fence_release(void);
#else
# error "no available pyatomic implementation for this platform/compiler"
#endif


// --- aliases ---------------------------------------------------------------

#if SIZEOF_LONG == 8
# define _Py_atomic_load_ulong _Py_atomic_load_uint64
# define _Py_atomic_load_ulong_relaxed _Py_atomic_load_uint64_relaxed
# define _Py_atomic_store_ulong _Py_atomic_store_uint64
# define _Py_atomic_store_ulong_relaxed _Py_atomic_store_uint64_relaxed
#elif SIZEOF_LONG == 4
# define _Py_atomic_load_ulong _Py_atomic_load_uint32
# define _Py_atomic_load_ulong_relaxed _Py_atomic_load_uint32_relaxed
# define _Py_atomic_store_ulong _Py_atomic_store_uint32
# define _Py_atomic_store_ulong_relaxed _Py_atomic_store_uint32_relaxed
#else
# error "long must be 4 or 8 bytes in size"
#endif // SIZEOF_LONG
24 changes: 0 additions & 24 deletions Include/internal/pycore_atomic.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,6 @@ typedef struct _Py_atomic_address {
atomic_uintptr_t _value;
} _Py_atomic_address;

typedef struct _Py_atomic_ulong {
atomic_ulong _value;
} _Py_atomic_ulong;

typedef struct _Py_atomic_int {
atomic_int _value;
} _Py_atomic_int;
Expand Down Expand Up @@ -81,10 +77,6 @@ typedef struct _Py_atomic_address {
uintptr_t _value;
} _Py_atomic_address;

typedef struct _Py_atomic_ulong {
unsigned long _value;
} _Py_atomic_ulong;

typedef struct _Py_atomic_int {
int _value;
} _Py_atomic_int;
Expand Down Expand Up @@ -123,10 +115,6 @@ typedef struct _Py_atomic_address {
uintptr_t _value;
} _Py_atomic_address;

typedef struct _Py_atomic_ulong {
unsigned long _value;
} _Py_atomic_ulong;

typedef struct _Py_atomic_int {
int _value;
} _Py_atomic_int;
Expand Down Expand Up @@ -263,10 +251,6 @@ typedef struct _Py_atomic_address {
volatile uintptr_t _value;
} _Py_atomic_address;

typedef struct _Py_atomic_ulong {
volatile unsigned long _value;
} _Py_atomic_ulong;

typedef struct _Py_atomic_int {
volatile int _value;
} _Py_atomic_int;
Expand Down Expand Up @@ -403,10 +387,6 @@ typedef struct _Py_atomic_address {
volatile uintptr_t _value;
} _Py_atomic_address;

typedef struct _Py_atomic_ulong {
volatile unsigned long _value;
} _Py_atomic_ulong;

typedef struct _Py_atomic_int {
volatile int _value;
} _Py_atomic_int;
Expand Down Expand Up @@ -544,10 +524,6 @@ typedef struct _Py_atomic_address {
uintptr_t _value;
} _Py_atomic_address;

typedef struct _Py_atomic_ulong {
unsigned long _value;
} _Py_atomic_ulong;

typedef struct _Py_atomic_int {
int _value;
} _Py_atomic_int;
Expand Down
9 changes: 5 additions & 4 deletions Include/internal/pycore_interp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ struct _is {
to access it, don't access it directly. */
_Py_atomic_address _finalizing;
/* The ID of the OS thread in which we are finalizing. */
_Py_atomic_ulong _finalizing_id;
unsigned long _finalizing_id;

struct _gc_runtime_state gc;

Expand Down Expand Up @@ -219,19 +219,20 @@ _PyInterpreterState_GetFinalizing(PyInterpreterState *interp) {

static inline unsigned long
_PyInterpreterState_GetFinalizingID(PyInterpreterState *interp) {
return _Py_atomic_load_relaxed(&interp->_finalizing_id);
return _Py_atomic_load_ulong_relaxed(&interp->_finalizing_id);
}

static inline void
_PyInterpreterState_SetFinalizing(PyInterpreterState *interp, PyThreadState *tstate) {
_Py_atomic_store_relaxed(&interp->_finalizing, (uintptr_t)tstate);
if (tstate == NULL) {
_Py_atomic_store_relaxed(&interp->_finalizing_id, 0);
_Py_atomic_store_ulong_relaxed(&interp->_finalizing_id, 0);
}
else {
// XXX Re-enable this assert once gh-109860 is fixed.
//assert(tstate->thread_id == PyThread_get_thread_ident());
_Py_atomic_store_relaxed(&interp->_finalizing_id, tstate->thread_id);
_Py_atomic_store_ulong_relaxed(&interp->_finalizing_id,
tstate->thread_id);
}
}

Expand Down
9 changes: 5 additions & 4 deletions Include/internal/pycore_runtime.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ typedef struct pyruntimestate {
to access it, don't access it directly. */
_Py_atomic_address _finalizing;
/* The ID of the OS thread in which we are finalizing. */
_Py_atomic_ulong _finalizing_id;
unsigned long _finalizing_id;

struct pyinterpreters {
PyThread_type_lock mutex;
Expand Down Expand Up @@ -307,19 +307,20 @@ _PyRuntimeState_GetFinalizing(_PyRuntimeState *runtime) {

static inline unsigned long
_PyRuntimeState_GetFinalizingID(_PyRuntimeState *runtime) {
return _Py_atomic_load_relaxed(&runtime->_finalizing_id);
return _Py_atomic_load_ulong_relaxed(&runtime->_finalizing_id);
}

static inline void
_PyRuntimeState_SetFinalizing(_PyRuntimeState *runtime, PyThreadState *tstate) {
_Py_atomic_store_relaxed(&runtime->_finalizing, (uintptr_t)tstate);
if (tstate == NULL) {
_Py_atomic_store_relaxed(&runtime->_finalizing_id, 0);
_Py_atomic_store_ulong_relaxed(&runtime->_finalizing_id, 0);
}
else {
// XXX Re-enable this assert once gh-109860 is fixed.
//assert(tstate->thread_id == PyThread_get_thread_ident());
_Py_atomic_store_relaxed(&runtime->_finalizing_id, tstate->thread_id);
_Py_atomic_store_ulong_relaxed(&runtime->_finalizing_id,
tstate->thread_id);
}
}

Expand Down