Surround the GIL with a ReentrantLock on the Julia side.

The Python GIL should only be able obtained by a single Julia thread / task.
To enforce this, we use a ReentrantLock on the Julia side that must be obtained
before trying to acquire the Python GIL.

Author: mkitti

src/GIL/GIL.jl

@@ -9,6 +9,18 @@ module GIL
 
 using ..C: C
 
+# Ensure that only one Julia thread tries to acquire the Python GIL
+# PyGILState_Ensure and PyGILState_Release may not be thread safe.
+# https://github.com/JuliaPy/PythonCall.jl/issues/627
+const _jl_gil_lock = ReentrantLock()
+
+"""
+    hasgil()
+
+Returns `true` if the current thread has the GIL or `false` otherwise.
+"""
+hasgil() = C.PyGILState_Check() == Cint(1)
+
 """
     lock(f)
 
@@ -21,11 +33,16 @@ threads. Since the main Julia thread holds the GIL by default, you will need to
 See [`@lock`](@ref) for the macro form.
 """
 function lock(f)
-    state = C.PyGILState_Ensure()
+    Base.lock(_jl_gil_lock)
     try
-        f()
+        state = C.PyGILState_Ensure()
+        try
+            f()
+        finally
+            C.PyGILState_Release(state)
+        end
     finally
-        C.PyGILState_Release(state)
+        Base.unlock(_jl_gil_lock)
     end
 end
 
@@ -42,11 +59,16 @@ The macro equivalent of [`lock`](@ref).
 """
 macro lock(expr)
     quote
-        state = C.PyGILState_Ensure()
+        Base.lock(_jl_gil_lock)
         try
-            $(esc(expr))
+            state = C.PyGILState_Ensure()
+            try
+                $(esc(expr))
+            finally
+                C.PyGILState_Release(state)
+            end
         finally
-            C.PyGILState_Release(state)
+            Base.unlock(_jl_gil_lock)
         end
     end
 end
@@ -63,11 +85,17 @@ Python code. That other thread can be a Julia thread, which must lock the GIL us
 See [`@unlock`](@ref) for the macro form.
 """
 function unlock(f)
-    state = C.PyEval_SaveThread()
+    _locked = Base.islocked(_jl_gil_lock)
+    _locked && Base.unlock(_jl_gil_lock)
     try
-        f()
+        state = C.PyEval_SaveThread()
+        try
+            f()
+        finally
+            C.PyEval_RestoreThread(state)
+        end
     finally
-        C.PyEval_RestoreThread(state)
+        _locked && Base.lock(_jl_gil_lock)
     end
 end
 
@@ -84,13 +112,26 @@ The macro equivalent of [`unlock`](@ref).
 """
 macro unlock(expr)
     quote
-        state = C.PyEval_SaveThread()
+        _locked = Base.islocked(_jl_gil_lock)
+        _locked && Base.unlock(_jl_gil_lock)
         try
-            $(esc(expr))
+            state = C.PyEval_SaveThread()
+            try
+                $(esc(expr))
+            finally
+                C.PyEval_RestoreThread(state)
+            end
         finally
-            C.PyEval_RestoreThread(state)
+            _locked && Base.lock(_jl_gil_lock)
         end
     end
 end
 
+# If the main thread already has the GIL, we should lock _jl_gil_lock.
+function __init__()
+    if hasgil()
+        Base.lock(_jl_gil_lock)
+    end
+end
+
 end

test/GC.jl

@@ -1,4 +1,7 @@
+using TestItemRunner
+
 @testitem "GC.gc()" begin
+    using PythonCall
     let
         pyobjs = map(pylist, 1:100)
         PythonCall.GIL.@unlock Threads.@threads for obj in pyobjs
@@ -13,6 +16,7 @@
 end
 
 @testitem "GC.GCHook" begin
+    using PythonCall
     let
         pyobjs = map(pylist, 1:100)
         PythonCall.GIL.@unlock Threads.@threads for obj in pyobjs
@@ -23,5 +27,10 @@ end
         VERSION >= v"1.10.0-" &&
         @test !isempty(PythonCall.GC.QUEUE.items)
     GC.gc()
+
+    # Unlock and relocking the ReentrantLock allows this test to pass
+    Base.unlock(PythonCall.GIL._jl_gil_lock)
+    Base.lock(PythonCall.GIL._jl_gil_lock)
+
     @test isempty(PythonCall.GC.QUEUE.items)
 end

test/GIL.jl

@@ -18,6 +18,14 @@
     if Threads.nthreads() ≥ 2
         @test 0.9 < t.time < 1.2
     end
+
+    @test PythonCall.GIL.hasgil()
+    PythonCall.GIL.unlock() do
+        @test !Base.islocked(PythonCall.GIL._jl_gil_lock)
+        PythonCall.GIL.lock() do
+            @test Base.islocked(PythonCall.GIL._jl_gil_lock)
+        end
+    end
 end
 
 @testitem "@unlock and @lock" begin
@@ -36,4 +44,13 @@ end
     if Threads.nthreads() ≥ 2
         @test 0.9 < t.time < 1.2
     end
+
+    @test PythonCall.GIL.hasgil()
+    PythonCall.GIL.@unlock begin
+        @test !Base.islocked(PythonCall.GIL._jl_gil_lock)
+        PythonCall.GIL.@lock begin
+            @test Base.islocked(PythonCall.GIL._jl_gil_lock)
+        end
+    end
+
 end