gh-115432: Add critical section variant that handles a NULL object (#115433)

colesbury · web-flow · commit ad4f909e0e78 · 2024-02-15T08:37:54.000-05:00
This adds `Py_XBEGIN_CRITICAL_SECTION` and
`Py_XEND_CRITICAL_SECTION`, which accept a possibly NULL object as an
argument. If the argument is NULL, then nothing is locked or unlocked.
Otherwise, they behave like `Py_BEGIN/END_CRITICAL_SECTION`.
diff --git a/Include/internal/pycore_critical_section.h b/Include/internal/pycore_critical_section.h
@@ -96,6 +96,15 @@ extern "C" {
         _PyCriticalSection_End(&_cs);                                   \
     }
 
+# define Py_XBEGIN_CRITICAL_SECTION(op)                                 \
+    {                                                                   \
+        _PyCriticalSection _cs_opt = {0};                               \
+        _PyCriticalSection_XBegin(&_cs_opt, _PyObject_CAST(op))
+
+# define Py_XEND_CRITICAL_SECTION()                                     \
+        _PyCriticalSection_XEnd(&_cs_opt);                              \
+    }
+
 # define Py_BEGIN_CRITICAL_SECTION2(a, b)                               \
     {                                                                   \
         _PyCriticalSection2 _cs2;                                       \
@@ -131,6 +140,8 @@ extern "C" {
 // The critical section APIs are no-ops with the GIL.
 # define Py_BEGIN_CRITICAL_SECTION(op)
 # define Py_END_CRITICAL_SECTION()
+# define Py_XBEGIN_CRITICAL_SECTION(op)
+# define Py_XEND_CRITICAL_SECTION()
 # define Py_BEGIN_CRITICAL_SECTION2(a, b)
 # define Py_END_CRITICAL_SECTION2()
 # define _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(mutex)
@@ -187,6 +198,16 @@ _PyCriticalSection_Begin(_PyCriticalSection *c, PyMutex *m)
     }
 }
 
+static inline void
+_PyCriticalSection_XBegin(_PyCriticalSection *c, PyObject *op)
+{
+#ifdef Py_GIL_DISABLED
+    if (op != NULL) {
+        _PyCriticalSection_Begin(c, &_PyObject_CAST(op)->ob_mutex);
+    }
+#endif
+}
+
 // Removes the top-most critical section from the thread's stack of critical
 // sections. If the new top-most critical section is inactive, then it is
 // resumed.
@@ -209,6 +230,14 @@ _PyCriticalSection_End(_PyCriticalSection *c)
     _PyCriticalSection_Pop(c);
 }
 
+static inline void
+_PyCriticalSection_XEnd(_PyCriticalSection *c)
+{
+    if (c->mutex) {
+        _PyCriticalSection_End(c);
+    }
+}
+
 static inline void
 _PyCriticalSection2_Begin(_PyCriticalSection2 *c, PyMutex *m1, PyMutex *m2)
 {
diff --git a/Modules/_testinternalcapi/test_critical_sections.c b/Modules/_testinternalcapi/test_critical_sections.c
@@ -49,6 +49,15 @@ test_critical_sections(PyObject *self, PyObject *Py_UNUSED(args))
     Py_END_CRITICAL_SECTION2();
     assert_nogil(!PyMutex_IsLocked(&d2->ob_mutex));
 
+    // Optional variant behaves the same if the object is non-NULL
+    Py_XBEGIN_CRITICAL_SECTION(d1);
+    assert_nogil(PyMutex_IsLocked(&d1->ob_mutex));
+    Py_XEND_CRITICAL_SECTION();
+
+    // No-op
+    Py_XBEGIN_CRITICAL_SECTION(NULL);
+    Py_XEND_CRITICAL_SECTION();
+
     Py_DECREF(d2);
     Py_DECREF(d1);
     Py_RETURN_NONE;
