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")