Issue #608: Switch proxy-registry to free-threaded support

This changeset does two things: 1. Add locking to the functions for updating and quering the registry (for the free-threaded build only) 2. Switch the mapping from Python to Objective-C proxies to using weak values for the Objective-C proxies. That makes clearing the mapping thread safe for both build variants, and allows removing the implementation of ``-release`` in the ``OC_Python*`` classes that acquired the GIL to remove a race condition between the invocation of ``-dealloc`` in a just released Objective-C value and removing that value from the mapping in a different thread.
ronaldoussoren · Nov 29, 2024 · 7ccb7eb · 7ccb7eb
1 parent ae7cc06
commit 7ccb7eb
Show file tree

Hide file tree

Showing 16 changed files with 210 additions and 287 deletions.
diff --git a/pyobjc-core/Modules/objc/OC_PythonArray.m b/pyobjc-core/Modules/objc/OC_PythonArray.m
@@ -48,29 +48,6 @@ + (BOOL)supportsSecureCoding
     return NO;
 }
 
-- (oneway void)release
-{
-    /* See comment in OC_PythonUnicode */
-    if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE
-        // LCOV_EXCL_START
-        [super release];
-        return;
-        // LCOV_EXCL_STOP
-    }
-    PyObjC_BEGIN_WITH_GIL
-
-        @try {
-            [super release];
-        } @catch (NSException* exc) { // LCOV_EXCL_LINE
-            // LCOV_EXCL_START
-            PyObjC_LEAVE_GIL;
-            @throw;
-            // LCOV_EXCL_STOP
-        }
-
-    PyObjC_END_WITH_GIL
-}
-
 - (void)dealloc
 {
     if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE
@@ -576,9 +553,10 @@ - (id _Nullable)initWithCoder:(NSCoder*)coder
 
                 id actual = PyObjC_RegisterObjCProxy(value, self);
                 if (actual != self) {
-                    [actual retain];
                     [self release];
                     self = actual;
+                } else if (actual != nil) {
+                    [actual release];
                 }
 
             PyObjC_END_WITH_GIL

diff --git a/pyobjc-core/Modules/objc/OC_PythonData.m b/pyobjc-core/Modules/objc/OC_PythonData.m
@@ -41,27 +41,6 @@ + (BOOL)supportsSecureCoding
     return NO;
 }
 
-- (oneway void)release
-{
-    /* See comment in OC_PythonUnicode */
-    if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE
-        // LCOV_EXCL_START
-        [super release];
-        return;
-        // LCOV_EXCL_STOP
-    }
-
-    PyObjC_BEGIN_WITH_GIL
-        @try {
-            [super release];
-        } @catch (NSException* exc) {
-            PyObjC_LEAVE_GIL;
-            [exc raise];
-        }
-
-    PyObjC_END_WITH_GIL
-}
-
 - (void)dealloc
 {
     if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE
@@ -296,9 +275,10 @@ - (id _Nullable)initWithCoder:(NSCoder*)coder
 
                 id actual = PyObjC_RegisterObjCProxy(value, self);
                 if (actual != self) {
-                    [actual retain];
                     [self release];
                     self = actual;
+                } else if (actual != nil) {
+                    [actual release];
                 }
 
 

diff --git a/pyobjc-core/Modules/objc/OC_PythonDate.m b/pyobjc-core/Modules/objc/OC_PythonDate.m
@@ -111,27 +111,6 @@ - (PyObject*)__pyobjc_PythonTransient__:(int*)cookie
     return value;
 }
 
-- (oneway void)release
-{
-    /* See comment in OC_PythonUnicode */
-    if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE
-        // LCOV_EXCL_START
-        [super release];
-        return;
-        // LCOV_EXCL_STOP
-    }
-
-    PyObjC_BEGIN_WITH_GIL
-        @try {
-            [super release];
-        } @catch (NSException* exc) {
-            PyObjC_LEAVE_GIL;
-            [exc raise];
-        }
-
-    PyObjC_END_WITH_GIL
-}
-
 - (void)dealloc
 {
     if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE
@@ -334,9 +313,10 @@ - (id _Nullable)initWithCoder:(NSCoder*)coder
                 SET_FIELD(value, v);
                 id actual = PyObjC_RegisterObjCProxy(value, self);
                 if (actual != self) {
-                    [actual retain];
                     [self release];
                     self = actual;
+                } else  if (actual != nil) {
+                    [actual release];
                 }
 
             PyObjC_END_WITH_GIL

diff --git a/pyobjc-core/Modules/objc/OC_PythonDictionary.m b/pyobjc-core/Modules/objc/OC_PythonDictionary.m
@@ -103,27 +103,6 @@ + (BOOL)supportsSecureCoding
     return NO;
 }
 
-- (oneway void)release
-{
-    /* See comment in OC_PythonUnicode */
-    if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE
-        // LCOV_EXCL_START
-        [super release];
-        return;
-        // LCOV_EXCL_STOP
-    }
-
-    PyObjC_BEGIN_WITH_GIL
-        @try {
-            [super release];
-        } @catch (NSObject* exc) {
-            PyObjC_LEAVE_GIL;
-            @throw;
-        }
-
-    PyObjC_END_WITH_GIL
-}
-
 - (void)dealloc
 {
     if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE
@@ -511,9 +490,10 @@ - (id _Nullable)initWithCoder:(NSCoder*)coder
 
                 id actual = PyObjC_RegisterObjCProxy(value, self);
                 if (actual != self) {
-                    [actual retain];
                     [self release];
                     self = actual;
+                } else if (actual != nil) {
+                    [actual release];
                 }
             PyObjC_END_WITH_GIL
 

diff --git a/pyobjc-core/Modules/objc/OC_PythonEnumerator.m b/pyobjc-core/Modules/objc/OC_PythonEnumerator.m
@@ -20,33 +20,6 @@ - (id)initWithPythonObject:(PyObject*)object
     return self;
 }
 
-- (oneway void)release
-{
-    /* See comment in OC_PythonUnicode */
-    if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE
-        // LCOV_EXCL_START
-        [super release];
-        return;
-        // LCOV_EXCL_STOP
-    }
-
-    PyObjC_BEGIN_WITH_GIL
-        @try {
-            [super release];
-            // LCOV_EXCL_START
-        } @catch (NSObject* exc) {
-            /* I'm 99% sure this path cannot be hit,
-             * this class cannot be subclassesed and
-             * -dealloc cannot raise.
-             */
-            PyObjC_LEAVE_GIL;
-            @throw;
-        }
-        // LCOV_EXCL_STOP
-
-    PyObjC_END_WITH_GIL
-}
-
 - (void)dealloc
 {
     if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE

diff --git a/pyobjc-core/Modules/objc/OC_PythonNumber.m b/pyobjc-core/Modules/objc/OC_PythonNumber.m
@@ -43,28 +43,6 @@ + (BOOL)supportsSecureCoding
     return NO;
 }
 
-- (oneway void)release
-{
-    /* See comment in OC_PythonUnicode */
-    if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE
-        // LCOV_EXCL_START
-        [super release];
-        return;
-        // LCOV_EXCL_STOP
-    }
-
-    PyObjC_BEGIN_WITH_GIL
-
-        @try {
-            [super release];
-        } @catch (NSObject* exc) {
-            PyObjC_LEAVE_GIL;
-            @throw;
-        }
-
-    PyObjC_END_WITH_GIL
-}
-
 - (void)dealloc
 {
     if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE
@@ -399,9 +377,10 @@ - (id _Nullable)initWithCoder:(NSCoder*)coder
 
             id actual = PyObjC_RegisterObjCProxy(value, self);
             if (actual != self) {
-                [actual retain];
                 [self release];
                 self = actual;
+            } else if (actual != nil) {
+                [actual release];
             }
 
         PyObjC_END_WITH_GIL

diff --git a/pyobjc-core/Modules/objc/OC_PythonObject.m b/pyobjc-core/Modules/objc/OC_PythonObject.m
@@ -44,42 +44,16 @@ - (id _Nullable)initWithPyObject:(PyObject*)obj
          * first one that got registered.
          */
         [self release];
-        [actual retain];
         return actual;
+    } else if (actual != nil) {
+        [actual release];
     }
 
     SET_FIELD_INCREF(pyObject, obj);
 
     return self;
 }
 
-- (oneway void)release
-{
-    /* See comment in OC_PythonUnicode */
-    if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE
-        // LCOV_EXCL_START
-        [super release];
-        return;
-        // LCOV_EXCL_STOP
-    }
-
-    PyObjC_BEGIN_WITH_GIL
-        @try {
-            [super release];
-
-        } @catch (NSObject* exc) { // LCOV_EXCL_LINE
-            /* This catch statement is here mostly
-             * for code consistency, [NSProxy release]
-             * should never raise.
-             */
-            // LCOV_EXCL_START
-            PyObjC_LEAVE_GIL;
-            @throw;
-            // LCOV_EXCL_STOP
-        }
-    PyObjC_END_WITH_GIL
-}
-
 - (void)dealloc
 {
     if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE

diff --git a/pyobjc-core/Modules/objc/OC_PythonSet.m b/pyobjc-core/Modules/objc/OC_PythonSet.m
@@ -42,27 +42,6 @@ + (BOOL)supportsSecureCoding
     return NO;
 }
 
-- (oneway void)release
-{
-    /* See comment in OC_PythonUnicode */
-    if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE
-        // LCOV_EXCL_START
-        [super release];
-        return;
-        // LCOV_EXCL_STOP
-    }
-
-    PyObjC_BEGIN_WITH_GIL
-        @try {
-            [super release];
-        } @catch (NSObject* exc) {
-            PyObjC_LEAVE_GIL;
-            @throw;
-        }
-
-    PyObjC_END_WITH_GIL
-}
-
 - (void)dealloc
 {
     if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE
@@ -249,9 +228,10 @@ - (id _Nullable)initWithCoder:(NSCoder*)coder
 
             id actual = PyObjC_RegisterObjCProxy(value, self);
             if (actual != self) {
-                [actual retain];
                 [self release];
                 self = actual;
+            } else if (actual != nil) {
+                [actual release];
             }
         PyObjC_END_WITH_GIL
 

diff --git a/pyobjc-core/Modules/objc/OC_PythonURL.m b/pyobjc-core/Modules/objc/OC_PythonURL.m
@@ -55,33 +55,6 @@ - (id _Nullable)initWithPythonObject:(PyObject*)object
     return self;
 }
 
-- (oneway void)release
-{
-    /* See comment in OC_PythonUnicode */
-    if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE
-        // LCOV_EXCL_START
-        [super release];
-        return;
-        // LCOV_EXCL_STOP
-    }
-
-    PyObjC_BEGIN_WITH_GIL
-        @try {
-            [super release];
-            // LCOV_EXCL_START
-        } @catch (NSObject* exc) {
-            /* I'm 99% sure this path cannot be hit,
-             * this class cannot be subclassesed and
-             * -dealloc cannot raise.
-             */
-            PyObjC_LEAVE_GIL;
-            @throw;
-        }
-        // LCOV_EXCL_STOP
-
-    PyObjC_END_WITH_GIL
-}
-
 - (void)dealloc
 {
     if (unlikely(!Py_IsInitialized())) { // LCOV_BR_EXCL_LINE