Skip to content

Commit 7d79ab5

Browse files
author
Dani Pinyol
committed
fix: no GC disable. avoid resize PYJLFREEVALUES in dealloc
1 parent b25076b commit 7d79ab5

1 file changed

Lines changed: 17 additions & 15 deletions

File tree

src/JlWrap/C.jl

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,13 @@ const PyJuliaBase_Type = Ref(C.PyNULL)
2323
# we store the actual julia values here
2424
# the `value` field of `PyJuliaValueObject` indexes into here
2525
const PYJLVALUES = []
26-
# unused indices in PYJLVALUES
26+
# unused indices in PYJLVALUES; kept the same length as PYJLVALUES (extra slots
27+
# are pushed alongside PYJLVALUES so the dealloc/finalizer path never has to
28+
# resize this vector). Only the first PYJLFREEVALUECOUNT[] entries are valid
29+
# free indices; the rest are placeholders.
2730
const PYJLFREEVALUES = Int[]
28-
# lock protecting PYJLVALUES and PYJLFREEVALUES from concurrent modification
31+
const PYJLFREEVALUECOUNT = Ref(0)
32+
# lock protecting PYJLVALUES, PYJLFREEVALUES and PYJLFREEVALUECOUNT
2933
const PYJL_LOCK = Threads.SpinLock()
3034

3135
function _pyjl_new(t::C.PyPtr, ::C.PyPtr, ::C.PyPtr)
@@ -41,16 +45,14 @@ end
4145
function _pyjl_dealloc(o::C.PyPtr)
4246
idx = C.@ft UnsafePtr{PyJuliaValueObject}(o).value[]
4347
if idx != 0
44-
# Disable GC to prevent push! from triggering a GC that runs finalizers
45-
# re-entrantly (the finalizer chain: push! → alloc → GC → py_finalizer →
46-
# enqueue → Py_DecRef → _pyjl_dealloc → push! on same vector →
47-
# ConcurrencyViolationError). The lock guards against true multi-thread races.
48-
prev = Base.GC.enable(false)
48+
# No allocation in this critical section: PYJLFREEVALUES has been pre-sized
49+
# to match PYJLVALUES by PyJuliaValue_SetValue, so recording a free index is
50+
# just a write to an existing slot plus a counter bump. This avoids the GC-triggered
4951
lock(PYJL_LOCK)
5052
PYJLVALUES[idx] = nothing
51-
push!(PYJLFREEVALUES, idx)
53+
PYJLFREEVALUECOUNT[] += 1
54+
PYJLFREEVALUES[PYJLFREEVALUECOUNT[]] = idx
5255
unlock(PYJL_LOCK)
53-
Base.GC.enable(prev)
5456
end
5557
(C.@ft UnsafePtr{PyJuliaValueObject}(o).weaklist[!]) == C.PyNULL || C.PyObject_ClearWeakRefs(o)
5658
freeptr = C.PyType_GetSlot(C.Py_Type(o), C.Py_tp_free)
@@ -385,19 +387,19 @@ PyJuliaValue_SetValue(_o, @nospecialize(v)) = Base.GC.@preserve _o begin
385387
o = C.asptr(_o)
386388
idx = C.@ft UnsafePtr{PyJuliaValueObject}(o).value[]
387389
if idx == 0
388-
# Disable GC to prevent push!/pop! from triggering a GC whose finalizers
389-
# re-entrantly resize the same vectors (see _pyjl_dealloc comment).
390-
prev = Base.GC.enable(false)
391390
lock(PYJL_LOCK)
392-
if isempty(PYJLFREEVALUES)
391+
if PYJLFREEVALUECOUNT[] == 0
392+
# Grow both vectors together so length(PYJLFREEVALUES) == length(PYJLVALUES)
393+
# is preserved. The 0 in PYJLFREEVALUES is just a placeholder
393394
push!(PYJLVALUES, v)
395+
push!(PYJLFREEVALUES, 0)
394396
idx = length(PYJLVALUES)
395397
else
396-
idx = pop!(PYJLFREEVALUES)
398+
idx = PYJLFREEVALUES[PYJLFREEVALUECOUNT[]]
399+
PYJLFREEVALUECOUNT[] -= 1
397400
PYJLVALUES[idx] = v
398401
end
399402
unlock(PYJL_LOCK)
400-
Base.GC.enable(prev)
401403
C.@ft UnsafePtr{PyJuliaValueObject}(o).value[] = idx
402404
else
403405
PYJLVALUES[idx] = v

0 commit comments

Comments
 (0)