diff --git a/src/Core/Py.jl b/src/Core/Py.jl index e56d74fa..82d2d85f 100644 --- a/src/Core/Py.jl +++ b/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) diff --git a/src/GC/GC.jl b/src/GC/GC.jl index 7bccfadc..058d024e 100644 --- a/src/GC/GC.jl +++ b/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 diff --git a/src/JlWrap/objectarray.jl b/src/JlWrap/objectarray.jl index a98189e8..ffd0fa80 100644 --- a/src/JlWrap/objectarray.jl +++ b/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) diff --git a/test/finalize_test_script.jl b/test/finalize_test_script.jl new file mode 100644 index 00000000..a5a86941 --- /dev/null +++ b/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())