More thread-safe GC (another version)

src/Core/Py.jl

@@ -43,7 +43,21 @@ mutable struct Py
 end
 export Py
 
-py_finalizer(x::Py) = GC.enqueue(getptr(x))
+const NUM_DECREFS = Ref(0)
+
+function py_finalizer(x::Py)
+    if C.CTX.is_initialized
+        if C.PyGILState_Check() == 1
+            # if this thread holds the GIL then decref now
+            C.Py_DecRef(getptr(x))
+            NUM_DECREFS[] += 1
+        else
+            # otherwise re-attach the finalizer and try again next GC
+            finalizer(py_finalizer, x)
+        end
+    end
+    nothing
+end
 
 ispy(::Py) = true
 getptr(x::Py) = getfield(x, :ptr)

src/GC/GC.jl

@@ -7,73 +7,30 @@ See `disable` and `enable`.
 """
 module GC
 
-using ..C: C
-
-const ENABLED = Ref(true)
-const QUEUE = C.PyPtr[]
-
 """
     PythonCall.GC.disable()
 
-Disable the PythonCall garbage collector.
+Do nothing.
 
-This means that whenever a Python object owned by Julia is finalized, it is not immediately
-freed but is instead added to a queue of objects to free later when `enable()` is called.
+!!! note
 
-Like most PythonCall functions, you must only call this from the main thread.
+    In earlier versions of PythonCall, this function disabled the PythonCall garbage
+    collector. This is no longer required because Python objects now have thread-safe
+    finalizers. This function will be removed in PythonCall v1.
 """
-function disable()
-    ENABLED[] = false
-    return
-end
+disable() = nothing
 
 """
     PythonCall.GC.enable()
 
-Re-enable the PythonCall garbage collector.
+Do nothing.
 
-This frees any Python objects which were finalized while the GC was disabled, and allows
-objects finalized in the future to be freed immediately.
+!!! note
 
-Like most PythonCall functions, you must only call this from the main thread.
+    In earlier versions of PythonCall, this function re-enabled the PythonCall garbage
+    collector. This is no longer required because Python objects now have thread-safe
+    finalizers. This function will be removed in PythonCall v1.
 """
-function enable()
-    ENABLED[] = true
-    if !isempty(QUEUE)
-        for ptr in QUEUE
-            if ptr != C.PyNULL
-                C.Py_DecRef(ptr)
-            end
-        end
-    end
-    empty!(QUEUE)
-    return
-end
-
-function enqueue(ptr::C.PyPtr)
-    if ptr != C.PyNULL && C.CTX.is_initialized
-        if ENABLED[]
-            C.Py_DecRef(ptr)
-        else
-            push!(QUEUE, ptr)
-        end
-    end
-    return
-end
-
-function enqueue_all(ptrs)
-    if C.CTX.is_initialized
-        if ENABLED[]
-            for ptr in ptrs
-                if ptr != C.PyNULL
-                    C.Py_DecRef(ptr)
-                end
-            end
-        else
-            append!(QUEUE, ptrs)
-        end
-    end
-    return
-end
+enable() = nothing
 
 end # module GC

src/JlWrap/objectarray.jl

@@ -27,7 +27,21 @@ PyObjectArray{N}(x::AbstractArray{T,N}) where {T,N} =
     copyto!(PyObjectArray{N}(undef, size(x)), x)
 PyObjectArray(x::AbstractArray{T,N}) where {T,N} = PyObjectArray{N}(x)
 
-pyobjectarray_finalizer(x::PyObjectArray) = GC.enqueue_all(x.ptrs)
+function pyobjectarray_finalizer(x::PyObjectArray)
+    if C.CTX.is_initialized
+        if C.PyGILState_Check() == 1
+            # if this thread holds the GIL then decref now
+            for ptr in x.ptrs
+                C.Py_DecRef(ptr)
+            end
+        else
+            # otherwise re-attach the finalizer and try again next GC
+            finalizer(pyobjectarray_finalizer, x)
+        end
+    end
+    nothing
+end
+
 
 Base.IndexStyle(x::PyObjectArray) = Base.IndexStyle(x.ptrs)
 

test/finalize_test_script.jl

@@ -0,0 +1,48 @@
+using PythonCall
+
+# This would consistently segfault pre-GC-thread-safety
+function test()
+    pyobjs = map(pyint, 1:10)
+    Threads.@threads for i = 1:10
+        finalize(pyobjs[i])
+    end
+    # The following loop is a workaround and can be removed if the issue is fixed:
+    # https://github.com/JuliaLang/julia/issues/40626#issuecomment-1054890774
+    Threads.@threads :static for _ = 1:Threads.nthreads()
+        Timer(Returns(nothing), 0; interval = 1)
+    end
+    nothing
+end
+
+function decrefs()
+    n = PythonCall.Core.NUM_DECREFS[]
+    PythonCall.Core.NUM_DECREFS[] = 0
+    n
+end
+
+GC.gc()
+decrefs()
+println("test()")
+test()
+println("  decrefs: ", decrefs())
+println("gc(false)")
+GC.gc(false)
+println("  decrefs: ", decrefs())
+println("gc(false)")
+GC.gc(false)
+println("  decrefs: ", decrefs())
+println("gc()")
+GC.gc()
+println("  decrefs: ", decrefs())
+println("gc()")
+GC.gc()
+println("  decrefs: ", decrefs())
+println("gc()")
+GC.gc()
+println("  decrefs: ", decrefs())
+println("gc()")
+GC.gc()
+println("  decrefs: ", decrefs())
+println("gc()")
+GC.gc()
+println("  decrefs: ", decrefs())