More free threading updates

Issue #608 Slowly getting closer to the core bridge being ready to enable GIL-less operation.
ronaldoussoren · Sep 6, 2024 · 178c765 · 178c765
1 parent 5b2fbfb
commit 178c765
Show file tree

Hide file tree

Showing 19 changed files with 264 additions and 202 deletions.
diff --git a/pyobjc-core/Modules/objc/OC_NSBundleHack.m b/pyobjc-core/Modules/objc/OC_NSBundleHack.m
@@ -9,6 +9,8 @@
 
 /* This code is never used on systems I use for coverage testing */
 // LCOV_EXCL_START
+//
+
 
 static void
 nsmaptable_objc_retain(NSMapTable* table __attribute__((__unused__)), const void* datum)
@@ -43,48 +45,62 @@ + (NSBundle*)bundleForClass:(Class)aClass
     static NSMapTable* bundleCache = nil;
     id                 rval;
 
-    if (unlikely(!mainBundle)) {
-        mainBundle = [[NSBundle mainBundle] retain];
-    }
+#ifdef Py_GIL_DISABLED
+    /*
+     * NSMapTable isn't thread-safe, therefore use a lock to access it.
+     *
+     * This implementation uses @synchronized because the BundleHack class
+     * is unused an recent enough versions of macOS (see comment at the start
+     * of this file).
+     */
+    @synchronized(self) {
+#endif
+
+        if (unlikely(!mainBundle)) {
+            mainBundle = [[NSBundle mainBundle] retain];
+        }
 
-    if (unlikely(!bundleCache)) {
-        bundleCache =
-            NSCreateMapTable(PyObjCUtil_PointerKeyCallBacks, PyObjC_ObjCValueCallBacks,
-                             PYOBJC_EXPECTED_CLASS_COUNT);
-    }
+        if (unlikely(!bundleCache)) {
+            bundleCache =
+                NSCreateMapTable(PyObjCUtil_PointerKeyCallBacks, PyObjC_ObjCValueCallBacks,
+                                 PYOBJC_EXPECTED_CLASS_COUNT);
+        }
 
-    if (!aClass) {
-        return mainBundle;
-    }
+        if (!aClass) {
+            return mainBundle;
+        }
 
-    rval = (id)NSMapGet(bundleCache, (const void*)aClass);
-    if (rval) {
-        return rval;
-    }
+        rval = (id)NSMapGet(bundleCache, (const void*)aClass);
+        if (rval) {
+            return rval;
+        }
 
-    rval = bundleForClassIMP(self, @selector(bundleForClass:), aClass);
-    if (rval == mainBundle) {
-        Class base_isa     = aClass;
-        Class nsobject_isa = object_getClass([NSObject class]);
+        rval = bundleForClassIMP(self, @selector(bundleForClass:), aClass);
+        if (rval == mainBundle) {
+            Class base_isa     = aClass;
+            Class nsobject_isa = object_getClass([NSObject class]);
 
-        while (base_isa != nsobject_isa) {
-            Class next_isa = object_getClass(base_isa);
-            if (!next_isa || next_isa == base_isa) {
-                break;
-            }
+            while (base_isa != nsobject_isa) {
+                Class next_isa = object_getClass(base_isa);
+                if (!next_isa || next_isa == base_isa) {
+                    break;
+                }
 
-            base_isa = next_isa;
-        }
+                base_isa = next_isa;
+            }
 
-        if (base_isa == nsobject_isa) {
-            if ([(id)aClass respondsToSelector:@selector(bundleForClass)]) {
-                rval = [(id)aClass performSelector:@selector(bundleForClass)];
+            if (base_isa == nsobject_isa) {
+                if ([(id)aClass respondsToSelector:@selector(bundleForClass)]) {
+                    rval = [(id)aClass performSelector:@selector(bundleForClass)];
+                }
             }
         }
-    }
 
-    NSMapInsert(bundleCache, (const void*)aClass, (const void*)rval);
-    return rval;
+        NSMapInsert(bundleCache, (const void*)aClass, (const void*)rval);
+        return rval;
+#ifdef Py_GIL_DISABLED
+    }
+#endif
 }
 
 static const char BUNDLE_FOR_CLASS_SIGNATURE[] = {_C_ID, _C_ID, _C_SEL, _C_CLASS, 0};

diff --git a/pyobjc-core/Modules/objc/OC_PythonObject.m b/pyobjc-core/Modules/objc/OC_PythonObject.m
@@ -36,7 +36,17 @@ @implementation OC_PythonObject
 
 - (id _Nullable)initWithPyObject:(PyObject*)obj
 {
-    PyObjC_RegisterObjCProxy(obj, self);
+    // XXX: Fix callers to do the registration for us.
+    id actual = PyObjC_RegisterObjCProxy(obj, self);
+    if (actual != self) {
+        /*
+         * Race between two threads creating a proxy, use the
+         * first one that got registered.
+         */
+        [self release];
+        [actual retain];
+        return actual;
+    }
 
     SET_FIELD_INCREF(pyObject, obj);
 

diff --git a/pyobjc-core/Modules/objc/block_support.m b/pyobjc-core/Modules/objc/block_support.m
@@ -510,7 +510,9 @@ SEL       _sel __attribute__((__unused__)))
     }
 
     if (rval != NULL) {
-        PyObjC_RegisterPythonProxy(self, rval);
+        PyObject* actual = PyObjC_RegisterPythonProxy(self, rval);
+        Py_DECREF(rval);
+        return actual;
     }
 
     return rval;

diff --git a/pyobjc-core/Modules/objc/class-builder.m b/pyobjc-core/Modules/objc/class-builder.m
@@ -103,10 +103,35 @@ static void object_method_copyWithZone_(ffi_cif* cif, void* resp, void** args,
  * methods.
  */
 
+#ifdef Py_GIL_DISABLED
+/*
+ * Class creation is not performance critical, use a mutex to protect
+ * against creating the same intermedia class twice in the free threaded
+ * build.
+ */
+static PyMutex intermediate_mutex = { 0 };
+#endif
+
+
 static Class _Nullable build_intermediate_class(Class base_class, char* name)
 {
     Class intermediate_class = nil;
 
+#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) {
+        PyObjC_Assert(class_getSuperclass(intermediate_class) == base_class, Nil);
+        return intermediate_class;
+    }
+
     intermediate_class = objc_allocateClassPair(base_class, name, 0);
     if (intermediate_class == NULL) { // LCOV_BR_EXCL_LINE
         // LCOV_EXCL_START
@@ -136,6 +161,9 @@ static Class _Nullable build_intermediate_class(Class base_class, char* name)
     }
 
     objc_registerClassPair(intermediate_class);
+#ifdef Py_GIL_DISABLED
+    PyMutex_Unlock(&intermediate_mutex);
+#endif
     return (Class)intermediate_class;
 
 error_cleanup:
@@ -144,6 +172,10 @@ static Class _Nullable build_intermediate_class(Class base_class, char* name)
         objc_disposeClassPair(intermediate_class);
     }
 
+#ifdef Py_GIL_DISABLED
+    PyMutex_Unlock(&intermediate_mutex);
+#endif
+
     return Nil;
     // LCOV_EXCL_STOP
 }
@@ -333,15 +365,12 @@ Class _Nullable PyObjCClass_BuildClass(Class super_class, PyObject* protocols, c
             // LCOV_EXCL_STOP
         }
 
-        intermediate_class = objc_lookUpClass(buf);
-        if (intermediate_class == NULL) {
-            intermediate_class = build_intermediate_class(super_class, buf);
-            if (intermediate_class == Nil) { // LCOV_BR_EXCL_LINE
-                /* build_intermediate_class can only fail when
-                 * running out of resources.
-                 */
-                goto error_cleanup; // LCOV_EXCL_LINE
-            }
+        intermediate_class = build_intermediate_class(super_class, buf);
+        if (intermediate_class == Nil) { // LCOV_BR_EXCL_LINE
+            /* build_intermediate_class can only fail when
+             * running out of resources.
+             */
+            goto error_cleanup; // LCOV_EXCL_LINE
         }
         Py_DECREF(py_superclass);
 

diff --git a/pyobjc-core/Modules/objc/closure_pool.m b/pyobjc-core/Modules/objc/closure_pool.m
@@ -19,7 +19,7 @@
     struct freelist* _Nullable next;
 } freelist;
 
-#if PY_VERSION_HEX >= 0x030d0000
+#ifdef Py_GIL_DISABLED
 /*
  * Mutex protecting *closure_freelist*. There is no need for
  * minimizing the amount of code protected by the lock, allocating
@@ -103,13 +103,13 @@
         return NULL;
     }
     PyObjC_Assert(codeloc, NULL);
-#if PY_VERSION_HEX >= 0x030d0000
+#ifdef Py_GIL_DISABLED
     PyMutex_Lock(&freelist_mutex);
 #endif
     if (closure_freelist == NULL) {
         closure_freelist = allocate_block();
         if (closure_freelist == NULL) {
-#if PY_VERSION_HEX >= 0x030d0000
+#ifdef Py_GIL_DISABLED
             PyMutex_Unlock(&freelist_mutex);
 #endif
             return NULL;
@@ -118,7 +118,7 @@
     ffi_closure* result = (ffi_closure*)closure_freelist;
     closure_freelist    = closure_freelist->next;
     *codeloc            = (void*)result;
-#if PY_VERSION_HEX >= 0x030d0000
+#ifdef Py_GIL_DISABLED
     PyMutex_Unlock(&freelist_mutex);
 #endif
     return result;
@@ -127,12 +127,12 @@
 int
 PyObjC_ffi_closure_free(ffi_closure* cl)
 {
-#if PY_VERSION_HEX >= 0x030d0000
+#ifdef Py_GIL_DISABLED
     PyMutex_Lock(&freelist_mutex);
 #endif
     ((freelist*)cl)->next = closure_freelist;
     closure_freelist      = (freelist*)cl;
-#if PY_VERSION_HEX >= 0x030d0000
+#ifdef Py_GIL_DISABLED
     PyMutex_Unlock(&freelist_mutex);
 #endif
     return 0;

diff --git a/pyobjc-core/Modules/objc/corefoundation.m b/pyobjc-core/Modules/objc/corefoundation.m
@@ -100,7 +100,9 @@ SEL       _sel __attribute__((__unused__)))
         }
 
         if (rval) {
-            PyObjC_RegisterPythonProxy(self, rval);
+            PyObject* actual = PyObjC_RegisterPythonProxy(self, rval);
+            Py_DECREF(rval);
+            return actual;
         }
     }
 

diff --git a/pyobjc-core/Modules/objc/ctests.m b/pyobjc-core/Modules/objc/ctests.m
@@ -990,31 +990,6 @@ - (void)methodWithMyStruct:(struct Struct2)val1 andShort:(short)val2
 
 END_UNITTEST
 
-BEGIN_UNITTEST(BytesInterning)
-PyObject* b1;
-PyObject* b2;
-
-b1 = PyObjCBytes_InternFromString("hello");
-FAIL_IF(b1 == NULL);
-if (!PyBytes_Check(b1)) {
-    Py_DECREF(b1);
-    ASSERT(0);
-}
-ASSERT_EQUALS(strcmp(PyBytes_AsString(b1), "hello"), 0, "%d");
-
-b2 = PyObjCBytes_InternFromStringAndSize("hello world", 5);
-FAIL_IF(b2 == NULL);
-if (!PyBytes_Check(b2)) {
-    Py_DECREF(b2);
-    ASSERT(0);
-}
-
-ASSERT_EQUALS(b1, b2, "%p");
-
-Py_DECREF(b1);
-Py_DECREF(b2);
-
-END_UNITTEST
 
 BEGIN_UNITTEST(MethodSignatureString)
 PyObjCMethodSignature* sig = PyObjCMethodSignature_WithMetaData("@@:d", NULL, NO);
@@ -1096,7 +1071,6 @@ - (void)methodWithMyStruct:(struct Struct2)val1 andShort:(short)val2
                                     TESTDEF(AlwaysFails),
                                     TESTDEF(InvalidObjCPointer),
                                     TESTDEF(InvalidRegistryUsage),
-                                    TESTDEF(BytesInterning),
                                     TESTDEF(MethodSignatureString),
                                     TESTDEF(ValidEncoding),
                                     {0, 0, 0, 0}};

diff --git a/pyobjc-core/Modules/objc/formal-protocol.m b/pyobjc-core/Modules/objc/formal-protocol.m
@@ -215,15 +215,10 @@
     }
 
     result->objc = theProtocol;
-    if (PyObjC_RegisterPythonProxy( // LCOV_BR_EXCL_LINE
-            result->objc, (PyObject*)result)
-        < 0) {
-        // LCOV_EXCL_START
-        Py_DECREF(result);
-        goto error;
-        // LCOV_EXCL_STOP
-    }
-    return (PyObject*)result;
+    PyObject* actual = PyObjC_RegisterPythonProxy( // LCOV_BR_EXCL_LINE
+            result->objc, (PyObject*)result);
+    Py_DECREF(result);
+    return actual;
 
 error:
     // LCOV_EXCL_START
@@ -500,8 +495,9 @@
     }
 
     result->objc = protocol;
-    PyObjC_RegisterPythonProxy(result->objc, (PyObject*)result);
-    return (PyObject*)result;
+    PyObject* actual = PyObjC_RegisterPythonProxy(result->objc, (PyObject*)result);
+    Py_DECREF(result);
+    return actual;
 }
 
 Protocol* _Nullable PyObjCFormalProtocol_GetProtocol(PyObject* object)