slightly more thread safe gc

Author: Christopher Doris

src/C/pointers.jl

@@ -22,6 +22,8 @@ const CAPI_FUNC_SIGS = Dict{Symbol,Pair{Tuple,Type}}(
     :PyEval_RestoreThread => (Ptr{Cvoid},) => Cvoid,
     :PyGILState_Ensure => () => PyGILState_STATE,
     :PyGILState_Release => (PyGILState_STATE,) => Cvoid,
+    :PyGILState_GetThisThreadState => () => Ptr{Cvoid},
+    :PyGILState_Check => () => Cint,
     # IMPORT
     :PyImport_ImportModule => (Ptr{Cchar},) => PyPtr,
     :PyImport_Import => (PyPtr,) => PyPtr,

src/GC/GC.jl

@@ -39,25 +39,36 @@ Like most PythonCall functions, you must only call this from the main thread.
 """
 function enable()
     ENABLED[] = true
-    if !isempty(QUEUE)
-        C.with_gil(false) do
-            for ptr in QUEUE
-                if ptr != C.PyNULL
-                    C.Py_DecRef(ptr)
-                end
-            end
+    if !isempty(QUEUE) && C.PyGILState_Check() == 1
+        free_queue()
+    end
+    return
+end
+
+function free_queue()
+    for ptr in QUEUE
+        if ptr != C.PyNULL
+            C.Py_DecRef(ptr)
         end
     end
     empty!(QUEUE)
-    return
+    nothing
+end
+
+function gc()
+    if ENABLED[] && C.PyGILState_Check() == 1
+        free_queue()
+        true
+    else
+        false
+    end
 end
 
 function enqueue(ptr::C.PyPtr)
     if ptr != C.PyNULL && C.CTX.is_initialized
-        if ENABLED[]
-            C.with_gil(false) do
-                C.Py_DecRef(ptr)
-            end
+        if ENABLED[] && C.PyGILState_Check() == 1
+            C.Py_DecRef(ptr)
+            isempty(QUEUE) || free_queue()
         else
             push!(QUEUE, ptr)
         end
@@ -67,19 +78,35 @@ end
 
 function enqueue_all(ptrs)
     if C.CTX.is_initialized
-        if ENABLED[]
-            C.with_gil(false) do
-                for ptr in ptrs
-                    if ptr != C.PyNULL
-                        C.Py_DecRef(ptr)
-                    end
+        if ENABLED[] && C.PyGILState_Check() == 1
+            for ptr in ptrs
+                if ptr != C.PyNULL
+                    C.Py_DecRef(ptr)
                 end
             end
+            isempty(QUEUE) || free_queue()
         else
             append!(QUEUE, ptrs)
         end
     end
     return
 end
 
+mutable struct GCHook
+    function GCHook()
+        finalizer(_gchook_finalizer, new())
+    end
+end
+
+function _gchook_finalizer(x)
+    gc()
+    finalizer(_gchook_finalizer, x)
+    nothing
+end
+
+function __init__()
+    GCHook()
+    nothing
+end
+
 end # module GC

test/finalize_test_script.jl

@@ -0,0 +1,24 @@
+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
+
+@show PythonCall.GC.ENABLED[]
+@show length(PythonCall.GC.QUEUE)
+GC.gc(false)
+# with GCHook, the queue should be empty now (the above gc() triggered GCHook to clear the PythonCall QUEUE)
+# without GCHook, gc() has no effect on the QUEUE
+@show length(PythonCall.GC.QUEUE)
+GC.gc(false)
+@show length(PythonCall.GC.QUEUE)
+GC.gc(false)
+@show length(PythonCall.GC.QUEUE)
+# with GCHook this is not necessary, GC.gc() is enough
+# without GCHook, this is required to free any objects in the PythonCall QUEUE
+PythonCall.GC.gc()
+@show length(PythonCall.GC.QUEUE)