Issue #608: Enable free-threading

This changeset fixes a number of small issues in the free-threading
support, and enables GIL-less operation by default.

This is the first point where tests in pyobjc-core pass with the
free-threaded build.

Next up: actually testing threaded usage.

pyobjc-core/Modules/objc/OC_PythonDictionary.m

@@ -52,31 +52,40 @@ - (void)dealloc
 - (id _Nullable)nextObject
 {
     id        key   = nil;
+    int       rv;
     PyObject* pykey = NULL;
 
     PyObjC_BEGIN_WITH_GIL
         PyObject* dct = [value __pyobjc_PythonObject__];
 
         Py_BEGIN_CRITICAL_SECTION(dct);
 
-        if (unlikely(!PyDict_Next(dct, &pos, &pykey, NULL))) {
+        rv = PyDict_Next(dct, &pos, &pykey, NULL);
+        if (rv) {
+            Py_XINCREF(pykey);
+            valid = YES;
+        } else {
+            valid = NO;
+        }
+
+        Py_END_CRITICAL_SECTION();
+
+        if (!rv) {
             key = nil;
 
         } else if (pykey == Py_None) {
             key = [NSNull null];
+            Py_DECREF(pykey);
 
         } else {
             if (depythonify_c_value(@encode(id), pykey, &key) == -1) {
                 Py_DECREF(dct);
-                Py_EXIT_CRITICAL_SECTION();
+                Py_DECREF(pykey);
                 PyObjC_GIL_FORWARD_EXC();
             }
+            Py_DECREF(pykey);
         }
 
-        valid = (key != nil) ? YES : NO;
-
-        Py_END_CRITICAL_SECTION();
-
         Py_DECREF(dct);
 
     PyObjC_END_WITH_GIL

pyobjc-core/Modules/objc/class-builder.m

@@ -119,16 +119,20 @@ static Class _Nullable build_intermediate_class(Class base_class, char* name)
 
 #ifdef Py_GIL_DISABLED
     PyMutex_Lock(&intermediate_mutex);
-
 #endif
+
     /*
      * The naming style for the intermediate class ensures that the class
      * we've found has the correct parent, so no need to check for this in
      * release mode.
      */
     intermediate_class = objc_lookUpClass(name);
     if (intermediate_class != Nil) {
+#ifdef Py_GIL_DISABLED
+        PyMutex_Unlock(&intermediate_mutex);
+#endif
         PyObjC_Assert(class_getSuperclass(intermediate_class) == base_class, Nil);
+
         return intermediate_class;
     }
 

pyobjc-core/Modules/objc/file_wrapper.m

@@ -59,21 +59,26 @@
 static PyObject* _Nullable file_close(PyObject* _self)
 {
     struct file_object* self = (struct file_object*)_self;
+    FILE* fp;
 
     Py_BEGIN_CRITICAL_SECTION(_self);
+    fp = self->fp;
+    self->fp = NULL;
+    Py_END_CRITICAL_SECTION();
 
-    if (self->fp == NULL) {
+    if (fp == NULL) {
         PyErr_SetString(PyExc_ValueError, "Closing closed file");
         return NULL;
     }
 
-    if (fclose(self->fp) < 0) {
+    if (fclose(fp) < 0) {
+    /* This is very unlikely, restore previous value */
+        Py_BEGIN_CRITICAL_SECTION(_self);
+        self->fp = fp;
+        Py_END_CRITICAL_SECTION();
         return PyErr_SetFromErrno(PyExc_OSError);
     }
 
-    self->fp = NULL;
-
-    Py_END_CRITICAL_SECTION();
 
     Py_RETURN_NONE;
 }
@@ -111,18 +116,20 @@
 
     if (self->fp == NULL) {
         PyErr_SetString(PyExc_ValueError, "Using closed file");
-        Py_EXIT_CRITICAL_SECTION();
-        return NULL;
+        result = -1;
     } else {
-        result = ferror(self->fp);
+        result = ferror(self->fp) != 0;
     }
 
     Py_END_CRITICAL_SECTION();
 
-    if (result) {
-        Py_RETURN_TRUE;
-    } else {
+    switch (result) {
+    case -1:
+        return NULL;
+    case 0:
         Py_RETURN_FALSE;
+    default:
+        Py_RETURN_TRUE;
     }
 }
 
@@ -135,17 +142,19 @@
 
     if (self->fp == NULL) {
         PyErr_SetString(PyExc_ValueError, "Using closed file");
-        Py_EXIT_CRITICAL_SECTION();
-        return NULL;
+        result = -1;
     } else {
-        result = feof(self->fp);
+        result = feof(self->fp) != 0;
     }
     Py_END_CRITICAL_SECTION();
 
-    if (result) {
-        Py_RETURN_TRUE;
-    } else {
+    switch (result) {
+    case -1:
+        return NULL;
+    case 0:
         Py_RETURN_FALSE;
+    default:
+        Py_RETURN_TRUE;
     }
 }
 
@@ -257,6 +266,7 @@
     Py_BEGIN_CRITICAL_SECTION(_self);
     if (self->fp == NULL) {
         PyErr_SetString(PyExc_ValueError, "Using closed file");
+        Py_EXIT_CRITICAL_SECTION();
         return NULL;
     } else {
         result = fgets(buffer, 2048, self->fp);

pyobjc-core/Modules/objc/module.m

@@ -370,12 +370,12 @@ + (void)targetForBecomingMultiThreaded:(id)sender
              * there's not much we can do if releasing raises.
              */
 #ifdef Py_GIL_DISABLED
-    PyMutex_Lock(&global_release_pool_lock);
+            PyMutex_Lock(&global_release_pool_lock);
 #endif
             pool                = global_release_pool;
             global_release_pool = nil;
 #ifdef Py_GIL_DISABLED
-    PyMutex_Unlock(&global_release_pool_lock);
+            PyMutex_Unlock(&global_release_pool_lock);
 #endif
             [pool release];
 
@@ -399,17 +399,20 @@ + (void)targetForBecomingMultiThreaded:(id)sender
                                                     __attribute__((__unused__)))
 {
     NSAutoreleasePool* pool;
-#ifdef Py_GIL_DISABLED
-    PyMutex_Lock(&global_release_pool_lock);
-#endif
     Py_BEGIN_ALLOW_THREADS
         @try {
             /* Unconditionally set global_release_pool to nil
              * before calling release. There's not much we can
              * do if draining fails with an exception.
              */
+#ifdef Py_GIL_DISABLED
+    PyMutex_Lock(&global_release_pool_lock);
+#endif
             pool                = global_release_pool;
             global_release_pool = nil;
+#ifdef Py_GIL_DISABLED
+    PyMutex_Unlock(&global_release_pool_lock);
+#endif
             [pool release];
 
         } @catch (NSObject* localException) {
@@ -421,9 +424,6 @@ + (void)targetForBecomingMultiThreaded:(id)sender
          */
         [OC_NSAutoreleasePoolCollector newAutoreleasePool];
     Py_END_ALLOW_THREADS
-#ifdef Py_GIL_DISABLED
-    PyMutex_Unlock(&global_release_pool_lock);
-#endif
 
     if (PyErr_Occurred())
         return NULL;
@@ -2388,7 +2388,7 @@ static int mod_exec_module(PyObject* m)
 #if PY_VERSION_HEX >= 0x030d0000
     {
         .slot = Py_mod_gil,
-        .value = Py_MOD_GIL_USED,
+        .value = Py_MOD_GIL_NOT_USED,
     },
 #endif
     {  /* Sentinel */

pyobjc-core/Modules/objc/objc-class.m

@@ -1315,7 +1315,7 @@ static Class _Nullable objc_metaclass_locate(PyObject* meta_class)
             } else {
                 should_call = 0;
             }
-            Py_END_CRITICAL_SECTION(info);
+            Py_END_CRITICAL_SECTION();
 
             if (should_call) {
 #endif
@@ -3219,9 +3219,7 @@ Class _Nullable PyObjCClass_GetClass(PyObject* cls)
     if (PyObjCClass_Check(cls)) {
         Class result = Nil;
 
-        Py_BEGIN_CRITICAL_SECTION(cls);
         result = ((PyObjCClassObject*)cls)->class;
-        Py_END_CRITICAL_SECTION();
 
         if (result == Nil) {
             /* XXX: Audit callers, the field can be Nil */

pyobjc-core/Modules/objc/proxy-registry.m

@@ -242,6 +242,8 @@ static void weak_value_release(NSMapTable* table __attribute__((__unused__)), vo
 
 id _Nullable NS_RETURNS_RETAINED PyObjC_FindObjCProxy(PyObject* original)
 {
+    id result;
+
     if (original == Py_None)       // LCOV_BR_EXCL_LINE
         PyObjCErr_InternalError(); // LCOV_EXCL_LINE
 
@@ -251,10 +253,10 @@ id _Nullable NS_RETURNS_RETAINED PyObjC_FindObjCProxy(PyObject* original)
 
     struct weak_value* record  = NSMapGet(objc_proxies, original);
     if (record == NULL) {
-        return nil;
+        result = nil ;
+    } else {
+        result =  objc_loadWeakRetained(&record->value);
     }
-
-    id result =  objc_loadWeakRetained(&record->value);
 #ifdef Py_GIL_DISABLED
     PyMutex_Unlock(&proxy_mutex);
 #endif

pyobjc-core/Modules/objc/pyobjc-compat.h

@@ -235,11 +235,11 @@ _PyObjCTuple_GetItem(PyObject* tuple, Py_ssize_t idx)
     } while (0)
 
 #if PY_VERSION_HEX < 0x030d0000
-#define Py_BEGIN_CRITICAL_SECTION(value) {
+#define Py_BEGIN_CRITICAL_SECTION(value) { (void)(value);
 #define Py_END_CRITICAL_SECTION() }
 #define Py_EXIT_CRITICAL_SECTION() ((void)0)
 
-#define Py_BEGIN_CRITICAL_SECTION2(value1, value2) {
+#define Py_BEGIN_CRITICAL_SECTION2(value1, value2) { (void)((value1),(value2));
 #define Py_END_CRITICAL_SECTION2() }
 #define Py_EXIT_CRITICAL_SECTION2() ((void)0)
 

pyobjc-core/Modules/objc/selector.m

@@ -253,11 +253,11 @@
     }
 
     if (PyObject_IsTrue(newVal)) {
-        Py_BEGIN_CRITICAL_SECTION(self);
+        Py_BEGIN_CRITICAL_SECTION(_self);
         ((PyObjCSelector*)_self)->sel_flags |= PyObjCSelector_kHIDDEN;
         Py_END_CRITICAL_SECTION();
     } else {
-        Py_BEGIN_CRITICAL_SECTION(self);
+        Py_BEGIN_CRITICAL_SECTION(_self);
         ((PyObjCSelector*)_self)->sel_flags &= ~PyObjCSelector_kHIDDEN;
         Py_END_CRITICAL_SECTION();
     }

pyobjc-core/Modules/objc/super-call.m

@@ -195,12 +195,13 @@
     Py_DECREF(entry);
 
     PyObjC_MappingCount += 1;
+
+exit:
 #ifdef Py_GIL_DISABLED
     PyMutex_Unlock(&registry_mutex);
 #endif
-
-exit:
     return retval;
+
 }
 
 int
@@ -391,16 +392,16 @@
      struct registry* rv = PyCapsule_GetPointer(result, "objc.__memblock__");
      Py_DECREF(result);
 #ifdef Py_GIL_DISABLED
-    PyMutex_Unlock(&registry_mutex);
+     PyMutex_Unlock(&registry_mutex);
 #endif
      return rv;
 
 error:
-    Py_CLEAR(special_class);
+     Py_CLEAR(special_class);
      Py_CLEAR(py_selname);
      Py_CLEAR(search_class);
 #ifdef Py_GIL_DISABLED
-    PyMutex_Unlock(&registry_mutex);
+     PyMutex_Unlock(&registry_mutex);
 #endif
      return NULL;
 }
@@ -420,7 +421,7 @@
 #endif
     PyObject* key = PyBytes_FromStringAndSize(NULL, strlen(signature) + 10);
     if (key == NULL) { // LCOV_BR_EXCL_LINE
-        return NULL;   // LCOV_EXCL_LINE
+        goto exit;
     }
 
     res = PyObjCRT_SimplifySignature(signature, PyBytes_AS_STRING(key),
@@ -456,9 +457,6 @@ PyObjC_CallFunc _Nullable PyObjC_FindCallFunc(Class class, SEL sel, const char*
 
     PyObjC_Assert(special_registry != NULL, NULL);
 
-#ifdef Py_GIL_DISABLED
-    PyMutex_Lock(&registry_mutex);
-#endif
     special = search_special(class, sel);
     if (special) {
         result = special->call_to_objc;
@@ -475,9 +473,6 @@ PyObjC_CallFunc _Nullable PyObjC_FindCallFunc(Class class, SEL sel, const char*
         }
     }
 
-#ifdef Py_GIL_DISABLED
-    PyMutex_Unlock(&registry_mutex);
-#endif
     return result;
 }