More thread-safe GC

src/GC/GC.jl

@@ -3,77 +3,135 @@
 
 Garbage collection of Python objects.
 
-See `disable` and `enable`.
+See [`enable`](@ref), [`disable`](@ref) and [`gc`](@ref).
 """
 module GC
 
 using ..C: C
 
-const ENABLED = Ref(true)
 const QUEUE = C.PyPtr[]
+const QUEUE_LOCK = Threads.SpinLock()
+const HOOK = WeakRef()
 
 """
     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.
+    Historically this would disable the PythonCall garbage collector. This was required
+    for safety in multi-threaded code but is no longer needed, so this is now a no-op.
 """
-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.
+    Historically this would enable the PythonCall garbage collector. This was required
+    for safety in multi-threaded code but is no longer needed, so this is now a no-op.
 """
-function enable()
-    ENABLED[] = true
-    if !isempty(QUEUE)
-        for ptr in QUEUE
-            if ptr != C.PyNULL
-                C.Py_DecRef(ptr)
-            end
+enable() = nothing
+
+"""
+    PythonCall.GC.gc()
+
+Free any Python objects waiting to be freed.
+
+These are objects that were finalized from a thread that was not holding the Python
+GIL at the time.
+
+Like most PythonCall functions, this must only be called from the main thread (i.e. the
+thread currently holding the Python GIL.)
+"""
+function gc()
+    if C.CTX.is_initialized
+        unsafe_free_queue()
+    end
+    nothing
+end
+
+function unsafe_free_queue()
+    lock(QUEUE_LOCK)
+    for ptr in QUEUE
+        if ptr != C.PyNULL
+            C.Py_DecRef(ptr)
         end
     end
     empty!(QUEUE)
-    return
+    unlock(QUEUE_LOCK)
+    nothing
 end
 
 function enqueue(ptr::C.PyPtr)
     if ptr != C.PyNULL && C.CTX.is_initialized
-        if ENABLED[]
+        if C.PyGILState_Check() == 1
             C.Py_DecRef(ptr)
+            if !isempty(QUEUE)
+                unsafe_free_queue()
+            end
         else
+            lock(QUEUE_LOCK)
             push!(QUEUE, ptr)
+            unlock(QUEUE_LOCK)
         end
     end
-    return
+    nothing
 end
 
 function enqueue_all(ptrs)
-    if C.CTX.is_initialized
-        if ENABLED[]
+    if any(ptr -> ptr != C.PYNULL, ptrs) && C.CTX.is_initialized
+        if C.PyGILState_Check() == 1
             for ptr in ptrs
                 if ptr != C.PyNULL
                     C.Py_DecRef(ptr)
                 end
             end
+            if !isempty(QUEUE)
+                unsafe_free_queue()
+            end
         else
+            lock(QUEUE_LOCK)
             append!(QUEUE, ptrs)
+            unlock(QUEUE_LOCK)
         end
     end
-    return
+    nothing
+end
+
+"""
+    GCHook()
+
+An immortal object which frees any pending Python objects when Julia's GC runs.
+
+This works by creating it but not holding any strong reference to it, so it is eligible
+to be finalized by Julia's GC. The finalizer empties the PythonCall GC queue if
+possible. The finalizer also re-attaches itself, so the object does not actually get
+collected and so the finalizer will run again at next GC.
+"""
+mutable struct GCHook
+    function GCHook()
+        finalizer(_gchook_finalizer, new())
+    end
+end
+
+function _gchook_finalizer(x)
+    if C.CTX.is_initialized
+        finalizer(_gchook_finalizer, x)
+        if !isempty(QUEUE) && C.PyGILState_Check() == 1
+            unsafe_free_queue()
+        end
+    end
+    nothing
+end
+
+function __init__()
+    HOOK.value = GCHook()
+    nothing
 end
 
 end # module GC

test/finalize_test_script.jl

@@ -0,0 +1,9 @@
+using PythonCall
+
+# This would consistently segfault pre-GC-thread-safety
+let
+    pyobjs = map(pylist, 1:100)
+    Threads.@threads for obj in pyobjs
+        finalize(obj)
+    end
+end