From de5ca0bf71760aad8f2b8449c89242498bff64c8 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 4 Apr 2024 14:09:38 -0400 Subject: gh-117435: Make `SemLock` thread-safe in free-threaded build (#117436) Use critical sections to make acquire, release, and _count thread-safe without the GIL. --- Modules/_multiprocessing/clinic/semaphore.c.h | 31 +++++++++++++++++++++++---- Modules/_multiprocessing/semaphore.c | 15 ++++++++----- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/Modules/_multiprocessing/clinic/semaphore.c.h b/Modules/_multiprocessing/clinic/semaphore.c.h index 7c85511..64e666b 100644 --- a/Modules/_multiprocessing/clinic/semaphore.c.h +++ b/Modules/_multiprocessing/clinic/semaphore.c.h @@ -6,6 +6,7 @@ preserve # include "pycore_gc.h" // PyGC_Head # include "pycore_runtime.h" // _Py_ID() #endif +#include "pycore_critical_section.h"// Py_BEGIN_CRITICAL_SECTION() #include "pycore_modsupport.h" // _PyArg_UnpackKeywords() #if defined(HAVE_MP_SEMAPHORE) && defined(MS_WINDOWS) @@ -75,7 +76,9 @@ _multiprocessing_SemLock_acquire(SemLockObject *self, PyObject *const *args, Py_ } timeout_obj = args[1]; skip_optional_pos: + Py_BEGIN_CRITICAL_SECTION(self); return_value = _multiprocessing_SemLock_acquire_impl(self, blocking, timeout_obj); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -100,7 +103,13 @@ _multiprocessing_SemLock_release_impl(SemLockObject *self); static PyObject * _multiprocessing_SemLock_release(SemLockObject *self, PyObject *Py_UNUSED(ignored)) { - return _multiprocessing_SemLock_release_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = _multiprocessing_SemLock_release_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } #endif /* defined(HAVE_MP_SEMAPHORE) && defined(MS_WINDOWS) */ @@ -172,7 +181,9 @@ _multiprocessing_SemLock_acquire(SemLockObject *self, PyObject *const *args, Py_ } timeout_obj = args[1]; skip_optional_pos: + Py_BEGIN_CRITICAL_SECTION(self); return_value = _multiprocessing_SemLock_acquire_impl(self, blocking, timeout_obj); + Py_END_CRITICAL_SECTION(); exit: return return_value; @@ -197,7 +208,13 @@ _multiprocessing_SemLock_release_impl(SemLockObject *self); static PyObject * _multiprocessing_SemLock_release(SemLockObject *self, PyObject *Py_UNUSED(ignored)) { - return _multiprocessing_SemLock_release_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = _multiprocessing_SemLock_release_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } #endif /* defined(HAVE_MP_SEMAPHORE) && !defined(MS_WINDOWS) */ @@ -340,7 +357,13 @@ _multiprocessing_SemLock__count_impl(SemLockObject *self); static PyObject * _multiprocessing_SemLock__count(SemLockObject *self, PyObject *Py_UNUSED(ignored)) { - return _multiprocessing_SemLock__count_impl(self); + PyObject *return_value = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); + return_value = _multiprocessing_SemLock__count_impl(self); + Py_END_CRITICAL_SECTION(); + + return return_value; } #endif /* defined(HAVE_MP_SEMAPHORE) */ @@ -542,4 +565,4 @@ exit: #ifndef _MULTIPROCESSING_SEMLOCK___EXIT___METHODDEF #define _MULTIPROCESSING_SEMLOCK___EXIT___METHODDEF #endif /* !defined(_MULTIPROCESSING_SEMLOCK___EXIT___METHODDEF) */ -/*[clinic end generated code: output=d57992037e6770b6 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=713b597256233716 input=a9049054013a1b77]*/ diff --git a/Modules/_multiprocessing/semaphore.c b/Modules/_multiprocessing/semaphore.c index f8f2afd..5bb055f 100644 --- a/Modules/_multiprocessing/semaphore.c +++ b/Modules/_multiprocessing/semaphore.c @@ -81,6 +81,7 @@ _GetSemaphoreValue(HANDLE handle, long *value) } /*[clinic input] +@critical_section _multiprocessing.SemLock.acquire block as blocking: bool = True @@ -92,7 +93,7 @@ Acquire the semaphore/lock. static PyObject * _multiprocessing_SemLock_acquire_impl(SemLockObject *self, int blocking, PyObject *timeout_obj) -/*[clinic end generated code: output=f9998f0b6b0b0872 input=e5b45f5cbb775166]*/ +/*[clinic end generated code: output=f9998f0b6b0b0872 input=079ca779975f3ad6]*/ { double timeout; DWORD res, full_msecs, nhandles; @@ -172,6 +173,7 @@ _multiprocessing_SemLock_acquire_impl(SemLockObject *self, int blocking, } /*[clinic input] +@critical_section _multiprocessing.SemLock.release Release the semaphore/lock. @@ -179,7 +181,7 @@ Release the semaphore/lock. static PyObject * _multiprocessing_SemLock_release_impl(SemLockObject *self) -/*[clinic end generated code: output=b22f53ba96b0d1db input=ba7e63a961885d3d]*/ +/*[clinic end generated code: output=b22f53ba96b0d1db input=9bd62d3645e7a531]*/ { if (self->kind == RECURSIVE_MUTEX) { if (!ISMINE(self)) { @@ -297,6 +299,7 @@ sem_timedwait_save(sem_t *sem, struct timespec *deadline, PyThreadState *_save) #endif /* !HAVE_SEM_TIMEDWAIT */ /*[clinic input] +@critical_section _multiprocessing.SemLock.acquire block as blocking: bool = True @@ -308,7 +311,7 @@ Acquire the semaphore/lock. static PyObject * _multiprocessing_SemLock_acquire_impl(SemLockObject *self, int blocking, PyObject *timeout_obj) -/*[clinic end generated code: output=f9998f0b6b0b0872 input=e5b45f5cbb775166]*/ +/*[clinic end generated code: output=f9998f0b6b0b0872 input=079ca779975f3ad6]*/ { int res, err = 0; struct timespec deadline = {0}; @@ -382,6 +385,7 @@ _multiprocessing_SemLock_acquire_impl(SemLockObject *self, int blocking, } /*[clinic input] +@critical_section _multiprocessing.SemLock.release Release the semaphore/lock. @@ -389,7 +393,7 @@ Release the semaphore/lock. static PyObject * _multiprocessing_SemLock_release_impl(SemLockObject *self) -/*[clinic end generated code: output=b22f53ba96b0d1db input=ba7e63a961885d3d]*/ +/*[clinic end generated code: output=b22f53ba96b0d1db input=9bd62d3645e7a531]*/ { if (self->kind == RECURSIVE_MUTEX) { if (!ISMINE(self)) { @@ -583,6 +587,7 @@ semlock_dealloc(SemLockObject* self) } /*[clinic input] +@critical_section _multiprocessing.SemLock._count Num of `acquire()`s minus num of `release()`s for this process. @@ -590,7 +595,7 @@ Num of `acquire()`s minus num of `release()`s for this process. static PyObject * _multiprocessing_SemLock__count_impl(SemLockObject *self) -/*[clinic end generated code: output=5ba8213900e517bb input=36fc59b1cd1025ab]*/ +/*[clinic end generated code: output=5ba8213900e517bb input=9fa6e0b321b16935]*/ { return PyLong_FromLong((long)self->count); } -- cgit v0.12