pythongh-119344: Make critical section API public

colesbury · May 21, 2024 · 861f3a6 · 861f3a6
1 parent f6da790
commit 861f3a6
Show file tree

Hide file tree

Showing 10 changed files with 184 additions and 51 deletions.
diff --git a/Include/Python.h b/Include/Python.h
@@ -119,6 +119,7 @@
 #include "import.h"
 #include "abstract.h"
 #include "bltinmodule.h"
+#include "critical_section.h"
 #include "cpython/pyctype.h"
 #include "pystrtod.h"
 #include "pystrcmp.h"

diff --git a/Include/cpython/critical_section.h b/Include/cpython/critical_section.h
@@ -0,0 +1,75 @@
+#ifndef Py_CPYTHON_CRITICAL_SECTION_H
+#  error "this header file must not be included directly"
+#endif
+
+// Python critical sections.
+//
+// These operations are no-ops in the default build. See
+// pycore_critical_section.h for details.
+
+typedef struct _PyCriticalSection _PyCriticalSection;
+typedef struct _PyCriticalSection2 _PyCriticalSection2;
+
+#ifndef Py_GIL_DISABLED
+# define Py_BEGIN_CRITICAL_SECTION(op)      \
+    {
+# define Py_END_CRITICAL_SECTION()          \
+    }
+# define Py_BEGIN_CRITICAL_SECTION2(a, b)   \
+    {
+# define Py_END_CRITICAL_SECTION2()         \
+    }
+#else /* !Py_GIL_DISABLED */
+
+// (private)
+struct _PyCriticalSection {
+    // Tagged pointer to an outer active critical section (or 0).
+    uintptr_t prev;
+
+    // Mutex used to protect critical section
+    struct _PyMutex *mutex;
+};
+
+// (private) A critical section protected by two mutexes. Use
+// Py_BEGIN_CRITICAL_SECTION2 and Py_END_CRITICAL_SECTION2.
+struct _PyCriticalSection2 {
+    struct _PyCriticalSection base;
+
+    struct _PyMutex *mutex2;
+};
+
+# define Py_BEGIN_CRITICAL_SECTION(op)                                  \
+    {                                                                   \
+        _PyCriticalSection _cs;                                         \
+        _PyCriticalSection_Begin(&_cs, _PyObject_CAST(op))
+
+# define Py_END_CRITICAL_SECTION()                                      \
+        _PyCriticalSection_End(&_cs);                                   \
+    }
+
+# define Py_BEGIN_CRITICAL_SECTION2(a, b)                               \
+    {                                                                   \
+        _PyCriticalSection2 _cs2;                                       \
+        _PyCriticalSection2_Begin(&_cs2, _PyObject_CAST(a), _PyObject_CAST(b))
+
+# define Py_END_CRITICAL_SECTION2()                                     \
+        _PyCriticalSection2_End(&_cs2);                                 \
+    }
+
+#endif
+
+// (private)
+PyAPI_FUNC(void)
+_PyCriticalSection_Begin(_PyCriticalSection *c, PyObject *op);
+
+// (private)
+PyAPI_FUNC(void)
+_PyCriticalSection_End(_PyCriticalSection *c);
+
+// CPython internals should use pycore_critical_section.h instead.
+#ifdef Py_BUILD_CORE
+# undef Py_BEGIN_CRITICAL_SECTION
+# undef Py_END_CRITICAL_SECTION
+# undef Py_BEGIN_CRITICAL_SECTION2
+# undef Py_END_CRITICAL_SECTION2
+#endif
diff --git a/Include/critical_section.h b/Include/critical_section.h
@@ -0,0 +1,16 @@
+#ifndef Py_CRITICAL_SECTION_H
+#define Py_CRITICAL_SECTION_H
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#ifndef Py_LIMITED_API
+#  define Py_CPYTHON_CRITICAL_SECTION_H
+#  include "cpython/critical_section.h"
+#  undef Py_CPYTHON_CRITICAL_SECTION_H
+#endif
+
+#ifdef __cplusplus
+}
+#endif
+#endif /* !Py_CRITICAL_SECTION_H */
diff --git a/Include/internal/pycore_critical_section.h b/Include/internal/pycore_critical_section.h
@@ -90,80 +90,60 @@ extern "C" {
 # define Py_BEGIN_CRITICAL_SECTION_MUT(mutex)                           \
     {                                                                   \
         _PyCriticalSection _cs;                                         \
-        _PyCriticalSection_Begin(&_cs, mutex)
+        _PyCriticalSection_BeginInline(&_cs, mutex)
 
 # define Py_BEGIN_CRITICAL_SECTION(op)                                  \
         Py_BEGIN_CRITICAL_SECTION_MUT(&_PyObject_CAST(op)->ob_mutex)
 
 # define Py_END_CRITICAL_SECTION()                                      \
-        _PyCriticalSection_End(&_cs);                                   \
+        _PyCriticalSection_EndInline(&_cs);                             \
     }
 
 # define Py_BEGIN_CRITICAL_SECTION2(a, b)                               \
     {                                                                   \
         _PyCriticalSection2 _cs2;                                       \
-        _PyCriticalSection2_Begin(&_cs2, &_PyObject_CAST(a)->ob_mutex, &_PyObject_CAST(b)->ob_mutex)
+        _PyCriticalSection2_BeginInline(&_cs2,                          \
+                                        &_PyObject_CAST(a)->ob_mutex,   \
+                                        &_PyObject_CAST(b)->ob_mutex)
 
 # define Py_END_CRITICAL_SECTION2()                                     \
-        _PyCriticalSection2_End(&_cs2);                                 \
+        _PyCriticalSection2_EndInline(&_cs2);                           \
     }
 
 // Asserts that the mutex is locked.  The mutex must be held by the
 // top-most critical section otherwise there's the possibility
 // that the mutex would be swalled out in some code paths.
-#define  _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(mutex) \
+#define _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(mutex) \
     _PyCriticalSection_AssertHeld(mutex)
 
 // Asserts that the mutex for the given object is locked. The mutex must
 // be held by the top-most critical section otherwise there's the
 // possibility that the mutex would be swalled out in some code paths.
 #ifdef Py_DEBUG
 
-#define  _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op)                           \
+# define _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op)                           \
     if (Py_REFCNT(op) != 1) {                                                    \
         _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&_PyObject_CAST(op)->ob_mutex); \
     }
 
 #else   /* Py_DEBUG */
 
-#define  _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op)
+# define _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op)
 
 #endif  /* Py_DEBUG */
 
 #else  /* !Py_GIL_DISABLED */
 // The critical section APIs are no-ops with the GIL.
-# define Py_BEGIN_CRITICAL_SECTION_MUT(mut)
-# define Py_BEGIN_CRITICAL_SECTION(op)
-# define Py_END_CRITICAL_SECTION()
-# define Py_BEGIN_CRITICAL_SECTION2(a, b)
-# define Py_END_CRITICAL_SECTION2()
+# define Py_BEGIN_CRITICAL_SECTION_MUT(mut) {
+# define Py_BEGIN_CRITICAL_SECTION(op) {
+# define Py_END_CRITICAL_SECTION() }
+# define Py_BEGIN_CRITICAL_SECTION2(a, b) {
+# define Py_END_CRITICAL_SECTION2() }
 # define _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(mutex)
 # define _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(op)
 #endif  /* !Py_GIL_DISABLED */
 
-typedef struct {
-    // Tagged pointer to an outer active critical section (or 0).
-    // The two least-significant-bits indicate whether the pointed-to critical
-    // section is inactive and whether it is a _PyCriticalSection2 object.
-    uintptr_t prev;
-
-    // Mutex used to protect critical section
-    PyMutex *mutex;
-} _PyCriticalSection;
-
-// A critical section protected by two mutexes. Use
-// _PyCriticalSection2_Begin and _PyCriticalSection2_End.
-typedef struct {
-    _PyCriticalSection base;
-
-    PyMutex *mutex2;
-} _PyCriticalSection2;
-
-static inline int
-_PyCriticalSection_IsActive(uintptr_t tag)
-{
-    return tag != 0 && (tag & _Py_CRITICAL_SECTION_INACTIVE) == 0;
-}
+typedef struct _PyCriticalSection2 _PyCriticalSection2;
 
 // Resumes the top-most critical section.
 PyAPI_FUNC(void)
@@ -177,8 +157,19 @@ PyAPI_FUNC(void)
 _PyCriticalSection2_BeginSlow(_PyCriticalSection2 *c, PyMutex *m1, PyMutex *m2,
                              int is_m1_locked);
 
+PyAPI_FUNC(void)
+_PyCriticalSection_SuspendAll(PyThreadState *tstate);
+
+#ifdef Py_GIL_DISABLED
+
+static inline int
+_PyCriticalSection_IsActive(uintptr_t tag)
+{
+    return tag != 0 && (tag & _Py_CRITICAL_SECTION_INACTIVE) == 0;
+}
+
 static inline void
-_PyCriticalSection_Begin(_PyCriticalSection *c, PyMutex *m)
+_PyCriticalSection_BeginInline(_PyCriticalSection *c, PyMutex *m)
 {
     if (PyMutex_LockFast(&m->v)) {
         PyThreadState *tstate = _PyThreadState_GET();
@@ -207,20 +198,20 @@ _PyCriticalSection_Pop(_PyCriticalSection *c)
 }
 
 static inline void
-_PyCriticalSection_End(_PyCriticalSection *c)
+_PyCriticalSection_EndInline(_PyCriticalSection *c)
 {
     PyMutex_Unlock(c->mutex);
     _PyCriticalSection_Pop(c);
 }
 
 static inline void
-_PyCriticalSection2_Begin(_PyCriticalSection2 *c, PyMutex *m1, PyMutex *m2)
+_PyCriticalSection2_BeginInline(_PyCriticalSection2 *c, PyMutex *m1, PyMutex *m2)
 {
     if (m1 == m2) {
         // If the two mutex arguments are the same, treat this as a critical
         // section with a single mutex.
         c->mutex2 = NULL;
-        _PyCriticalSection_Begin(&c->base, m1);
+        _PyCriticalSection_BeginInline(&c->base, m1);
         return;
     }
 
@@ -253,7 +244,7 @@ _PyCriticalSection2_Begin(_PyCriticalSection2 *c, PyMutex *m1, PyMutex *m2)
 }
 
 static inline void
-_PyCriticalSection2_End(_PyCriticalSection2 *c)
+_PyCriticalSection2_EndInline(_PyCriticalSection2 *c)
 {
     if (c->mutex2) {
         PyMutex_Unlock(c->mutex2);
@@ -262,11 +253,6 @@ _PyCriticalSection2_End(_PyCriticalSection2 *c)
     _PyCriticalSection_Pop(&c->base);
 }
 
-PyAPI_FUNC(void)
-_PyCriticalSection_SuspendAll(PyThreadState *tstate);
-
-#ifdef Py_GIL_DISABLED
-
 static inline void
 _PyCriticalSection_AssertHeld(PyMutex *mutex)
 {
@@ -285,7 +271,7 @@ _PyCriticalSection_AssertHeld(PyMutex *mutex)
 #endif
 }
 
-#endif
+#endif /* Py_GIL_DISABLED */
 
 #ifdef __cplusplus
 }

diff --git a/Makefile.pre.in b/Makefile.pre.in
@@ -1002,6 +1002,7 @@ PYTHON_HEADERS= \
 		$(srcdir)/Include/codecs.h \
 		$(srcdir)/Include/compile.h \
 		$(srcdir)/Include/complexobject.h \
+		$(srcdir)/Include/critical_section.h \
 		$(srcdir)/Include/descrobject.h \
 		$(srcdir)/Include/dictobject.h \
 		$(srcdir)/Include/dynamic_annotations.h \
@@ -1078,6 +1079,7 @@ PYTHON_HEADERS= \
 		$(srcdir)/Include/cpython/compile.h \
 		$(srcdir)/Include/cpython/complexobject.h \
 		$(srcdir)/Include/cpython/context.h \
+		$(srcdir)/Include/cpython/critical_section.h \
 		$(srcdir)/Include/cpython/descrobject.h \
 		$(srcdir)/Include/cpython/dictobject.h \
 		$(srcdir)/Include/cpython/fileobject.h \

diff --git a/Misc/NEWS.d/next/C API/2024-05-21-19-41-41.gh-issue-119344.QKvzQb.rst b/Misc/NEWS.d/next/C API/2024-05-21-19-41-41.gh-issue-119344.QKvzQb.rst
@@ -0,0 +1 @@
+The critical section API is now public as part of the non-limited C API.
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
@@ -65,20 +65,20 @@ class object "PyObject *" "&PyBaseObject_Type"
 #define BEGIN_TYPE_LOCK()                                               \
     {                                                                   \
         _PyCriticalSection _cs;                                         \
-        _PyCriticalSection_Begin(&_cs, TYPE_LOCK);                      \
+        _PyCriticalSection_BeginInline(&_cs, TYPE_LOCK);                \
 
 #define END_TYPE_LOCK()                                                 \
-        _PyCriticalSection_End(&_cs);                                   \
+        _PyCriticalSection_EndInline(&_cs);                             \
     }
 
 #define BEGIN_TYPE_DICT_LOCK(d)                                         \
     {                                                                   \
         _PyCriticalSection2 _cs;                                        \
-        _PyCriticalSection2_Begin(&_cs, TYPE_LOCK,                      \
-                                  &_PyObject_CAST(d)->ob_mutex);        \
+        _PyCriticalSection2_BeginInline(&_cs, TYPE_LOCK,                \
+                                        &_PyObject_CAST(d)->ob_mutex);  \
 
 #define END_TYPE_DICT_LOCK()                                            \
-        _PyCriticalSection2_End(&_cs);                                  \
+        _PyCriticalSection2_EndInline(&_cs);                            \
     }
 
 #define ASSERT_TYPE_LOCK_HELD() \

diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj
@@ -135,6 +135,7 @@
     <ClInclude Include="..\Include\codecs.h" />
     <ClInclude Include="..\Include\compile.h" />
     <ClInclude Include="..\Include\complexobject.h" />
+    <ClInclude Include="..\Include\critical_section.h" />
     <ClInclude Include="..\Include\cpython\abstract.h" />
     <ClInclude Include="..\Include\cpython\bytearrayobject.h" />
     <ClInclude Include="..\Include\cpython\bytesobject.h" />
@@ -145,6 +146,7 @@
     <ClInclude Include="..\Include\cpython\compile.h" />
     <ClInclude Include="..\Include\cpython\complexobject.h" />
     <ClInclude Include="..\Include\cpython\context.h" />
+    <ClInclude Include="..\Include\cpython\critical_section.h" />
     <ClInclude Include="..\Include\cpython\descrobject.h" />
     <ClInclude Include="..\Include\cpython\dictobject.h" />
     <ClInclude Include="..\Include\cpython\fileobject.h" />

diff --git a/PCbuild/pythoncore.vcxproj.filters b/PCbuild/pythoncore.vcxproj.filters
@@ -72,6 +72,9 @@
     <ClInclude Include="..\Include\complexobject.h">
       <Filter>Include</Filter>
     </ClInclude>
+    <ClInclude Include="..\Include\critical_section.h">
+      <Filter>Include</Filter>
+    </ClInclude>
     <ClInclude Include="..\Include\datetime.h">
       <Filter>Include</Filter>
     </ClInclude>
@@ -372,6 +375,9 @@
     <ClInclude Include="..\Include\cpython\context.h">
       <Filter>Include\cpython</Filter>
     </ClInclude>
+    <ClInclude Include="..\Include\cpython\critical_section.h">
+      <Filter>Include\cpython</Filter>
+    </ClInclude>
     <ClInclude Include="..\Include\cpython\descrobject.h">
       <Filter>Include\cpython</Filter>
     </ClInclude>