From 50f561c2fc806c3ef5f6a22caf9403e8c2c4a108 Mon Sep 17 00:00:00 2001 From: Jack Champagne Date: Wed, 1 Jul 2026 03:10:19 -0400 Subject: [PATCH] Root freshly-boxed call args immediately, not after the whole list box_python_value's box_* calls return a jl_value_t* with nothing on the Julia side keeping it alive. JuliaVal.__call__ (positional args and kwargs), JuliaArray.__setitem__/jl_getindex, and JuliaDict.__setitem__ all boxed multiple values via separate ctypes calls and only rooted them once the full list was handed to the consuming Julia call -- leaving each already-boxed element GC-invisible while later elements were still being boxed. auto_convert_and_root roots each value via jl2py_ref_create the instant it's boxed, released after the call. Reproduced the corruption directly (forced GC between box calls reused freed memory for the next box, corrupting the previous value) and confirmed the fix closes it; added tests/test_arg_rooting.py covering the positional, kwargs, and dict-setitem call paths. Note: small ints are unaffected -- Julia's boxed-int cache means jl_box_int64 returns an already-rooted, permanent pointer for small values, so this only bites floats/strings/complex/large ints. --- out/jl2py/src/jl2py/array.py | 27 +++++++--- out/jl2py/src/jl2py/containers.py | 15 +++--- out/jl2py/src/jl2py/convert.py | 36 +++++++++++++ out/jl2py/src/jl2py/val.py | 83 ++++++++++++++++------------- out/jl2py/tests/test_arg_rooting.py | 73 +++++++++++++++++++++++++ 5 files changed, 184 insertions(+), 50 deletions(-) create mode 100644 out/jl2py/tests/test_arg_rooting.py diff --git a/out/jl2py/src/jl2py/array.py b/out/jl2py/src/jl2py/array.py index d8dc74a..8996330 100644 --- a/out/jl2py/src/jl2py/array.py +++ b/out/jl2py/src/jl2py/array.py @@ -248,19 +248,24 @@ def __getitem__(self, idx): def __setitem__(self, idx, value): lib = get_lib() + from jl2py.convert import auto_convert_and_root, release_rooted if isinstance(idx, int): n = self.__len__() if idx < 0: idx += n jl_idx = lib.jl2py_box_int64(idx + 1) + idx_ref = lib.jl2py_ref_create(jl_idx) else: - jl_idx = auto_convert_arg(idx) + jl_idx, idx_ref = auto_convert_and_root(idx) - jl_val = auto_convert_arg(value) - setindex_fn = lib.jl2py_get_function(lib.jl2py_base_module(), b"setindex!") - result = lib.jl2py_call3(setindex_fn, self.ptr, jl_val, jl_idx) - _check_result(result) + jl_val, val_ref = auto_convert_and_root(value) + try: + setindex_fn = lib.jl2py_get_function(lib.jl2py_base_module(), b"setindex!") + result = lib.jl2py_call3(setindex_fn, self.ptr, jl_val, jl_idx) + _check_result(result) + finally: + release_rooted([idx_ref, val_ref]) def __iter__(self): n = self.__len__() @@ -278,12 +283,18 @@ def __contains__(self, item): def jl_getindex(self, *indices): """Raw 1-based Julia getindex (no offset conversion).""" lib = get_lib() + from jl2py.convert import auto_convert_and_root, release_rooted getindex_fn = lib.jl2py_get_function(lib.jl2py_base_module(), b"getindex") - jl_args = [auto_convert_arg(i) for i in indices] + boxed = [auto_convert_and_root(i) for i in indices] + jl_args = [ptr for ptr, _ in boxed] + refs = [ref for _, ref in boxed] all_args = [self.ptr] + jl_args arr = (ctypes.c_void_p * len(all_args))(*all_args) - result = lib.jl2py_call(getindex_fn, arr, len(all_args)) - val = _check_result(result) + try: + result = lib.jl2py_call(getindex_fn, arr, len(all_args)) + val = _check_result(result) + finally: + release_rooted(refs) from jl2py.types import wrap_julia_value return wrap_julia_value(val) diff --git a/out/jl2py/src/jl2py/containers.py b/out/jl2py/src/jl2py/containers.py index aeaa465..f2d6e17 100644 --- a/out/jl2py/src/jl2py/containers.py +++ b/out/jl2py/src/jl2py/containers.py @@ -9,7 +9,7 @@ from jl2py._lib import get_lib from jl2py.convert import ( TAG_TUPLE, TAG_NAMEDTUPLE, TAG_DICT, TAG_SET, TAG_PAIR, TAG_ARRAY, - auto_convert_arg, + auto_convert_arg, auto_convert_and_root, release_rooted, ) @@ -219,11 +219,14 @@ def __getitem__(self, key): def __setitem__(self, key, value): lib = get_lib() - jl_key = auto_convert_arg(key) - jl_val = auto_convert_arg(value) - setindex_fn = _jl_fn("setindex!") - result = lib.jl2py_call3(setindex_fn, self.ptr, jl_val, jl_key) - _check_result(result) + jl_key, key_ref = auto_convert_and_root(key) + jl_val, val_ref = auto_convert_and_root(value) + try: + setindex_fn = _jl_fn("setindex!") + result = lib.jl2py_call3(setindex_fn, self.ptr, jl_val, jl_key) + _check_result(result) + finally: + release_rooted([key_ref, val_ref]) def __contains__(self, key): lib = get_lib() diff --git a/out/jl2py/src/jl2py/convert.py b/out/jl2py/src/jl2py/convert.py index c791ed1..9e0ebaf 100644 --- a/out/jl2py/src/jl2py/convert.py +++ b/out/jl2py/src/jl2py/convert.py @@ -93,3 +93,39 @@ def auto_convert_arg(val): f"Cannot auto-convert {type(val).__name__} to Julia value. " f"Use an explicit wrapper (JuliaArray, JuliaDict, etc.)." ) + + +def auto_convert_and_root(val): + """ + Like auto_convert_arg, but roots freshly-boxed values immediately via + jl2py_ref_create. + + box_python_value's box_* calls return a jl_value_t* with nothing on the + Julia side keeping it alive; if several of these are collected into a + Python list (positional args, kwargs, multi-index getindex, ...) before + a single call consumes them, each one sits GC-invisible for the rest of + that loop. Rooting right after boxing closes that window. Existing + JuliaVal arguments are left unrooted here — they're already kept alive + by the caller's own Python reference for at least as long as this call. + + Returns (ptr, ref). Pass every non-None ref returned this way to + release_rooted() once the boxed values have been consumed by the call. + """ + from jl2py.val import JuliaVal + + if isinstance(val, JuliaVal): + return val.ptr, None + + ptr = auto_convert_arg(val) + ref = get_lib().jl2py_ref_create(ptr) + if not ref: + raise MemoryError("Failed to root freshly-boxed Julia value") + return ptr, ref + + +def release_rooted(refs): + """Release refs created by auto_convert_and_root, once consumed.""" + lib = get_lib() + for ref in refs: + if ref: + lib.jl2py_ref_release(ref) diff --git a/out/jl2py/src/jl2py/val.py b/out/jl2py/src/jl2py/val.py index 4f42e67..2ad25f7 100644 --- a/out/jl2py/src/jl2py/val.py +++ b/out/jl2py/src/jl2py/val.py @@ -60,49 +60,60 @@ def __init__(self, ptr): def __call__(self, *args, **kwargs): """Forward the call to Julia's dispatch mechanism.""" lib = get_lib() - - # Convert all positional args - jl_args = [auto_convert_arg(a) for a in args] + from jl2py.convert import auto_convert_and_root, release_rooted + + # Convert all positional args, rooting each freshly-boxed value + # immediately (see auto_convert_and_root) so it survives assembly + # of the rest of the arg/kwarg list before the call below consumes + # it. + boxed_args = [auto_convert_and_root(a) for a in args] + jl_args = [ptr for ptr, _ in boxed_args] + refs = [ref for _, ref in boxed_args] nargs = len(jl_args) - if kwargs: - # Dispatch via Core.kwcall(NamedTuple, f, args...) - kwnames_list = list(kwargs.keys()) - kwvals_list = [auto_convert_arg(v) for v in kwargs.values()] - nkw = len(kwnames_list) - - kwnames_arr = (ctypes.c_char_p * nkw)(*[k.encode("utf-8") for k in kwnames_list]) - kwvals_arr = (ctypes.c_void_p * nkw)(*kwvals_list) - args_arr = (ctypes.c_void_p * nargs)(*jl_args) if nargs > 0 else None + try: + if kwargs: + # Dispatch via Core.kwcall(NamedTuple, f, args...) + kwnames_list = list(kwargs.keys()) + boxed_kwvals = [auto_convert_and_root(v) for v in kwargs.values()] + kwvals_list = [ptr for ptr, _ in boxed_kwvals] + refs.extend(ref for _, ref in boxed_kwvals) + nkw = len(kwnames_list) + + kwnames_arr = (ctypes.c_char_p * nkw)(*[k.encode("utf-8") for k in kwnames_list]) + kwvals_arr = (ctypes.c_void_p * nkw)(*kwvals_list) + args_arr = (ctypes.c_void_p * nargs)(*jl_args) if nargs > 0 else None + + result = lib.jl2py_call_kw(self.ptr, args_arr, nargs, + kwnames_arr, kwvals_arr, nkw) + val = _check_result(result) + if val is None or val == 0: + return None + from jl2py.types import wrap_julia_value + return wrap_julia_value(val) + + # Use fixed-arity calls for 0-3 args (slightly faster) + if nargs == 0: + result = lib.jl2py_call0(self.ptr) + elif nargs == 1: + result = lib.jl2py_call1(self.ptr, jl_args[0]) + elif nargs == 2: + result = lib.jl2py_call2(self.ptr, jl_args[0], jl_args[1]) + elif nargs == 3: + result = lib.jl2py_call3(self.ptr, jl_args[0], jl_args[1], jl_args[2]) + else: + arr = (ctypes.c_void_p * nargs)(*jl_args) + result = lib.jl2py_call(self.ptr, arr, nargs) - result = lib.jl2py_call_kw(self.ptr, args_arr, nargs, - kwnames_arr, kwvals_arr, nkw) val = _check_result(result) + + # Wrap result in the appropriate typed wrapper if val is None or val == 0: - return None + return None # Julia returned nothing from jl2py.types import wrap_julia_value return wrap_julia_value(val) - - # Use fixed-arity calls for 0-3 args (slightly faster) - if nargs == 0: - result = lib.jl2py_call0(self.ptr) - elif nargs == 1: - result = lib.jl2py_call1(self.ptr, jl_args[0]) - elif nargs == 2: - result = lib.jl2py_call2(self.ptr, jl_args[0], jl_args[1]) - elif nargs == 3: - result = lib.jl2py_call3(self.ptr, jl_args[0], jl_args[1], jl_args[2]) - else: - arr = (ctypes.c_void_p * nargs)(*jl_args) - result = lib.jl2py_call(self.ptr, arr, nargs) - - val = _check_result(result) - - # Wrap result in the appropriate typed wrapper - if val is None or val == 0: - return None # Julia returned nothing - from jl2py.types import wrap_julia_value - return wrap_julia_value(val) + finally: + release_rooted(refs) # ── Representation ─────────────────────────────────────── diff --git a/out/jl2py/tests/test_arg_rooting.py b/out/jl2py/tests/test_arg_rooting.py new file mode 100644 index 0000000..0e9b819 --- /dev/null +++ b/out/jl2py/tests/test_arg_rooting.py @@ -0,0 +1,73 @@ +"""Regression: freshly-boxed call arguments must survive a GC cycle that +runs while the rest of the argument list is still being assembled. + +box_python_value's box_* calls return a jl_value_t* with nothing on the +Julia side keeping it alive. JuliaVal.__call__ (and a few container +call sites) used to box each argument via a separate ctypes call, collect +the raw pointers into a Python list, and only then pass them to the +consuming Julia call -- leaving every already-boxed element GC-invisible +for the rest of that loop. auto_convert_and_root closes the window by +rooting each value (via jl2py_ref_create) the instant it's boxed. + +These tests force a full GC between each argument's boxing (via a +monkeypatch on auto_convert_and_root) to make the race deterministic +instead of relying on organic GC pressure, which is exactly the kind of +timing that made this bug invisible to the existing test suite. +""" +import os +import sys + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "src")) + +import pytest +from jl2py import convert as _convert + + +@pytest.fixture +def gc_after_every_box(jl): + """Force a full Julia GC immediately after every auto_convert_and_root + call, for the duration of the test.""" + from jl2py._lib import get_lib + lib = get_lib() + + original = _convert.auto_convert_and_root + + def _patched(val): + result = original(val) + lib.jl2py_eval(b"GC.gc(true); GC.gc(true); GC.gc(true)") + return result + + _convert.auto_convert_and_root = _patched + try: + yield + finally: + _convert.auto_convert_and_root = original + + +def test_multi_float_args_survive_gc_during_boxing(jl, gc_after_every_box): + """Regression for the unrooted-list gap: 5 float positional args, each + boxed and GC-stressed before the next is boxed, must all reach Julia + intact.""" + f = jl.eval("(a, b, c, d, e) -> (a, b, c, d, e)") + result = f(1.1, 2.2, 3.3, 4.4, 5.5) + vals = [float(result[i]) for i in range(5)] + assert vals == [1.1, 2.2, 3.3, 4.4, 5.5] + print("[PASS] multi-float positional args survive GC pressure during boxing") + + +def test_multi_float_kwargs_survive_gc_during_boxing(jl, gc_after_every_box): + """Same race, via the kwargs/NamedTuple dispatch path.""" + g = jl.eval("f(; a=0.0, b=0.0, c=0.0) = (a, b, c)") + result = g(a=1.1, b=2.2, c=3.3) + vals = [float(result[i]) for i in range(3)] + assert vals == [1.1, 2.2, 3.3] + print("[PASS] multi-float kwargs survive GC pressure during boxing") + + +def test_dict_setitem_survives_gc_between_key_and_value(jl, gc_after_every_box): + """JuliaDict.__setitem__ boxes key then value before the consuming + call; both must survive a GC cycle in between.""" + d = jl.eval("Dict{Float64,Float64}()") + d[1.5] = 2.5 + assert float(d[1.5]) == 2.5 + print("[PASS] dict key/value survive GC pressure between boxing calls")