From 7f85a2bdbc6072e165af8dee629ecb8d04e6a99d Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Fri, 14 Jun 2024 16:05:51 -0600 Subject: [PATCH 1/3] Avoid a race on _PyRuntime.types.managed_static.types[i].interp_count. --- Include/internal/pycore_typeobject.h | 1 + Objects/typeobject.c | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h index 32bd19d968b917..402cae784303a7 100644 --- a/Include/internal/pycore_typeobject.h +++ b/Include/internal/pycore_typeobject.h @@ -31,6 +31,7 @@ struct _types_runtime_state { unsigned int next_version_tag; struct { + PyMutex mutex; struct { PyTypeObject *type; int64_t interp_count; diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 98e00bd25c3205..524d350ce70672 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -244,9 +244,11 @@ managed_static_type_state_init(PyInterpreterState *interp, PyTypeObject *self, ? index : index + _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES; + PyMutex_Lock(&_PyRuntime.types.managed_static.mutex); assert((initial == 1) == (_PyRuntime.types.managed_static.types[full_index].interp_count == 0)); _PyRuntime.types.managed_static.types[full_index].interp_count += 1; + PyMutex_Unlock(&_PyRuntime.types.managed_static.mutex); if (initial) { assert(_PyRuntime.types.managed_static.types[full_index].type == NULL); @@ -300,7 +302,9 @@ managed_static_type_state_clear(PyInterpreterState *interp, PyTypeObject *self, state->type = NULL; assert(state->tp_weaklist == NULL); // It was already cleared out. + PyMutex_Lock(&_PyRuntime.types.managed_static.mutex); _PyRuntime.types.managed_static.types[full_index].interp_count -= 1; + PyMutex_Unlock(&_PyRuntime.types.managed_static.mutex); if (final) { assert(!_PyRuntime.types.managed_static.types[full_index].interp_count); _PyRuntime.types.managed_static.types[full_index].type = NULL; From e459043662769c03b22de498644dabe32972cb9e Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 17 Jun 2024 10:23:39 -0600 Subject: [PATCH 2/3] Stop skipping the stress test. --- Lib/test/test_interpreters/test_stress.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_interpreters/test_stress.py b/Lib/test/test_interpreters/test_stress.py index 40d2d77a7b9d3e..e400535b2a0e4e 100644 --- a/Lib/test/test_interpreters/test_stress.py +++ b/Lib/test/test_interpreters/test_stress.py @@ -22,7 +22,6 @@ def test_create_many_sequential(self): interp = interpreters.create() alive.append(interp) - @unittest.skip('(temporary) gh-120524: there is a race that needs fixing') @support.requires_resource('cpu') def test_create_many_threaded(self): alive = [] From f6a527bef7a8af2c62382f74aa518d9aa07b3cb7 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 17 Jun 2024 10:59:52 -0600 Subject: [PATCH 3/3] Use _Py_atomic_add_int64() instead of a mutex. --- Include/internal/pycore_typeobject.h | 1 - Objects/typeobject.c | 10 ++++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/Include/internal/pycore_typeobject.h b/Include/internal/pycore_typeobject.h index 402cae784303a7..32bd19d968b917 100644 --- a/Include/internal/pycore_typeobject.h +++ b/Include/internal/pycore_typeobject.h @@ -31,7 +31,6 @@ struct _types_runtime_state { unsigned int next_version_tag; struct { - PyMutex mutex; struct { PyTypeObject *type; int64_t interp_count; diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 86ba88c2560aa3..958f42430c80a2 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -244,11 +244,10 @@ managed_static_type_state_init(PyInterpreterState *interp, PyTypeObject *self, ? index : index + _Py_MAX_MANAGED_STATIC_BUILTIN_TYPES; - PyMutex_Lock(&_PyRuntime.types.managed_static.mutex); assert((initial == 1) == (_PyRuntime.types.managed_static.types[full_index].interp_count == 0)); - _PyRuntime.types.managed_static.types[full_index].interp_count += 1; - PyMutex_Unlock(&_PyRuntime.types.managed_static.mutex); + (void)_Py_atomic_add_int64( + &_PyRuntime.types.managed_static.types[full_index].interp_count, 1); if (initial) { assert(_PyRuntime.types.managed_static.types[full_index].type == NULL); @@ -302,9 +301,8 @@ managed_static_type_state_clear(PyInterpreterState *interp, PyTypeObject *self, state->type = NULL; assert(state->tp_weaklist == NULL); // It was already cleared out. - PyMutex_Lock(&_PyRuntime.types.managed_static.mutex); - _PyRuntime.types.managed_static.types[full_index].interp_count -= 1; - PyMutex_Unlock(&_PyRuntime.types.managed_static.mutex); + (void)_Py_atomic_add_int64( + &_PyRuntime.types.managed_static.types[full_index].interp_count, -1); if (final) { assert(!_PyRuntime.types.managed_static.types[full_index].interp_count); _PyRuntime.types.managed_static.types[full_index].type = NULL;