[Python-checkins] bpo-39877: _PyRuntimeState.finalizing becomes atomic (GH-18816)

[Victor Stinner]

https://github.com/python/cpython/commit/7b3c252dc7f44d4bdc4c7c82d225ebd09c78f520
commit: 7b3c252dc7f44d4bdc4c7c82d225ebd09c78f520
branch: master
author: Victor Stinner <vstinner at python.org>
committer: GitHub <noreply at github.com>
date: 2020-03-07T00:24:23+01:00
summary:

bpo-39877: _PyRuntimeState.finalizing becomes atomic (GH-18816)

Convert _PyRuntimeState.finalizing field to an atomic variable:

* Rename it to _finalizing
* Change its type to _Py_atomic_address
* Add _PyRuntimeState_GetFinalizing() and _PyRuntimeState_SetFinalizing()
  functions
* Remove _Py_CURRENTLY_FINALIZING() function: replace it with testing
  directly _PyRuntimeState_GetFinalizing() value

Convert _PyRuntimeState_GetThreadState() to static inline function.

files:
M Include/internal/pycore_pystate.h
M Python/ceval.c
M Python/pylifecycle.c
M Python/pystate.c
M Python/sysmodule.c

diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h
index 405efb9f46060..b5f509547207d 100644
--- a/Include/internal/pycore_pystate.h
+++ b/Include/internal/pycore_pystate.h
@@ -223,8 +223,11 @@ typedef struct pyruntimestate {
     int initialized;
 
     /* Set by Py_FinalizeEx(). Only reset to NULL if Py_Initialize()
-       is called again. */
-    PyThreadState *finalizing;
+       is called again.
+
+       Use _PyRuntimeState_GetFinalizing() and _PyRuntimeState_SetFinalizing()
+       to access it, don't access it directly. */
+    _Py_atomic_address _finalizing;
 
     struct pyinterpreters {
         PyThread_type_lock mutex;
@@ -279,8 +282,15 @@ PyAPI_FUNC(PyStatus) _PyRuntime_Initialize(void);
 
 PyAPI_FUNC(void) _PyRuntime_Finalize(void);
 
-#define _Py_CURRENTLY_FINALIZING(runtime, tstate) \
-    (runtime->finalizing == tstate)
+static inline PyThreadState*
+_PyRuntimeState_GetFinalizing(_PyRuntimeState *runtime) {
+    return (PyThreadState*)_Py_atomic_load_relaxed(&runtime->_finalizing);
+}
+
+static inline void
+_PyRuntimeState_SetFinalizing(_PyRuntimeState *runtime, PyThreadState *tstate) {
+    _Py_atomic_store_relaxed(&runtime->_finalizing, (uintptr_t)tstate);
+}
 
 PyAPI_FUNC(int) _Py_IsMainInterpreter(PyThreadState* tstate);
 
@@ -288,8 +298,9 @@ PyAPI_FUNC(int) _Py_IsMainInterpreter(PyThreadState* tstate);
 /* Variable and macro for in-line access to current thread
    and interpreter state */
 
-#define _PyRuntimeState_GetThreadState(runtime) \
-    ((PyThreadState*)_Py_atomic_load_relaxed(&(runtime)->gilstate.tstate_current))
+static inline PyThreadState* _PyRuntimeState_GetThreadState(_PyRuntimeState *runtime) {
+    return (PyThreadState*)_Py_atomic_load_relaxed(&runtime->gilstate.tstate_current);
+}
 
 /* Get the current Python thread state.
 
diff --git a/Python/ceval.c b/Python/ceval.c
index ef4aac2f9abc8..20e32e224e7d4 100644
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -240,7 +240,8 @@ exit_thread_if_finalizing(PyThreadState *tstate)
 {
     _PyRuntimeState *runtime = tstate->interp->runtime;
     /* _Py_Finalizing is protected by the GIL */
-    if (runtime->finalizing != NULL && !_Py_CURRENTLY_FINALIZING(runtime, tstate)) {
+    PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(runtime);
+    if (finalizing != NULL && finalizing != tstate) {
         drop_gil(&runtime->ceval, tstate);
         PyThread_exit_thread();
     }
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index 84ced424499de..23d74ee9503aa 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.c
@@ -103,7 +103,7 @@ _PyRuntime_Finalize(void)
 int
 _Py_IsFinalizing(void)
 {
-    return _PyRuntime.finalizing != NULL;
+    return _PyRuntimeState_GetFinalizing(&_PyRuntime) != NULL;
 }
 
 /* Hack to force loading of object files */
@@ -507,7 +507,7 @@ pycore_init_runtime(_PyRuntimeState *runtime,
      * threads still hanging around from a previous Py_Initialize/Finalize
      * pair :(
      */
-    runtime->finalizing = NULL;
+    _PyRuntimeState_SetFinalizing(runtime, NULL);
 
     PyStatus status = _Py_HashRandomization_Init(config);
     if (_PyStatus_EXCEPTION(status)) {
@@ -1366,7 +1366,7 @@ Py_FinalizeEx(void)
 
     /* Remaining threads (e.g. daemon threads) will automatically exit
        after taking the GIL (in PyEval_RestoreThread()). */
-    runtime->finalizing = tstate;
+    _PyRuntimeState_SetFinalizing(runtime, tstate);
     runtime->initialized = 0;
     runtime->core_initialized = 0;
 
@@ -2131,8 +2131,9 @@ static void
 fatal_error_dump_runtime(FILE *stream, _PyRuntimeState *runtime)
 {
     fprintf(stream, "Python runtime state: ");
-    if (runtime->finalizing) {
-        fprintf(stream, "finalizing (tstate=%p)", runtime->finalizing);
+    PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(runtime);
+    if (finalizing) {
+        fprintf(stream, "finalizing (tstate=%p)", finalizing);
     }
     else if (runtime->initialized) {
         fprintf(stream, "initialized");
diff --git a/Python/pystate.c b/Python/pystate.c
index 4001c63ff25ee..900266919dfc4 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -292,7 +292,7 @@ PyInterpreterState_Clear(PyInterpreterState *interp)
     Py_CLEAR(interp->after_forkers_parent);
     Py_CLEAR(interp->after_forkers_child);
 #endif
-    if (runtime->finalizing == NULL) {
+    if (_PyRuntimeState_GetFinalizing(runtime) == NULL) {
         _PyWarnings_Fini(interp);
     }
     // XXX Once we have one allocator per interpreter (i.e.
diff --git a/Python/sysmodule.c b/Python/sysmodule.c
index bfacf314dfc9e..f086514a03243 100644
--- a/Python/sysmodule.c
+++ b/Python/sysmodule.c
@@ -289,8 +289,9 @@ _PySys_ClearAuditHooks(void)
     /* Must be finalizing to clear hooks */
     _PyRuntimeState *runtime = &_PyRuntime;
     PyThreadState *ts = _PyRuntimeState_GetThreadState(runtime);
-    assert(!ts || _Py_CURRENTLY_FINALIZING(runtime, ts));
-    if (!ts || !_Py_CURRENTLY_FINALIZING(runtime, ts)) {
+    PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(runtime);
+    assert(!ts || finalizing == ts);
+    if (!ts || finalizing != ts) {
         return;
     }