diff --git a/majit/majit-backend-cranelift/src/compiler.rs b/majit/majit-backend-cranelift/src/compiler.rs index 3e4ec0bedbc..8cbd8506101 100644 --- a/majit/majit-backend-cranelift/src/compiler.rs +++ b/majit/majit-backend-cranelift/src/compiler.rs @@ -1737,7 +1737,8 @@ fn collect_full_via_active_runtime() { fn collect_step_via_active_runtime() -> majit_gc::GcStepTransition { with_cranelift_gc(|gc| gc.collect_step()).unwrap_or(majit_gc::GcStepTransition { - old_state: majit_gc::GcStepTransition::SCANNING, + // `rgc.py:20-31`: SCANNING on both sides would never report completion. + old_state: majit_gc::GcStepTransition::MARKING, new_state: majit_gc::GcStepTransition::SCANNING, }) } diff --git a/majit/majit-backend-wasm/src/lib.rs b/majit/majit-backend-wasm/src/lib.rs index 9857cccca50..59eea88019c 100644 --- a/majit/majit-backend-wasm/src/lib.rs +++ b/majit/majit-backend-wasm/src/lib.rs @@ -616,7 +616,8 @@ fn wasm_collect_full() { fn wasm_collect_step() -> majit_gc::GcStepTransition { with_wasm_active_gc_mut(|gc| gc.collect_step()).unwrap_or(majit_gc::GcStepTransition { - old_state: majit_gc::GcStepTransition::SCANNING, + // `rgc.py:20-31`: SCANNING on both sides would never report completion. + old_state: majit_gc::GcStepTransition::MARKING, new_state: majit_gc::GcStepTransition::SCANNING, }) } diff --git a/majit/majit-gc/src/collector.rs b/majit/majit-gc/src/collector.rs index 2bd1be84ad0..f777ef18f5f 100644 --- a/majit/majit-gc/src/collector.rs +++ b/majit/majit-gc/src/collector.rs @@ -60,9 +60,9 @@ struct HeapDumpWriter { buffer: Vec, } -// POSIX EIO. `libc`'s wasm32-unknown-unknown surface exposes no errno -// constants, but inspector.py uses EIO for a short raw write on every target. -const HEAP_DUMP_EIO: i32 = 5; +// Some targets and host-seam failures provide no OS errno. Use the POSIX EIO +// value for those cases so every heap-dump failure still carries an error code. +pub const HEAP_DUMP_EIO: i32 = 5; impl HeapDumpWriter { const BUFSIZE: usize = 8192; @@ -94,48 +94,58 @@ impl HeapDumpWriter { return Ok(()); } let byte_len = self.buffer.len() * std::mem::size_of::(); - // Neither `write` nor `_write` exists here, so no call was made and - // `errno` still names some unrelated earlier one. Report the dump's own - // failure code instead of reading a stale `errno`. - #[cfg(not(any(unix, windows)))] - { - let _ = (byte_len, self.fd); - return Err(HEAP_DUMP_EIO); - } - #[cfg(any(unix, windows))] - { - #[cfg(unix)] - let written: isize = unsafe { - libc::write( - self.fd, - self.buffer.as_ptr().cast::(), - byte_len, - ) - }; - // The CRT entry point is `_write`, but `libc` exports it under the - // POSIX name with a `#[link_name = "_write"]` alias, so the Rust - // path is `libc::write` on this target as well. It takes a - // `c_uint` count and returns `c_int`, unlike the `size_t`/`ssize_t` - // unix signature above. - #[cfg(windows)] - let written: isize = unsafe { - libc::write( - self.fd, - self.buffer.as_ptr().cast::(), - byte_len as libc::c_uint, - ) as isize - }; - if written < 0 { - return Err(std::io::Error::last_os_error() - .raw_os_error() - .unwrap_or(HEAP_DUMP_EIO)); + // SAFETY: the initialized `isize` elements occupy exactly `byte_len` + // bytes and remain borrowed for the duration of the write. + let bytes = + unsafe { std::slice::from_raw_parts(self.buffer.as_ptr().cast::(), byte_len) }; + let write_result = if let Some(result) = crate::try_heap_dump_write(self.fd, bytes) { + result + } else { + // Neither `write` nor `_write` exists here, so no call was made and + // `errno` still names some unrelated earlier one. Report the dump's own + // failure code instead of reading a stale `errno`. + #[cfg(not(any(unix, windows)))] + { + Err(HEAP_DUMP_EIO) } - if written as usize != byte_len { - return Err(HEAP_DUMP_EIO); + #[cfg(any(unix, windows))] + { + #[cfg(unix)] + let written: isize = unsafe { + libc::write( + self.fd, + self.buffer.as_ptr().cast::(), + byte_len, + ) + }; + // The CRT entry point is `_write`, but `libc` exports it under the + // POSIX name with a `#[link_name = "_write"]` alias, so the Rust + // path is `libc::write` on this target as well. It takes a + // `c_uint` count and returns `c_int`, unlike the `size_t`/`ssize_t` + // unix signature above. + #[cfg(windows)] + let written: isize = unsafe { + libc::write( + self.fd, + self.buffer.as_ptr().cast::(), + byte_len as libc::c_uint, + ) as isize + }; + if written < 0 { + Err(std::io::Error::last_os_error() + .raw_os_error() + .unwrap_or(HEAP_DUMP_EIO)) + } else { + Ok(written) + } } - self.buffer.clear(); - Ok(()) + }; + let written = write_result?; + if written as usize != byte_len { + return Err(HEAP_DUMP_EIO); } + self.buffer.clear(); + Ok(()) } } diff --git a/majit/majit-gc/src/lib.rs b/majit/majit-gc/src/lib.rs index db46ab92804..358450b444a 100644 --- a/majit/majit-gc/src/lib.rs +++ b/majit/majit-gc/src/lib.rs @@ -1,3 +1,4 @@ +pub use collector::HEAP_DUMP_EIO; pub use gcreftracer::{GcTable, install_gc_table_walker}; /// GC traits and interfaces for the JIT. /// @@ -2461,10 +2462,12 @@ pub fn is_app_level_object(obj: GcRef) -> bool { } pub type DumpRpyHeapFn = fn(i32) -> Result; +pub type HeapDumpWriteFn = fn(i32, &[u8]) -> Result; pub type GetTypeidsTextFn = fn() -> Option>; pub type GetTypeidsListFn = fn() -> Option>; global_hook!(static ACTIVE_DUMP_RPY_HEAP: DumpRpyHeapFn); +global_hook!(static HEAP_DUMP_WRITE: HeapDumpWriteFn); global_hook!(static ACTIVE_GET_TYPEIDS_TEXT: GetTypeidsTextFn); global_hook!(static ACTIVE_GET_TYPEIDS_LIST: GetTypeidsListFn); @@ -2472,6 +2475,17 @@ pub fn set_active_dump_rpy_heap(hook: Option) { ACTIVE_DUMP_RPY_HEAP.set(hook); } +/// Install the host write used by `inspector.py:212-223 HeapDumper.flush`. +/// Sandboxed interpreters use this to translate guest descriptors through +/// their host seam; when absent, the collector retains its native raw write. +pub fn set_heap_dump_write(hook: Option) { + HEAP_DUMP_WRITE.set(hook); +} + +pub(crate) fn try_heap_dump_write(fd: i32, bytes: &[u8]) -> Option> { + HEAP_DUMP_WRITE.get().map(|write| write(fd, bytes)) +} + pub fn set_active_get_typeids_text(hook: Option) { ACTIVE_GET_TYPEIDS_TEXT.set(hook); } @@ -2889,6 +2903,11 @@ pub fn gc_set_enabled(enabled: bool) { /// major-progress path returns early while it is clear, and an explicit /// `gc.collect()` passes `force_enabled` to get past it. pub fn gc_isenabled() -> bool { + // Before `store_singleton` there is no collector to suppress, and nothing + // could have disabled automatic collection yet. + if !gc_sync::is_initialized() { + return true; + } gc_sync::gc_query_reentrant(|gc| gc.isenabled()) } diff --git a/pyre/bench/synth/gc_hook_worker_thread.cranelift.jitstats b/pyre/bench/synth/gc_hook_worker_thread.cranelift.jitstats new file mode 100644 index 00000000000..21df1dd3f43 --- /dev/null +++ b/pyre/bench/synth/gc_hook_worker_thread.cranelift.jitstats @@ -0,0 +1,15 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=0 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=0 +retraces_compiled=0 diff --git a/pyre/bench/synth/gc_hook_worker_thread.dynasm.jitstats b/pyre/bench/synth/gc_hook_worker_thread.dynasm.jitstats new file mode 100644 index 00000000000..21df1dd3f43 --- /dev/null +++ b/pyre/bench/synth/gc_hook_worker_thread.dynasm.jitstats @@ -0,0 +1,15 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=0 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=0 +retraces_compiled=0 diff --git a/pyre/bench/synth/gc_hook_worker_thread.py b/pyre/bench/synth/gc_hook_worker_thread.py new file mode 100644 index 00000000000..842431dec73 --- /dev/null +++ b/pyre/bench/synth/gc_hook_worker_thread.py @@ -0,0 +1,33 @@ +# pyre-check: no-cpython +# pyre-check: skip-backends=wasm +# The wasm guest has no OS-thread implementation, while this fixture verifies +# which native mutator dispatches an object-space GC action. +import _thread +import gc + + +main_ident = _thread.get_ident() +callback_idents = [] +worker_idents = [] +done = _thread.allocate_lock() +done.acquire() + + +def on_collect(stats): + callback_idents.append(_thread.get_ident()) + + +def worker(): + worker_idents.append(_thread.get_ident()) + gc.collect() + done.release() + + +gc.hooks.on_gc_collect = on_collect +_thread.start_new_thread(worker, ()) +done.acquire() + +assert callback_idents +assert callback_idents[-1] == worker_idents[-1] +assert callback_idents[-1] != main_ident +print("gc hook ran on collecting worker") diff --git a/pyre/bench/synth/gc_native_strings_collectable.cranelift.jitstats b/pyre/bench/synth/gc_native_strings_collectable.cranelift.jitstats new file mode 100644 index 00000000000..21df1dd3f43 --- /dev/null +++ b/pyre/bench/synth/gc_native_strings_collectable.cranelift.jitstats @@ -0,0 +1,15 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=0 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=0 +retraces_compiled=0 diff --git a/pyre/bench/synth/gc_native_strings_collectable.dynasm.jitstats b/pyre/bench/synth/gc_native_strings_collectable.dynasm.jitstats new file mode 100644 index 00000000000..21df1dd3f43 --- /dev/null +++ b/pyre/bench/synth/gc_native_strings_collectable.dynasm.jitstats @@ -0,0 +1,15 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=0 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=0 +retraces_compiled=0 diff --git a/pyre/bench/synth/gc_native_strings_collectable.py b/pyre/bench/synth/gc_native_strings_collectable.py new file mode 100644 index 00000000000..669d0d8fce4 --- /dev/null +++ b/pyre/bench/synth/gc_native_strings_collectable.py @@ -0,0 +1,60 @@ +# pyre-check: no-cpython +# pyre-check: skip-backends=wasm + +import _io +import gc +import os +import time + + +results = [] + + +def managed(label, value): + results.append((label, value)) + return value + + +# Native filesystem objects have state-dependent repr branches. +entry = next(os.scandir(".")) +managed("DirEntry repr", os.DirEntry.__repr__(entry)) + +file = _io.FileIO(__file__, "r") +managed("FileIO name repr", _io.FileIO.__repr__(file)) +del file.name +managed("FileIO fd repr", _io.FileIO.__repr__(file)) +file.close() +managed("FileIO closed repr", _io.FileIO.__repr__(file)) + + +# Unix and Windows have separate libc-backed strftime implementations, while +# asctime and ctime share the upstream-style formatter. +calendar = (2020, 2, 3, 4, 5, 6, 0, 34, -1) +strftime_value = time.strftime("%Y-%m-%d %H:%M:%S", calendar) +assert strftime_value == "2020-02-03 04:05:06" +managed("strftime", strftime_value) + +asctime_value = time.asctime(calendar) +assert asctime_value == "Mon Feb 3 04:05:06 2020" +managed("asctime", asctime_value) +managed("ctime", time.ctime(0)) + + +# pyre's mmap implementation is currently Unix-only. The module imports on +# Windows without exposing mmap.mmap, so keep the platform branch in this one +# native fixture instead of maintaining another process/baseline pair. +import mmap + + +if hasattr(mmap, "mmap"): + mapping = mmap.mmap(-1, 1) + managed("mmap live repr", mmap.mmap.__repr__(mapping)) + mapping.close() + managed("mmap closed repr", mmap.mmap.__repr__(mapping)) + + +objects = gc.get_objects() +for label, value in results: + assert any(obj is value for obj in objects), label + +print("native runtime string results are collectable") diff --git a/pyre/bench/synth/gc_runtime_strings_collectable.cranelift.jitstats b/pyre/bench/synth/gc_runtime_strings_collectable.cranelift.jitstats new file mode 100644 index 00000000000..21df1dd3f43 --- /dev/null +++ b/pyre/bench/synth/gc_runtime_strings_collectable.cranelift.jitstats @@ -0,0 +1,15 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=0 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=0 +retraces_compiled=0 diff --git a/pyre/bench/synth/gc_runtime_strings_collectable.dynasm.jitstats b/pyre/bench/synth/gc_runtime_strings_collectable.dynasm.jitstats new file mode 100644 index 00000000000..21df1dd3f43 --- /dev/null +++ b/pyre/bench/synth/gc_runtime_strings_collectable.dynasm.jitstats @@ -0,0 +1,15 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=0 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=0 +retraces_compiled=0 diff --git a/pyre/bench/synth/gc_runtime_strings_collectable.py b/pyre/bench/synth/gc_runtime_strings_collectable.py new file mode 100644 index 00000000000..de645087034 --- /dev/null +++ b/pyre/bench/synth/gc_runtime_strings_collectable.py @@ -0,0 +1,326 @@ +# pyre-check: no-cpython + +import array +import contextvars +import gc +import json +import pickle +import re +import sys +import types +import unicodedata +import weakref +from collections import deque + + +results = [] + + +def managed(label, value): + results.append((label, value)) + return value + + +# _json: encoder/decoder results, error notes, one-shot chunks, and float keys. +json_source = "gc-json-probe-" + ("x" * 37) +managed("json encode", json.encoder.encode_basestring(json_source)) +json_decoded, json_end = json.decoder.scanstring('"' + json_source + '"', 1) +assert json_decoded == json_source +assert json_end == len(json_source) + 2 +managed("json decode", json_decoded) + +try: + json.dumps([object()]) +except TypeError as exc: + if not getattr(exc, "__notes__", None): + exc.add_note("gc-json-note-" + ("x" * 37)) + managed("json note", exc.__notes__[-1]) +else: + raise AssertionError("json.dumps accepted an unsupported object") + +json_chunks = list(json.JSONEncoder().iterencode({"runtime": json_source}, _one_shot=True)) +assert json_chunks +if json.encoder.c_make_encoder is not None: + managed("json chunk", json_chunks[0]) + +float_key = 1.2345678901234567e123 +float_keys = [] + + +def observe_float_key(key): + if key.startswith("1.234567890123456") and key.endswith("e+123"): + float_keys.append(key) + return json.encoder.encode_basestring_ascii(key) + + +if json.encoder.c_make_encoder is None: + managed("json float key oracle", repr(float_key)) +else: + encoder = json.encoder.c_make_encoder( + {}, lambda obj: None, observe_float_key, None, ": ", ", ", False, False, True + ) + assert encoder({float_key: None}, 0) == ['{"1.2345678901234567e+123": null}'] + assert len(float_keys) == 1 + managed("json float key", float_keys[0]) + + +# Formatted repr paths whose ordinary repr formatting is covered elsewhere. +alias_name = "RuntimeAlias" + ("X" * 19) +alias_type = type(alias_name, (), {}) +managed("GenericAlias repr", types.GenericAlias.__repr__(list[alias_type])) + +context_name = "runtime-context-" + ("x" * 29) +context_var = contextvars.ContextVar(context_name) +managed("ContextVar repr", repr(context_var)) +context_token = context_var.set(object()) +managed("ContextVar token repr", repr(context_token)) + +managed("structseq repr", repr(sys.version_info)) +managed("array tounicode", array.array("u", "abc").tounicode()) +managed("array int repr", array.array.__repr__(array.array("i", [1, 2, 3]))) +managed("array unicode repr", array.array.__repr__(array.array("u", "abc"))) +managed("deque repr", deque.__repr__(deque([1, 2, 3], maxlen=4))) + + +# _sre: final repr/substitution strings and every slice-container assembly shape. +pattern = re.compile(r"(?P[a-z]+)-(?P\d+)") +subject = "alpha-123-omega" +match = pattern.search(subject) +managed("SRE pattern repr", type(pattern).__repr__(pattern)) +managed("SRE match repr", type(match).__repr__(match)) +managed("SRE sub", pattern.sub("word", subject)) +subn_result, subn_count = pattern.subn("word", subject) +assert subn_count == 1 +managed("SRE subn", subn_result) +managed("SRE expand", match.expand(r"<\1:\2>")) + +managed("SRE group", match.group(1)) +managed("SRE getitem", match[2]) +managed("SRE multi-group", match.group(1, 2)[0]) +managed("SRE groups", match.groups()[1]) +managed("SRE groupdict", match.groupdict()["word"]) +managed("SRE findall plain", re.findall(r"[a-z]+", subject)[0]) +managed("SRE findall single", re.findall(r"([a-z]+)", subject)[1]) +managed("SRE findall multiple", pattern.findall(subject)[0][1]) +split_parts = re.split(r"(-)", subject) +managed("SRE split text", split_parts[0]) +managed("SRE split group", split_parts[1]) + + +class Text(str): + pass + + +unchanged = pattern.sub("replacement", Text("no match here")) +assert type(unchanged) is str +managed("SRE subclass normalization", unchanged) + + +# _pickle's three Unicode opcodes share one loader but have distinct lengths. +pickle_text = b"pickle runtime string" +pickle_payloads = ( + b"\x80\x04\x8c" + bytes([len(pickle_text)]) + pickle_text + b".", + b"\x80\x04X" + len(pickle_text).to_bytes(4, "little") + pickle_text + b".", + b"\x80\x04\x8d" + len(pickle_text).to_bytes(8, "little") + pickle_text + b".", +) +pickle_opcodes = ("SHORT_BINUNICODE", "BINUNICODE", "BINUNICODE8") +for opcode, payload in zip(pickle_opcodes, pickle_payloads): + managed(opcode, pickle.loads(payload)) + + +# normalize has separate non-ASCII, exact-ASCII identity, and subclass paths. +for form, source, expected in ( + ("NFC", "e\u0301 runtime", "é runtime"), + ("NFD", "é runtime", "e\u0301 runtime"), + ("NFKC", "\ufb03 runtime", "ffi runtime"), + ("NFKD", "\ufb03 runtime", "ffi runtime"), +): + normalized = unicodedata.normalize(form, source) + assert normalized == expected + managed("normalize " + form, normalized) + +ascii_source = "".join(["ascii", " runtime"]) +ascii_result = unicodedata.normalize("NFC", ascii_source) +assert ascii_result is ascii_source +managed("normalize ASCII identity", ascii_result) +subclass_result = unicodedata.normalize("NFC", Text("subclass runtime")) +assert type(subclass_result) is str +managed("normalize subclass", subclass_result) + + +# SimpleNamespace's recursive guard is a separate result branch. +namespace = types.SimpleNamespace(alpha="value", count=3) +managed("SimpleNamespace repr", types.SimpleNamespace.__repr__(namespace)) +recursive_result = None + + +class CaptureRecursiveRepr: + def __repr__(self): + global recursive_result + recursive_result = repr(recursive_namespace) + return "captured" + + +recursive_namespace = types.SimpleNamespace(value=CaptureRecursiveRepr()) +assert repr(recursive_namespace) == "namespace(value=captured)" +assert recursive_result == "namespace(...)" +managed("SimpleNamespace recursive repr", recursive_result) + + +# BaseException rendering has arity/identity/specialized branches. +for label, exception in ( + ("exception repr empty", ValueError()), + ("exception repr single", ValueError("x")), + ("exception repr multiple", ValueError("x", 1)), +): + managed(label, BaseException.__repr__(exception)) + + +class Render: + def __init__(self, result): + self.result = result + + def __str__(self): + return self.result + + +runtime_text = "runtime-exception-value-" + str(id(gc)) +rendered_text = "rendered-exception-value-" + str(id(runtime_text)) +assert BaseException.__str__(ValueError(runtime_text)) is runtime_text +managed("exception str identity", runtime_text) +assert BaseException.__str__(ValueError(Render(rendered_text))) is rendered_text +managed("exception str delegated identity", rendered_text) +managed("exception str multiple", BaseException.__str__(ValueError(runtime_text, 1))) +managed("KeyError str", KeyError.__str__(KeyError(runtime_text))) + +group_message = "runtime-exception-group-message-" + str(id(results)) +group = ExceptionGroup(group_message, [ValueError("runtime group leaf")]) +managed("ExceptionGroup str", ExceptionGroup.__str__(group)) +managed("ExceptionGroup repr", ExceptionGroup.__repr__(group)) + + +# Direct descriptor paths share typedef formatting but have distinct owners. +class RuntimeOwner: + def runtime_method(self): + pass + + +owner = RuntimeOwner() +managed("bound method repr", type(owner.runtime_method).__repr__(owner.runtime_method)) +managed("super repr", super.__repr__(super(RuntimeOwner, owner))) +union = int | str +managed("union repr", type(union).__repr__(union)) + + +def runtime_function(): + pass + + +managed("function repr", type(runtime_function).__repr__(runtime_function)) +managed("builtin function repr", type(len).__repr__(len)) +managed("builtin method repr", type([].append).__repr__([].append)) +wrapper = (1).__add__ +managed("method-wrapper repr", type(wrapper).__repr__(wrapper)) + + +class RuntimeType: + pass + + +managed("type repr", type.__repr__(RuntimeType)) + + +def make_cell(value): + def inner(): + return value + + return inner.__closure__[0] + + +filled_cell = make_cell(42) +empty_cell = make_cell(42) +del empty_cell.cell_contents +managed("filled cell repr", type(filled_cell).__repr__(filled_cell)) +managed("empty cell repr", type(empty_cell).__repr__(empty_cell)) + + +def generate(): + yield 1 + + +generator = generate() +managed("generator repr", type(generator).__repr__(generator)) +generator.close() + + +async def run(): + return 1 + + +coroutine = run() +managed("coroutine repr", type(coroutine).__repr__(coroutine)) +coroutine.close() + + +class SlotOwner: + __slots__ = ("value",) + + +for label, descriptor in ( + ("getset descriptor repr", type.__dict__["__name__"]), + ("member descriptor repr", SlotOwner.__dict__["value"]), + ("method descriptor repr", list.__dict__["append"]), + ("classmethod descriptor repr", dict.__dict__["fromkeys"]), +): + managed(label, type(descriptor).__repr__(descriptor)) + + +# Stateful reprs have distinct live/released and live/dead branches. +view = memoryview(b"x") +managed("memoryview live repr", memoryview.__repr__(view)) +view.release() +managed("memoryview released repr", memoryview.__repr__(view)) + +weak_marker = "".join(["dynamic", " weak proxy"]) + + +class Referent: + def __str__(self): + return weak_marker + + +class CallableReferent: + def __call__(self): + pass + + +referent = Referent() +callable_referent = CallableReferent() +reference = weakref.ref(referent) +proxy = weakref.proxy(referent) +callable_proxy = weakref.proxy(callable_referent) +proxy_text = type(proxy).__str__(proxy) +assert proxy_text is weak_marker +managed("weak proxy str identity", proxy_text) +managed("weakref live repr", weakref.ReferenceType.__repr__(reference)) +managed("weak proxy live repr", weakref.ProxyType.__repr__(proxy)) +managed("callable weak proxy live repr", weakref.CallableProxyType.__repr__(callable_proxy)) + +del referent +del callable_referent +gc.collect() +for label, value in ( + ("weakref dead repr", weakref.ReferenceType.__repr__(reference)), + ("weak proxy dead repr", weakref.ProxyType.__repr__(proxy)), + ("callable weak proxy dead repr", weakref.CallableProxyType.__repr__(callable_proxy)), +): + assert "; dead>" in value + managed(label, value) + + +# Take one heap census after every result is rooted in `results`. +objects = gc.get_objects() +for label, value in results: + assert any(obj is value for obj in objects), label + +print("runtime string results are collectable") diff --git a/pyre/bench/synth/gc_runtime_strings_collectable.wasm.jitstats b/pyre/bench/synth/gc_runtime_strings_collectable.wasm.jitstats new file mode 100644 index 00000000000..21df1dd3f43 --- /dev/null +++ b/pyre/bench/synth/gc_runtime_strings_collectable.wasm.jitstats @@ -0,0 +1,15 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=0 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=0 +retraces_compiled=0 diff --git a/pyre/pyre-interpreter/src/_pypy_generic_alias.rs b/pyre/pyre-interpreter/src/_pypy_generic_alias.rs index f8207656d32..055327057e1 100644 --- a/pyre/pyre-interpreter/src/_pypy_generic_alias.rs +++ b/pyre/pyre-interpreter/src/_pypy_generic_alias.rs @@ -215,7 +215,9 @@ fn self_alias(args: &[PyObjectRef]) -> Result { /// `GenericAlias.__repr__` (`_pypy_generic_alias.py:57`). fn ga_repr(args: &[PyObjectRef]) -> crate::PyResult { let self_ = self_alias(args)?; - Ok(pyre_object::w_str_from_wtf8(unsafe { repr(self_)? })) + Ok(pyre_object::w_str_from_wtf8_managed(unsafe { + repr(self_)? + })) } /// `GenericAlias.__hash__` (`_pypy_generic_alias.py:82`). diff --git a/pyre/pyre-interpreter/src/_structseq.rs b/pyre/pyre-interpreter/src/_structseq.rs index f82a998eaad..0e32c780707 100644 --- a/pyre/pyre-interpreter/src/_structseq.rs +++ b/pyre/pyre-interpreter/src/_structseq.rs @@ -172,7 +172,7 @@ fn structseq_repr(args: &[PyObjectRef]) -> Result { out.push_wtf8(&unsafe { crate::py_repr_wtf8(item)? }); } out.push_str(")"); - Ok(pyre_object::w_str_from_wtf8(out)) + Ok(pyre_object::w_str_from_wtf8_managed(out)) } /// `lib_pypy/_structseq.py structseq_reduce` — `return type(self), diff --git a/pyre/pyre-interpreter/src/builtins.rs b/pyre/pyre-interpreter/src/builtins.rs index 3a26d3da88b..983496a7554 100644 --- a/pyre/pyre-interpreter/src/builtins.rs +++ b/pyre/pyre-interpreter/src/builtins.rs @@ -1771,7 +1771,9 @@ fn memoryview_repr(args: &[PyObjectRef]) -> Result } else { "memory" }; - Ok(w_str_new(&format!( + // baseobjspace.py:115-117 `getrepr` returns `space.newtext(...)` for + // both the live and released labels. + Ok(w_str_new_managed(&format!( "<{label} at {}>", crate::display::repr_addr(mv as usize) ))) @@ -7601,9 +7603,46 @@ fn exc_system_exit_init(args: &[PyObjectRef]) -> crate::PyResult { /// which reports the args alone. It is what `BaseException.__str__(exc)` /// runs even when `exc`'s own class registers a `descr_str` override. fn base_exception_str_method(args: &[PyObjectRef]) -> crate::PyResult { - let obj = args[0]; - let text = unsafe { crate::display::base_exception_str_wtf8(obj)? }; - Ok(pyre_object::w_str_from_wtf8(text)) + // `W_BaseException.descr_str` reads the internal `args_w` list directly: + // no args returns the empty text singleton, one arg returns + // `space.str(args_w[0])` unchanged, and several args stringify a fresh + // tuple. In particular, flattening the one-arg result through WTF-8 + // loses both its identity and its managed lifetime. + let _roots = pyre_object::gc_roots::push_roots(); + let obj_slot = pyre_object::gc_roots::pin_roots(&[args[0]]); + let obj = pyre_object::gc_roots::shadow_stack_get(obj_slot); + let stored = unsafe { pyre_object::interp_exceptions::w_exception_get_args_storage(obj) }; + let len = if stored.is_null() { + 0 + } else if unsafe { pyre_object::is_list(stored) } { + unsafe { pyre_object::w_list_len(stored) } + } else if unsafe { pyre_object::is_tuple(stored) } { + unsafe { pyre_object::w_tuple_len(stored) } + } else { + 0 + }; + if len == 0 { + return Ok(w_str_new("")); + } + if len == 1 { + let first = if unsafe { pyre_object::is_list(stored) } { + unsafe { pyre_object::w_list_getitem(stored, 0) } + } else { + unsafe { pyre_object::w_tuple_getitem(stored, 0) } + } + .unwrap_or(pyre_object::PY_NULL); + pyre_object::gc_roots::pin_root(first); + let first_slot = pyre_object::gc_roots::shadow_stack_len() - 1; + return builtin_str(&[pyre_object::gc_roots::shadow_stack_get(first_slot)]); + } + let tuple = unsafe { + pyre_object::interp_exceptions::w_exception_get_args( + pyre_object::gc_roots::shadow_stack_get(obj_slot), + ) + }; + pyre_object::gc_roots::pin_root(tuple); + let tuple_slot = pyre_object::gc_roots::shadow_stack_len() - 1; + builtin_str(&[pyre_object::gc_roots::shadow_stack_get(tuple_slot)]) } /// The `descr_str` of the class that registers one, falling back to @@ -7611,13 +7650,18 @@ fn base_exception_str_method(args: &[PyObjectRef]) -> crate::PyResult { /// (`KeyError('a', 'b')`, an `OSError` with neither errno nor strerror). fn exception_str_method(args: &[PyObjectRef]) -> crate::PyResult { let obj = args[0]; - let text = unsafe { - match crate::display::exception_kind_str_wtf8(obj)? { - Some(s) => s, - None => crate::display::base_exception_str_wtf8(obj)?, - } + let Some(text) = (unsafe { crate::display::exception_kind_str_wtf8(obj)? }) else { + // The subclass has no `descr_str` override for this argument shape, + // so inherit `W_BaseException.descr_str` as PyPy does. Calling the + // object-returning path matters for one argument: `space.str(arg)` + // preserves an exact str's identity instead of flattening it through + // WTF-8 and allocating a replacement. + return base_exception_str_method(args); }; - Ok(pyre_object::w_str_from_wtf8(text)) + // A builtin override (KeyError / OSError / UnicodeError / SyntaxError) + // constructs a fresh text result. It is an ordinary GC object, not a + // prebuilt constant tied to the descriptor. + Ok(pyre_object::w_str_from_wtf8_managed(text)) } /// `interp_exceptions.py:135-151 W_BaseException.descr_repr` — every builtin @@ -7625,7 +7669,9 @@ fn exception_str_method(args: &[PyObjectRef]) -> crate::PyResult { /// alone and reads the receiver's own class name. fn exception_repr_method(args: &[PyObjectRef]) -> crate::PyResult { let obj = args[0]; - Ok(pyre_object::w_str_from_wtf8(unsafe { + // W_BaseException.descr_repr returns `space.newtext(clsname + args_repr)` + // for the zero-, one-, and multi-argument forms. + Ok(pyre_object::w_str_from_wtf8_managed(unsafe { crate::display::py_repr_wtf8(obj)? })) } @@ -8981,7 +9027,7 @@ fn exception_group_str(args: &[PyObjectRef]) -> Result Result { @@ -9028,7 +9074,7 @@ fn exception_group_repr(args: &[PyObjectRef]) -> Result Result Result"))); + return Ok(pyre_object::w_str_new_managed(&format!( + "<{repr_type} [closed]>" + ))); } let closefd = if file_closefd(self_obj) { "True" @@ -15173,11 +15226,11 @@ fn fileio_method_repr(args: &[PyObjectRef]) -> Result" - ))) + // W_FileIO.repr_w returns `space.newtext(...)` for the name, fd, and + // closed forms. + Ok(pyre_object::w_str_from_wtf8_managed( + crate::display::wtf8_format!(format!("<{repr_type} "), body, ">"), + )) } /// `_io.FileIO.__init__` — PyPy `W_FileIO.descr_init`. diff --git a/pyre/pyre-interpreter/src/eval.rs b/pyre/pyre-interpreter/src/eval.rs index 11401545cb9..912e6b6e4d6 100644 --- a/pyre/pyre-interpreter/src/eval.rs +++ b/pyre/pyre-interpreter/src/eval.rs @@ -953,17 +953,6 @@ pub unsafe fn walk_pyframe_roots_area( } { visitor(unsafe { &mut *(hook as *mut majit_ir::GcRef) }); } - // pending_with_disabled_del is a GC-visible list upstream - // (executioncontext.py:652); pyre's Vec lives in the boxed - // UserDelAction, so its element slots are visited here. - let action = unsafe { (*ec).user_del_action }; - if !action.is_null() - && let Some(list) = unsafe { (*action).pending_with_disabled_del.as_mut() } - { - for slot in list.iter_mut() { - visitor(unsafe { &mut *(slot as *mut PyObjectRef as *mut majit_ir::GcRef) }); - } - } }; visit_ec_slots(frame_ec); if ambient_ec != frame_ec { @@ -1177,6 +1166,7 @@ pub unsafe fn walk_pyframe_roots_area( /// faulthandler's — so it registers once for the process. fn walk_interpreter_global_roots(visitor: &mut dyn FnMut(&mut majit_ir::GcRef)) { walk_global_prebuilt_roots(visitor); + crate::executioncontext::walk_space_user_del_action_roots(visitor); crate::module::gc::hook::walk_hook_roots(visitor); crate::module::thread::walk_thread_roots(visitor); #[cfg(all(not(target_arch = "wasm32"), not(feature = "sandbox")))] diff --git a/pyre/pyre-interpreter/src/executioncontext.rs b/pyre/pyre-interpreter/src/executioncontext.rs index fe4fac2fe6f..a7391ed88d6 100644 --- a/pyre/pyre-interpreter/src/executioncontext.rs +++ b/pyre/pyre-interpreter/src/executioncontext.rs @@ -214,14 +214,52 @@ impl WRootFinalizerQueue { } fn finalizer_queue_trigger() { - let ec = crate::call::getexecutioncontext() as *mut ExecutionContext; - if ec.is_null() { + let action = space_user_del_action(); + if !action.is_null() { + unsafe { (*action).fire() }; + } +} + +// `baseobjspace.py:450` / `executioncontext.py:724`: both the action flag and +// the user-finalizer action belong to the object space, not to an individual +// execution context. Pyre does not yet expose a typed Rust `ObjSpace`, so the +// process-global runtime instance is its storage owner. ECs carry a thin +// reference to the flag, while finalizer users resolve the action here just as +// PyPy reaches `self.space.user_del_action`. +static SPACE_USER_DEL_ACTION: OnceLock = OnceLock::new(); + +fn install_space_user_del_action( + space: PyObjectRef, + actionflag: &mut (dyn ActionFlagOps + 'static), +) -> *mut UserDelAction { + *SPACE_USER_DEL_ACTION + .get_or_init(|| Box::into_raw(UserDelAction::new(space, actionflag)) as usize) + as *mut UserDelAction +} + +pub fn space_user_del_action() -> *mut UserDelAction { + SPACE_USER_DEL_ACTION.get().copied().unwrap_or(0) as *mut UserDelAction +} + +/// Trace the object-space-owned finalizer action exactly once as a non-stack +/// root. In translated PyPy this happens through the object-space graph; the +/// leaked Rust allocation is outside that graph, so its managed slots must be +/// forwarded explicitly. +pub(crate) fn walk_space_user_del_action_roots(visitor: &mut dyn FnMut(&mut majit_ir::GcRef)) { + let action = space_user_del_action(); + if action.is_null() { return; } - unsafe { - let action = (*ec).user_del_action; - if !action.is_null() { - (*action).fire(); + let action = unsafe { &mut *action }; + let mut forward = |slot: &mut PyObjectRef| { + if !slot.is_null() { + visitor(unsafe { &mut *(slot as *mut PyObjectRef as *mut majit_ir::GcRef) }); + } + }; + forward(&mut action.base.space); + if let Some(pending) = action.pending_with_disabled_del.as_mut() { + for obj in pending { + forward(obj); } } } @@ -281,10 +319,9 @@ pub struct ExecutionContext { /// `os_local.py:ExecutionContext._thread_local_objs`: weak references to /// `_thread._local` objects which own a dictionary for this EC. pub thread_local_refs: Vec, - pub actionflag: ActionFlag, - /// `space.user_del_action`, allocated after the ExecutionContext reaches - /// its stable process-lifetime address. - pub user_del_action: *mut UserDelAction, + /// Cached access to process-owned `space.actionflag`. Every EC points to + /// the same flag, matching `baseobjspace.py:450`. + pub actionflag: SpaceActionFlag, /// `pypy/objspace/std/dictmultiobject.py:60-69 /// allocate_and_init_instance(module=True)` parity — the builtins /// module's `w_dict` is a `W_ModuleDictObject` backed by @@ -307,14 +344,6 @@ pub struct ExecutionContext { /// pointer into the leaked action owned by `module::_signal` /// (`install_signal_handling`); `None` until installed. pub check_signal_action: Option<*mut dyn AsyncActionOps>, - /// `gil.py:20-23 GILThreadLocals.initialize` — the `GILReleaseAction` - /// registered on the ticker, which yields the GIL every - /// `sys.getcheckinterval()` bytecodes. There is one per execution context, - /// because the ticker it is registered on is too, so unlike the - /// process-global signal action it is owned rather than leaked: the - /// pointer is the box `module::thread::gil::shutdown` gives back. `None` - /// until installed. - pub gil_release_action: Option<*mut dyn AsyncActionOps>, /// `executioncontext.py sys_exc_operror` — the active exception for /// `sys.exc_info()` / bare `raise`, saved/restored across handler /// regions by PUSH_EXC_INFO / POP_EXCEPT. Single source of truth @@ -400,12 +429,10 @@ impl ExecutionContext { trace_all_generation: 0, profile_all_generation: 0, thread_local_refs: Vec::new(), - actionflag: ActionFlag::new(), - user_del_action: std::ptr::null_mut(), + actionflag: SpaceActionFlag::new(), builtins_module, builtin_dict_cache: std::cell::Cell::new(pyre_object::PY_NULL), check_signal_action: None, - gil_release_action: None, sys_exc_value: pyre_object::PY_NULL, coroutine_origin_tracking_depth: 0, current_gen_or_coroutine: pyre_object::PY_NULL, @@ -418,9 +445,9 @@ impl ExecutionContext { /// `OSThreadLocals.enter_thread()` / `ExecutionContext(space)` parity. /// - /// Interpreter-wide objects (the object space and builtins module) remain - /// shared, while frame, exception, tracing, profiling, generator, and - /// action state starts fresh for each OS thread. + /// Interpreter-wide objects (the object space, action flag, finalizer + /// action, and builtins module) remain shared, while frame, exception, + /// tracing, profiling, and generator state starts fresh for each OS thread. pub fn clone_for_thread(&self) -> Self { let mut ec = self.clone(); ec.topframeref = std::ptr::null_mut(); @@ -437,11 +464,7 @@ impl ExecutionContext { ec.trace_all_generation = 0; ec.profile_all_generation = 0; ec.thread_local_refs.clear(); - ec.actionflag = ActionFlag::new(); - // The periodic actions are registered on the actionflag just replaced, - // so the new thread registers its own. - ec.gil_release_action = None; - ec.user_del_action = std::ptr::null_mut(); + ec.actionflag = SpaceActionFlag::new(); // The cached Module wrapper is execution-context-owned and movable. // A new thread lazily builds its own wrapper over the shared // builtins_module, matching a fresh PyPy ExecutionContext. @@ -482,11 +505,9 @@ impl ExecutionContext { } pub fn install_user_del_action(&mut self) { - if self.user_del_action.is_null() { - let action = UserDelAction::new(self.space, &mut self.actionflag); - self.user_del_action = Box::into_raw(action); - } - crate::module::gc::hook::initialize(self.space, &mut self.actionflag); + let actionflag = self.actionflag.shared_mut(); + install_space_user_del_action(self.space, actionflag); + crate::module::gc::hook::initialize(self.space, actionflag); pyre_object::gc_hook::register_maybe_finalizer_hook(maybe_register_user_finalizer); } @@ -821,8 +842,9 @@ impl ExecutionContext { } pub fn _run_finalizers_now(&mut self) { - if !self.user_del_action.is_null() { - unsafe { (*self.user_del_action)._run_finalizers() }; + let action = space_user_del_action(); + if !action.is_null() { + unsafe { (*action)._run_finalizers() }; } } @@ -842,8 +864,9 @@ impl ExecutionContext { /// coroutine's frame. The action runs before the next opcode, after the /// attribute receiver has left the value stack. pub fn finalize_discarded_coroutine_after_frame_get(&mut self) { - if !self.user_del_action.is_null() { - unsafe { (*self.user_del_action).collect_oldgen_and_fire() }; + let action = space_user_del_action(); + if !action.is_null() { + unsafe { (*action).collect_oldgen_and_fire() }; } } @@ -1927,6 +1950,85 @@ impl ActionFlagOps for ActionFlag { } } +/// Stable reference to the process-owned `space.actionflag`. +/// +/// PyPy stores one `ActionFlag` on `ObjSpace` and every execution context +/// reaches it through `self.space.actionflag` (`executioncontext.py:163-165`). +/// Pyre's opaque `PyObjectRef` space has no typed Rust fields yet, so the +/// process runtime owns the equivalent allocation and each EC carries this +/// thin reference. The GIL serializes interpreter access just as it does for +/// PyPy's plain Python fields; the OS signal handler is the only asynchronous +/// writer and already uses the registered ticker address. +#[derive(Clone, Copy)] +pub struct SpaceActionFlag { + ptr: *mut ActionFlag, +} + +static SPACE_ACTIONFLAG: OnceLock = OnceLock::new(); + +impl Default for SpaceActionFlag { + fn default() -> Self { + Self::new() + } +} + +impl SpaceActionFlag { + pub fn new() -> Self { + let ptr = *SPACE_ACTIONFLAG + .get_or_init(|| Box::into_raw(Box::new(ActionFlag::new())) as usize) + as *mut ActionFlag; + Self { ptr } + } + + fn inner(&self) -> &ActionFlag { + // SAFETY: SPACE_ACTIONFLAG owns one leaked process-lifetime value. + // Interpreter calls are serialized by the GIL; shared signal writes + // touch only `_ticker` through its registered raw address. + unsafe { &*self.ptr } + } + + fn inner_mut(&mut self) -> &mut ActionFlag { + // SAFETY: see `inner`; callers hold the GIL while mutating the flag. + unsafe { &mut *self.ptr } + } + + /// The stable object-space allocation used when an action caches its + /// `space.actionflag` back-reference. Returning the shared allocation, + /// rather than this EC's thin wrapper, keeps the pointer valid even if an + /// embedding drops the EC that first registered the action. + pub fn shared_mut(&mut self) -> &'static mut ActionFlag { + // SAFETY: SPACE_ACTIONFLAG owns one leaked process-lifetime value. + // Registration and action dispatch are serialized by the GIL. + unsafe { &mut *self.ptr } + } + + pub fn ticker_addr(&mut self) -> *mut isize { + self.inner_mut().ticker_addr() + } +} + +impl ActionFlagOps for SpaceActionFlag { + fn abstract_flag(&self) -> &AbstractActionFlag { + self.inner().abstract_flag() + } + + fn abstract_flag_mut(&mut self) -> &mut AbstractActionFlag { + self.inner_mut().abstract_flag_mut() + } + + fn get_ticker(&self) -> isize { + self.inner().get_ticker() + } + + fn reset_ticker(&mut self, value: isize) { + self.inner_mut().reset_ticker(value); + } + + fn decrement_ticker(&mut self, by: isize) -> isize { + self.inner_mut().decrement_ticker(by) + } +} + pub struct AsyncAction { pub space: PyObjectRef, _action_index: isize, @@ -1938,12 +2040,11 @@ pub struct AsyncAction { /// uses `self.space.actionflag` — a constant lookup once the action /// is constructed because PyPy's `space.actionflag` is set once at /// `pypy/interpreter/baseobjspace.py:447` and never replaced. - /// Pyre keeps the actionflag on `ExecutionContext` rather than on - /// `space`, so we cache the back-reference at registration time - /// (`AsyncAction::new` / `UserDelAction::new` / - /// `AsyncActionOps::register_periodic_action`). The pointer is - /// stable for the process lifetime because pyrex owns the EC for - /// the entire run. `fire()` dereferences this slot directly, + /// Pyre's EC stores a thin reference to the process-owned flag, so we cache + /// the registering reference here (`AsyncAction::new` / + /// `UserDelAction::new` / `AsyncActionOps::register_periodic_action`). + /// The process-owned flag outlives every action. `fire()` dereferences + /// this slot directly, /// matching PyPy's `self.space.actionflag.fire(self)` 1:1 without /// the TLS detour. `null` means the action has not been registered /// yet — calling `fire` in that state is a programmer error diff --git a/pyre/pyre-interpreter/src/function.rs b/pyre/pyre-interpreter/src/function.rs index 4ddde28e86f..d112d59f8c5 100644 --- a/pyre/pyre-interpreter/src/function.rs +++ b/pyre/pyre-interpreter/src/function.rs @@ -2873,13 +2873,9 @@ pub unsafe fn descr_method_repr(obj: PyObjectRef) -> Result" - ))) + Ok(pyre_object::w_str_from_wtf8_managed( + crate::display::wtf8_format!(""), + )) } #[inline] diff --git a/pyre/pyre-interpreter/src/module/_collections/mod.rs b/pyre/pyre-interpreter/src/module/_collections/mod.rs index bc24350941d..f050e85af2b 100644 --- a/pyre/pyre-interpreter/src/module/_collections/mod.rs +++ b/pyre/pyre-interpreter/src/module/_collections/mod.rs @@ -1302,7 +1302,7 @@ impl W_Deque { Some(m) => out.push_str(&format!("], maxlen={m})")), None => out.push_str("])"), } - Ok(pyre_object::w_str_from_wtf8(out)) + Ok(pyre_object::w_str_from_wtf8_managed(out)) } #[getter] diff --git a/pyre/pyre-interpreter/src/module/_contextvars/mod.rs b/pyre/pyre-interpreter/src/module/_contextvars/mod.rs index ae1ea26f182..91c82b875f8 100644 --- a/pyre/pyre-interpreter/src/module/_contextvars/mod.rs +++ b/pyre/pyre-interpreter/src/module/_contextvars/mod.rs @@ -249,9 +249,9 @@ fn context_var_repr_string(obj: PyObjectRef) -> Result Result { - Ok(pyre_object::w_str_from_wtf8(context_var_repr_string( - args[0], - )?)) + Ok(pyre_object::w_str_from_wtf8_managed( + context_var_repr_string(args[0])?, + )) } fn token_type() -> PyObjectRef { @@ -384,7 +384,9 @@ fn token_repr_string(token: PyObjectRef) -> Result Result { - Ok(pyre_object::w_str_from_wtf8(token_repr_string(args[0])?)) + Ok(pyre_object::w_str_from_wtf8_managed(token_repr_string( + args[0], + )?)) } fn token_enter(args: &[PyObjectRef]) -> Result { diff --git a/pyre/pyre-interpreter/src/module/_json/mod.rs b/pyre/pyre-interpreter/src/module/_json/mod.rs index e2319ae62a1..4556d2dbc04 100644 --- a/pyre/pyre-interpreter/src/module/_json/mod.rs +++ b/pyre/pyre-interpreter/src/module/_json/mod.rs @@ -98,9 +98,9 @@ fn json_decode_error(msg: String, doc: PyObjectRef, pos: usize) -> PyError { fn encode_basestring_impl(obj: PyObjectRef, ascii_only: bool) -> PyResult { let value = require_string(obj)?; - Ok(pyre_object::w_str_from_wtf8(machinery::encode_string( - value, ascii_only, - ))) + Ok(pyre_object::w_str_from_wtf8_managed( + machinery::encode_string(value, ascii_only), + )) } fn scanstring_impl(doc: PyObjectRef, end: i64, strict_obj: PyObjectRef) -> PyResult { @@ -115,10 +115,17 @@ fn scanstring_impl(doc: PyObjectRef, end: i64, strict_obj: PyObjectRef) -> PyRes json_decode_error("Unterminated string starting at".to_owned(), doc, start) })?; match machinery::scan_string(rest, start, strict) { - Ok((decoded, next, _)) => Ok(pyre_object::w_tuple_new(vec![ - pyre_object::w_str_from_wtf8(decoded), - pyre_object::w_int_new(next as i64), - ])), + Ok((decoded, next, _)) => { + let _roots = gc_roots::push_roots(); + let decoded_slot = gc_roots::shadow_stack_len(); + gc_roots::pin_root(pyre_object::w_str_from_wtf8_managed(decoded)); + let next_slot = gc_roots::shadow_stack_len(); + gc_roots::pin_root(pyre_object::w_int_new(next as i64)); + Ok(pyre_object::w_tuple_new(vec![ + gc_roots::shadow_stack_get(decoded_slot), + gc_roots::shadow_stack_get(next_slot), + ])) + } Err(err) => Err(json_decode_error(err.msg, doc, err.pos)), } } @@ -818,7 +825,7 @@ fn add_json_note(mut err: PyError, note: impl Into) -> gc_roots::pin_root(exc); // The note quotes a key the caller supplied, which may hold a lone // surrogate, so it is carried as the WTF-8 it is. - let note = pyre_object::w_str_from_wtf8(note.into()); + let note = pyre_object::w_str_from_wtf8_managed(note.into()); let note_slot = gc_roots::shadow_stack_len(); gc_roots::pin_root(note); if let Ok(add_note) = @@ -1058,7 +1065,7 @@ fn coerce_key(self_obj: PyObjectRef, key: PyObjectRef) -> Result PyResult { let encoded = encode_value(self_obj, obj, level.max(0))?; - Ok(pyre_object::w_list_new(vec![pyre_object::w_str_from_wtf8( - encoded, + let _roots = gc_roots::push_roots(); + let encoded_slot = gc_roots::shadow_stack_len(); + gc_roots::pin_root(pyre_object::w_str_from_wtf8_managed(encoded)); + Ok(pyre_object::w_list_new(vec![gc_roots::shadow_stack_get( + encoded_slot, )])) } diff --git a/pyre/pyre-interpreter/src/module/_pickle/mod.rs b/pyre/pyre-interpreter/src/module/_pickle/mod.rs index 8c507e9de40..bf5f970a7f7 100644 --- a/pyre/pyre-interpreter/src/module/_pickle/mod.rs +++ b/pyre/pyre-interpreter/src/module/_pickle/mod.rs @@ -621,7 +621,7 @@ pub(crate) fn str_from_utf8(data: &[u8]) -> Result { // surrogates are valid pickle payloads and are represented internally as // WTF-8, while malformed byte sequences must still be rejected. let s = rustpython_wtf8::Wtf8Buf::from_bytes(data.to_vec()).map_err(utf8_decode_error)?; - Ok(pyre_object::unicodeobject::w_str_from_wtf8(s)) + Ok(pyre_object::unicodeobject::w_str_from_wtf8_managed(s)) } /// Construct Python's UTF-8 decode error details from Rust's `Utf8Error`. diff --git a/pyre/pyre-interpreter/src/module/_sre/interp_sre.rs b/pyre/pyre-interpreter/src/module/_sre/interp_sre.rs index 0abd501914a..c794afaf09a 100644 --- a/pyre/pyre-interpreter/src/module/_sre/interp_sre.rs +++ b/pyre/pyre-interpreter/src/module/_sre/interp_sre.rs @@ -682,6 +682,25 @@ impl Subject { } } +/// One translated live GCREF in the shadow stack. RPython's GC transform +/// inserts the equivalent push/reload around allocations automatically; +/// Rust collections otherwise retain the pre-move pointer while `_sre` +/// builds tuples, lists, and dictionaries of freshly sliced strings. +#[derive(Clone, Copy)] +struct RootedObject(usize); + +impl RootedObject { + fn pin(obj: PyObjectRef) -> Self { + let slot = pyre_object::gc_roots::shadow_stack_len(); + pyre_object::gc_roots::pin_root(obj); + Self(slot) + } + + fn get(self) -> PyObjectRef { + pyre_object::gc_roots::shadow_stack_get(self.0) + } +} + /// `is_known_bytes` (interp_sre.py:208-212) — the pattern was compiled from /// a bytes-like object (a `None` pattern is unknown, accepting either). fn pattern_is_known_bytes(pat: PyObjectRef) -> bool { @@ -806,7 +825,9 @@ unsafe fn subject_of(string: PyObjectRef, w_buffer: PyObjectRef) -> Subject { fn slice_subject(subj: Subject, span: (i64, i64), w_default: PyObjectRef) -> PyObjectRef { match subj { Subject::Str(s) => char_slice(s, span.0, span.1) - .map(|s| w_str_from_wtf8(s.to_owned())) + // interp_sre.py:68/76 uses `space.newutf8`: a captured slice is + // an ordinary runtime string and must participate in the GC. + .map(|s| w_str_from_wtf8_managed(s.to_owned())) .unwrap_or(w_default), Subject::Bytes(b) => byte_slice(b, span.0, span.1) .map(pyre_object::bytesobject::w_bytes_from_bytes) @@ -836,7 +857,12 @@ fn subject_span_bytes(subj: Subject, span: (i64, i64)) -> Option<&'static [u8]> /// the (valid UTF-8) builder, or `bytes` (subx result, interp_sre.py:541-548). fn finish_output(subj: Subject, out: Vec) -> PyObjectRef { match subj { - Subject::Str(_) => w_str_from_wtf8(Wtf8Buf::from_bytes(out).unwrap_or_default()), + // interp_sre.py:567 returns `space.newutf8(...)`: substitution and + // expansion results are ordinary runtime strings, not bootstrap + // structural strings that may live outside the managed heap. + Subject::Str(_) => { + w_str_from_wtf8_managed(Wtf8Buf::from_bytes(out).unwrap_or_default()) + } Subject::Bytes(_) => pyre_object::bytesobject::w_bytes_from_bytes(&out), } } @@ -1138,6 +1164,7 @@ fn required_arg_kw( /// with two or more a tuple of the groups. Unmatched groups become the /// empty string (`w_emptystr`, :344-347). fn sre_pattern_findall(args: &[PyObjectRef]) -> Result { + let _roots = pyre_object::gc_roots::push_roots(); let (args, kwargs) = crate::builtins::split_builtin_kwargs(args); let pat = args .first() @@ -1158,22 +1185,26 @@ fn sre_pattern_findall(args: &[PyObjectRef]) -> Result collect_matches(s, pos, endpos, code, pat), Subject::Bytes(b) => collect_matches(b, pos, endpos, code, pat), }; - let mut results = Vec::with_capacity(matches.len()); + // `matchlist_w = []` in interp_sre.py:341 is a GC-managed list. Build + // that same shape here instead of retaining every result in an off-heap + // Rust Vec (which would require an O(number of matches) shadow stack). + let results = RootedObject::pin(w_list_new_empty()); for snap in &matches { + let _item_roots = pyre_object::gc_roots::push_roots(); let spans = &snap.spans; let w_item = if num_groups == 0 { - slice_subject(subj, spans[0], w_empty) + RootedObject::pin(slice_subject(subj, spans[0], w_empty)) } else if num_groups == 1 { - slice_subject(subj, spans[1], w_empty) + RootedObject::pin(slice_subject(subj, spans[1], w_empty)) } else { - let grps: Vec = (1..=num_groups) - .map(|g| slice_subject(subj, spans[g], w_empty)) + let grps: Vec = (1..=num_groups) + .map(|g| RootedObject::pin(slice_subject(subj, spans[g], w_empty))) .collect(); - w_tuple_new(grps) + RootedObject::pin(w_tuple_new(grps.into_iter().map(RootedObject::get).collect())) }; - results.push(w_item); + unsafe { w_list_append(results.get(), w_item.get()) }; } - Ok(w_list_new(results)) + Ok(results.get()) } /// `finditer_w` (interp_sre.py:368-376) — returns the lazy @@ -1220,8 +1251,11 @@ fn sre_pattern_sub(args: &[PyObjectRef]) -> Result /// `subn_w` (interp_sre.py:415-419) — returns `(new_string, count)`. fn sre_pattern_subn(args: &[PyObjectRef]) -> Result { + let _roots = pyre_object::gc_roots::push_roots(); let (w_item, n) = subx(args)?; - Ok(w_tuple_new(vec![w_item, w_int_new(n)])) + let w_item = RootedObject::pin(w_item); + let w_n = RootedObject::pin(w_int_new(n)); + Ok(w_tuple_new(vec![w_item.get(), w_n.get()])) } /// `subx` (interp_sre.py:421-558) — the shared sub/subn body. `repl` is a @@ -1366,6 +1400,7 @@ fn is_exact_str_or_bytes(w: PyObjectRef) -> bool { /// (0 = unlimited) caps the number of splits; the unsplit remainder is the /// final item. fn sre_pattern_split(args: &[PyObjectRef]) -> Result { + let _roots = pyre_object::gc_roots::push_roots(); let (args, kwargs) = crate::builtins::split_builtin_kwargs(args); let pat = args .first() @@ -1379,19 +1414,26 @@ fn sre_pattern_split(args: &[PyObjectRef]) -> Result = Vec::new(); + // `splitlist = []` in interp_sre.py:383 is itself the long-lived root; + // keep only the item currently being appended on the shadow stack. + let results = RootedObject::pin(w_list_new_empty()); let mut last = 0i64; + let append_slice = |span, w_default| { + let _item_roots = pyre_object::gc_roots::push_roots(); + let w_item = RootedObject::pin(slice_subject(subj, span, w_default)); + unsafe { w_list_append(results.get(), w_item.get()) }; + }; // interp_sre.py:387 `while not maxsplit or n < maxsplit` — 0 is unlimited, // a negative cap performs no splits; matching streams so `maxsplit` bounds // the scan rather than discarding already-found matches. let on_match = |snap: &MatchSnapshot| -> Result<(), crate::PyError> { let (mstart, mend) = snap.spans[0]; // interp_sre.py:393 — the slice preceding this match. - results.push(slice_subject(subj, (last, mstart), w_empty)); + append_slice((last, mstart), w_empty); // interp_sre.py:396-399 — interleave each group's capture; an // unmatched group span `(-1, -1)` becomes None via slice_subject. for g in 1..=num_groups { - results.push(slice_subject(subj, snap.spans[g], w_none())); + append_slice(snap.spans[g], w_none()); } last = mend; Ok(()) @@ -1401,8 +1443,8 @@ fn sre_pattern_split(args: &[PyObjectRef]) -> Result stream_matches(b, 0, endpos, code, pat, maxsplit, on_match)?, }; // interp_sre.py:405 — the trailing remainder after the last match. - results.push(slice_subject(subj, (last, endpos as i64), w_empty)); - Ok(w_list_new(results)) + append_slice((last, endpos as i64), w_empty); + Ok(results.get()) } // ── Replacement-template parser (`re._parser.parse_template`) ────────── @@ -1564,52 +1606,86 @@ fn sre_match_group(args: &[PyObjectRef]) -> Result let span = do_span(m, group_args.first().copied())?; return Ok(unsafe { slice_w(m, span, w_none()) }); } - let mut results = Vec::with_capacity(group_args.len()); + let _roots = pyre_object::gc_roots::push_roots(); + let m = RootedObject::pin(m as PyObjectRef); + let mut results: Vec = Vec::with_capacity(group_args.len()); for &w_arg in group_args { - let span = do_span(m, Some(w_arg))?; - results.push(unsafe { slice_w(m, span, w_none()) }); + let span = do_span(m.get() as *const W_SRE_Match, Some(w_arg))?; + results.push(RootedObject::pin(unsafe { + slice_w(m.get() as *const W_SRE_Match, span, w_none()) + })); } - Ok(w_tuple_new(results)) + Ok(w_tuple_new( + results.into_iter().map(RootedObject::get).collect(), + )) } /// `groups_w` (interp_sre.py:728-732) — pyre reads the flattened span /// table directly; unmatched groups (span `(-1, -1)`) take the optional /// `default` argument. fn sre_match_groups(args: &[PyObjectRef]) -> Result { - let m = sre_match_self(args)?; - let w_default = args.get(1).copied().unwrap_or_else(w_none); - let n = unsafe { (*m).spans_len }; - let mut groups = Vec::new(); + let _roots = pyre_object::gc_roots::push_roots(); + let m = RootedObject::pin(sre_match_self(args)? as PyObjectRef); + let w_default = RootedObject::pin(args.get(1).copied().unwrap_or_else(w_none)); + let n = unsafe { (*(m.get() as *const W_SRE_Match)).spans_len }; + let mut groups: Vec = Vec::new(); for gi in 1..n { - let span = unsafe { w_sre_match_get_span(m as PyObjectRef, gi) }.unwrap_or((-1, -1)); - groups.push(unsafe { slice_w(m, span, w_default) }); + let span = unsafe { w_sre_match_get_span(m.get(), gi) }.unwrap_or((-1, -1)); + groups.push(RootedObject::pin(unsafe { + slice_w( + m.get() as *const W_SRE_Match, + span, + w_default.get(), + ) + })); } - Ok(w_tuple_new(groups)) + Ok(w_tuple_new( + groups.into_iter().map(RootedObject::get).collect(), + )) } /// `groupdict_w` (interp_sre.py:735-751) — name→group-text map built by /// iterating `srepat.w_groupindex`; unmatched groups take `default`. fn sre_match_groupdict(args: &[PyObjectRef]) -> Result { - let m = sre_match_self(args)?; - let w_default = args.get(1).copied().unwrap_or_else(w_none); - let w_groupindex = unsafe { (*(*m).w_srepat.cast::()).w_groupindex }; - let w_dict = w_dict_new(); + let _roots = pyre_object::gc_roots::push_roots(); + let m = RootedObject::pin(sre_match_self(args)? as PyObjectRef); + let w_default = RootedObject::pin(args.get(1).copied().unwrap_or_else(w_none)); + let w_groupindex = RootedObject::pin(unsafe { + (*(*(m.get() as *const W_SRE_Match)) + .w_srepat + .cast::()) + .w_groupindex + }); + let w_dict = RootedObject::pin(w_dict_new()); // interp_sre.py:735-751 groupdict_w — walk `w_groupindex` through the // object-space iterator / item protocol, resolving each value with // `do_span` so a duck-typed group number works. - let w_iterator = crate::baseobjspace::iter(w_groupindex)?; + let w_iterator = RootedObject::pin(crate::baseobjspace::iter(w_groupindex.get())?); loop { - let w_key = match crate::baseobjspace::next(w_iterator) { - Ok(k) => k, + let _item_roots = pyre_object::gc_roots::push_roots(); + let w_key = match crate::baseobjspace::next(w_iterator.get()) { + Ok(k) => RootedObject::pin(k), Err(e) if e.kind == crate::PyErrorKind::StopIteration => break, Err(e) => return Err(e), }; - let w_value = crate::baseobjspace::getitem(w_groupindex, w_key)?; - let span = do_span(m, Some(w_value))?; - let w_grp = unsafe { slice_w(m, span, w_default) }; - crate::baseobjspace::setitem(w_dict, w_key, w_grp)?; + let w_value = RootedObject::pin(crate::baseobjspace::getitem( + w_groupindex.get(), + w_key.get(), + )?); + let span = do_span( + m.get() as *const W_SRE_Match, + Some(w_value.get()), + )?; + let w_grp = RootedObject::pin(unsafe { + slice_w( + m.get() as *const W_SRE_Match, + span, + w_default.get(), + ) + }); + crate::baseobjspace::setitem(w_dict.get(), w_key.get(), w_grp.get())?; } - Ok(w_dict) + Ok(w_dict.get()) } /// `fget_regs` (interp_sre.py:853-864) — `((start, end), ...)` for group @@ -1692,12 +1768,17 @@ fn sre_match_expand(args: &[PyObjectRef]) -> Result pub(crate) fn sre_match_repr_str( m: PyObjectRef, ) -> Result { - let mp = m as *const W_SRE_Match; - let span = unsafe { w_sre_match_get_span(m, 0) }.unwrap_or((-1, -1)); + let _roots = pyre_object::gc_roots::push_roots(); + let m = RootedObject::pin(m); + let mp = m.get() as *const W_SRE_Match; + let span = unsafe { w_sre_match_get_span(m.get(), 0) }.unwrap_or((-1, -1)); let (start, end) = span; let subj = unsafe { subject_of((*mp).w_string, (*mp).w_buffer) }; - let w_match_str = slice_subject(subj, span, w_none()); - let matchrepr = truncate_code_points(unsafe { crate::display::py_repr_wtf8(w_match_str) }?, 50); + let w_match_str = RootedObject::pin(slice_subject(subj, span, w_none())); + let matchrepr = truncate_code_points( + unsafe { crate::display::py_repr_wtf8(w_match_str.get()) }?, + 50, + ); Ok(crate::display::wtf8_format!( format!("", type_name, crate::display::repr_addr(addr), @@ -1528,9 +1530,7 @@ pub fn proxy_trunc(args: &[PyObjectRef]) -> Result { pub fn proxy_str(args: &[PyObjectRef]) -> Result { let w_obj0 = force(args[0])?; - Ok(pyre_object::w_str_from_wtf8(unsafe { - crate::display::py_str_wtf8(w_obj0)? - })) + crate::builtins::builtin_str(&[w_obj0]) } pub fn proxy_bool(args: &[PyObjectRef]) -> Result { diff --git a/pyre/pyre-interpreter/src/module/array/mod.rs b/pyre/pyre-interpreter/src/module/array/mod.rs index 0cbd212f195..9c3333af924 100644 --- a/pyre/pyre-interpreter/src/module/array/mod.rs +++ b/pyre/pyre-interpreter/src/module/array/mod.rs @@ -864,7 +864,7 @@ fn array_tounicode_method(args: &[PyObjectRef]) -> PyResult { .ok_or_else(|| PyError::value_error("character out of range"))?; wb.push(point); } - Ok(pyre_object::unicodeobject::w_str_from_wtf8(wb)) + Ok(pyre_object::unicodeobject::w_str_from_wtf8_managed(wb)) } fn array_fromunicode_method(args: &[PyObjectRef]) -> PyResult { @@ -954,7 +954,9 @@ pub fn array_repr_wtf8(obj: PyObjectRef) -> Result PyResult { check_arity(args, 1, "array.__repr__")?; - Ok(pyre_object::w_str_from_wtf8(array_repr_wtf8(args[0])?)) + Ok(pyre_object::w_str_from_wtf8_managed(array_repr_wtf8( + args[0], + )?)) } // `interp_array.py compare_arrays`: compare each element with the requested diff --git a/pyre/pyre-interpreter/src/module/gc/hook.rs b/pyre/pyre-interpreter/src/module/gc/hook.rs index 2d06b126338..47a8bc6e174 100644 --- a/pyre/pyre-interpreter/src/module/gc/hook.rs +++ b/pyre/pyre-interpreter/src/module/gc/hook.rs @@ -413,22 +413,11 @@ fn app_hooks_ptr() -> Option<*mut W_AppLevelHooks> { .then_some(obj as *mut W_AppLevelHooks) } -/// Create the space-owned singleton and bind its three actions to an -/// actionflag. The main ExecutionContext calls this during bootstrap, before -/// worker ECs can import `gc`, so the flag it passes outlives every later EC. -/// -/// Reach, however, is not lifetime, and this is where pyre still departs from -/// PyPy. There `actionflag` belongs to the `space`, so every execution context -/// dispatches the same set; here it is a field on each `ExecutionContext` -/// (`executioncontext.rs` `pub actionflag: ActionFlag`), and an `AsyncAction` -/// binds to exactly one flag when `register_nonperiodic_action` hands it an -/// `_action_index` for that flag's bitmask. `get_or_init` therefore leaves the -/// hooks registered with the bootstrapping EC alone: a worker thread that runs -/// a collection fires a bit its own dispatch loop never reads, so its callback -/// waits for the bootstrapping thread to reach an opcode. Closing this means -/// moving `ActionFlag` onto the space the way upstream has it, not -/// re-registering per EC — the second flag would overwrite the index the first -/// one handed out. +/// Create the space-owned singleton and bind its three actions to the shared +/// `space.actionflag`. The main ExecutionContext calls this during bootstrap; +/// worker ECs carry [`crate::executioncontext::SpaceActionFlag`] references to +/// that same flag, so a collection performed by a worker fires and dispatches +/// the same action indexes there, matching PyPy's process-owned object space. pub fn initialize( space: PyObjectRef, actionflag: &mut (dyn ActionFlagOps + 'static), diff --git a/pyre/pyre-interpreter/src/module/gc/mod.rs b/pyre/pyre-interpreter/src/module/gc/mod.rs index 32a4fc601e2..1a6db9efab5 100644 --- a/pyre/pyre-interpreter/src/module/gc/mod.rs +++ b/pyre/pyre-interpreter/src/module/gc/mod.rs @@ -656,7 +656,6 @@ fn pin_referents(w_obj: PyObjectRef) { /// `referents.py:35-39 wrap`: app-level objects pass through, internal nodes /// become `W_GcRef`. Results are rooted as they are made because constructing /// a later wrapper can initialize a type and allocate. -/// Wrap the raw nodes rooted at `first..last`, leaving the wrappers pinned. /// /// Returns the first shadow-stack slot of the wrapped range, which runs to the /// stack top on return. A slot range rather than a `Vec` because @@ -705,11 +704,7 @@ fn list_from_roots(first: usize) -> PyObjectRef { } fn user_del_action() -> Option<&'static mut crate::executioncontext::UserDelAction> { - let ec = crate::call::getexecutioncontext() as *mut crate::PyExecutionContext; - if ec.is_null() { - return None; - } - let action = unsafe { (*ec).user_del_action }; + let action = crate::executioncontext::space_user_del_action(); if action.is_null() { None } else { @@ -982,7 +977,21 @@ fn gc_call_method( } } +#[cfg(feature = "sandbox")] +fn heap_dump_write_via_host(fd: i32, bytes: &[u8]) -> Result { + crate::host_seam::ops::write(fd, bytes) + .map(|written| written as isize) + // A non-OS seam failure still needs an errno. Use the collector's code + // for targets and failure modes that cannot supply one. + .map_err(|error| match error { + crate::host_seam::SeamError::Os(errno) => errno, + _ => majit_gc::HEAP_DUMP_EIO, + }) +} + fn dump_rpy_heap_fd(fd: i32) -> Result<(), crate::PyError> { + #[cfg(feature = "sandbox")] + majit_gc::set_heap_dump_write(Some(heap_dump_write_via_host)); match majit_gc::dump_rpy_heap(fd) { Ok(true) => Ok(()), Ok(false) => Err(crate::PyError::not_implemented( diff --git a/pyre/pyre-interpreter/src/module/mmap/interp_mmap.rs b/pyre/pyre-interpreter/src/module/mmap/interp_mmap.rs index bbb6acfed51..aeeb1244e60 100644 --- a/pyre/pyre-interpreter/src/module/mmap/interp_mmap.rs +++ b/pyre/pyre-interpreter/src/module/mmap/interp_mmap.rs @@ -1168,7 +1168,7 @@ fn init_mmap_type(ns: pyre_object::PyObjectRef) { // access / length / pos / offset. Capitalised True / // False matches CPython's bool repr. if mmap_get_attr_i64(obj, "_ptr") == 0 { - return Ok(pyre_object::w_str_new("")); + return Ok(pyre_object::w_str_new_managed("")); } let len = mmap_get_attr_i64(obj, "_len"); let pos = mmap_get_attr_i64(obj, "_pos"); @@ -1180,7 +1180,9 @@ fn init_mmap_type(ns: pyre_object::PyObjectRef) { x if x == MMAP_ACCESS_COPY => "ACCESS_COPY", _ => "ACCESS_DEFAULT", }; - Ok(pyre_object::w_str_new(&format!( + // W_MMap.descr_repr returns `space.newtext(...)` for both + // live and closed mappings. + Ok(pyre_object::w_str_new_managed(&format!( "" ))) }, diff --git a/pyre/pyre-interpreter/src/module/posix/interp_posix.rs b/pyre/pyre-interpreter/src/module/posix/interp_posix.rs index bb68ab9c5c1..77821cc4380 100644 --- a/pyre/pyre-interpreter/src/module/posix/interp_posix.rs +++ b/pyre/pyre-interpreter/src/module/posix/interp_posix.rs @@ -4522,7 +4522,9 @@ pub fn register_module(ns: pyre_object::PyObjectRef) { out.push_str(""); - Ok(pyre_object::w_str_from_wtf8(out)) + // interp_scandir.py:230 returns `space.newtext(...)`: this rendered + // value is an ordinary collectable runtime string. + Ok(pyre_object::w_str_from_wtf8_managed(out)) } /// `interp_scandir.py:463-465 descr_reduce_ex` — an entry names a live /// position in a directory listing, so it refuses to be pickled. `%T` is diff --git a/pyre/pyre-interpreter/src/module/signal/interp_signal.rs b/pyre/pyre-interpreter/src/module/signal/interp_signal.rs index 6e8c885d427..02832edb03f 100644 --- a/pyre/pyre-interpreter/src/module/signal/interp_signal.rs +++ b/pyre/pyre-interpreter/src/module/signal/interp_signal.rs @@ -451,7 +451,7 @@ pub fn install_signal_handling(ec: &mut ExecutionContext) { // into it for the whole run), so it is leaked deliberately. let action: &'static mut CheckSignalAction = Box::leak(CheckSignalAction::new(ec.space)); let async_ptr: *mut dyn AsyncActionOps = &mut *action; - action.register_periodic_action(&mut ec.actionflag, false); + action.register_periodic_action(ec.actionflag.shared_mut(), false); ec.check_signal_action = Some(async_ptr); // Hand the ticker cell address to the OS handler so it can force the diff --git a/pyre/pyre-interpreter/src/module/sys/vm.rs b/pyre/pyre-interpreter/src/module/sys/vm.rs index 281e032d08b..271faefec11 100644 --- a/pyre/pyre-interpreter/src/module/sys/vm.rs +++ b/pyre/pyre-interpreter/src/module/sys/vm.rs @@ -375,7 +375,7 @@ fn simple_namespace_repr(args: &[PyObjectRef]) -> crate::PyResult { unsafe { w_type_get_name(actual_type) }.to_string() }; let Some(_guard) = crate::display::ReprGuard::enter(self_obj) else { - return Ok(w_str_new(&format!("{name}(...)"))); + return Ok(w_str_new_managed(&format!("{name}(...)"))); }; let dict = crate::baseobjspace::getattr_str( pyre_object::gc_roots::shadow_stack_get(sp), @@ -427,7 +427,7 @@ fn simple_namespace_repr(args: &[PyObjectRef]) -> crate::PyResult { text.push_wtf8(part); } text.push_str(")"); - Ok(pyre_object::w_str_from_wtf8(text)) + Ok(pyre_object::w_str_from_wtf8_managed(text)) } /// `_structseq.py:185 SimpleNamespace.__eq__` — structural over `__dict__` diff --git a/pyre/pyre-interpreter/src/module/thread/gil.rs b/pyre/pyre-interpreter/src/module/thread/gil.rs index 0591b5d5c28..02c7b7fdf1b 100644 --- a/pyre/pyre-interpreter/src/module/thread/gil.rs +++ b/pyre/pyre-interpreter/src/module/thread/gil.rs @@ -7,6 +7,7 @@ //! `GILReleaseAction` is that hand-off — an action registered on the ticker //! which yields the GIL every `sys.getcheckinterval()` bytecodes. +use std::sync::OnceLock; use std::sync::atomic::{AtomicBool, Ordering}; use crate::executioncontext::{ @@ -52,41 +53,36 @@ impl AsyncActionOps for GilReleaseAction { impl PeriodicAsyncActionOps for GilReleaseAction {} +static GIL_RELEASE_ACTION: OnceLock = OnceLock::new(); + /// gil.py:20-23 `GILThreadLocals.initialize` — "add the GIL-releasing callback /// as an action on the space". /// /// `use_bytecode_counter=True` is what puts it at the end of the periodic list /// (executioncontext.py:503-504: "hack to put the release-the-GIL one at the /// end of the list"), behind the signal check. Idempotent; the actionflag -/// holds the action's heap address, so ownership stays with the execution -/// context until [`shutdown`] gives it back. +/// holds the action's heap address, so the process-owned flag retains it for +/// the runtime lifetime. pub fn initialize(ec: &mut ExecutionContext) { - if ec.gil_release_action.is_some() { - return; - } - let action: &'static mut GilReleaseAction = Box::leak(GilReleaseAction::new(ec.space)); - let async_ptr: *mut dyn AsyncActionOps = &mut *action; - action.register_periodic_action(&mut ec.actionflag, true); - ec.gil_release_action = Some(async_ptr); + GIL_RELEASE_ACTION.get_or_init(|| { + let action: &'static mut GilReleaseAction = Box::leak(GilReleaseAction::new(ec.space)); + action.register_periodic_action(ec.actionflag.shared_mut(), true); + action as *mut GilReleaseAction as usize + }); } -/// Reclaim what [`initialize`] registered. -/// -/// Upstream has nothing to reclaim: one `GILReleaseAction` is registered on -/// `space.actionflag` for the process. pyre gives each execution context its -/// own ticker, so each one also allocates its own action, and a thread which -/// leaves without giving it back leaks it. -/// -/// The caller's execution context, and with it the actionflag that still names -/// this action, is dropped immediately afterwards without running any Python -/// in between. -pub fn shutdown(ec: &mut ExecutionContext) { - let Some(action) = ec.gil_release_action.take() else { +/// Trace the process-owned periodic action's object-space reference. The +/// action is a translated object-space child in PyPy; pyre's leaked Rust box +/// needs the corresponding explicit non-stack root. +pub(super) fn walk_action_roots(visitor: &mut dyn FnMut(&mut majit_ir::GcRef)) { + let Some(&addr) = GIL_RELEASE_ACTION.get() else { return; }; - // SAFETY: the pointer is the one `initialize` leaked out of a `Box`, and - // this is the only place that takes it back. - drop(unsafe { Box::from_raw(action) }); + let action = unsafe { &mut *(addr as *mut GilReleaseAction) }; + let slot = &mut action.base.base.space; + if !slot.is_null() { + visitor(unsafe { &mut *(slot as *mut PyObjectRef as *mut majit_ir::GcRef) }); + } } /// gil.py:17-18 `GILThreadLocals.gil_ready`, quasi-immutable and "changed (to @@ -111,8 +107,6 @@ pub fn setup_threads(ec: &mut ExecutionContext) -> bool { majit_gc::rgil::allocate(); GIL_READY.store(true, Ordering::Release); } - // Registering the action is per-execution-context, unlike upstream's - // once-per-space `initialize`, because the ticker it hangs on is too. initialize(ec); first } diff --git a/pyre/pyre-interpreter/src/module/thread/mod.rs b/pyre/pyre-interpreter/src/module/thread/mod.rs index 0b662e76580..72f3a88caf8 100644 --- a/pyre/pyre-interpreter/src/module/thread/mod.rs +++ b/pyre/pyre-interpreter/src/module/thread/mod.rs @@ -197,15 +197,6 @@ pub(crate) fn walk_thread_roots(visitor: &mut dyn FnMut(&mut majit_ir::GcRef)) { forward(&mut frame); ec.topframeref = frame as *mut crate::PyFrame; } - if !ec.user_del_action.is_null() { - let action = unsafe { &mut *ec.user_del_action }; - forward(&mut action.base.space); - if let Some(pending) = action.pending_with_disabled_del.as_mut() { - for obj in pending { - forward(obj); - } - } - } // `builtins_module` / `builtin_dict_cache` / `thread_local_refs`. // `clone_for_thread` copies the builtins reference into the child // EC, so each copy is its own slot: forwarding only the parent's @@ -214,6 +205,7 @@ pub(crate) fn walk_thread_roots(visitor: &mut dyn FnMut(&mut majit_ir::GcRef)) { ec.walk_builtin_roots(visitor); } } + gil::walk_action_roots(visitor); let mut forwarded = Vec::new(); for handle in SHUTDOWN_HANDLES.lock().iter_mut() { let old = *handle; @@ -1408,9 +1400,6 @@ fn thread_is_stopping(ec: &mut crate::PyExecutionContext) { local.thread_is_stopping(ident); } } - // Last: nothing above runs bytecode, so nothing can reach the ticker this - // action is registered on after it is gone. - gil::shutdown(ec); } /// The calling thread's identity. @@ -1639,13 +1628,12 @@ fn spawn_thread( }); let ec_ptr = &*ec as *const crate::PyExecutionContext; crate::call::set_last_exec_ctx(ec_ptr); - // `install_user_del_action` can allocate. Publish the fresh EC - // first, matching OSThreadLocals.enter_thread() installing the - // ExecutionContext before thread bootstrap invokes Python code. + // Publish the fresh EC first, matching + // OSThreadLocals.enter_thread() installing the ExecutionContext + // before thread bootstrap invokes Python code. ec.install_user_del_action(); - // Each mutator owns its ticker, so the GIL-releasing action has to - // be registered on this thread's own actionflag; without it a - // worker would hold the GIL until its next external call. + // The space-owned GIL action is already registered on the shared + // actionflag; this is an idempotent bootstrap guard. gil::initialize(&mut ec); let ident = current_ident(); if has_handle { diff --git a/pyre/pyre-interpreter/src/module/time/interp_time.rs b/pyre/pyre-interpreter/src/module/time/interp_time.rs index 71fe9ce2f79..13b9a1b7263 100644 --- a/pyre/pyre-interpreter/src/module/time/interp_time.rs +++ b/pyre/pyre-interpreter/src/module/time/interp_time.rs @@ -1376,7 +1376,9 @@ pub fn strftime(args: &[PyObjectRef]) -> Result { // is not valid WTF-8; fall back to surrogateescape rather than raising, // mirroring str_decode_locale_surrogateescape. let result = match rustpython_wtf8::Wtf8Buf::from_bytes(rendered) { - Ok(wtf8) => pyre_object::w_str_from_wtf8(wtf8), + // interp_time.py:1188 returns `space.newutf8(decoded, size)`: + // strftime's formatted value is an ordinary runtime string. + Ok(wtf8) => pyre_object::w_str_from_wtf8_managed(wtf8), Err(bytes) => crate::typedef::charp2uni(&bytes), }; Ok(result) @@ -1411,7 +1413,7 @@ pub fn strftime(args: &[PyObjectRef]) -> Result { // surrogateescape rather than raising. let rendered = buf[..n].to_vec(); return Ok(match rustpython_wtf8::Wtf8Buf::from_bytes(rendered) { - Ok(wtf8) => w_str_from_wtf8(wtf8), + Ok(wtf8) => w_str_from_wtf8_managed(wtf8), Err(bytes) => crate::typedef::charp2uni(&bytes), }); } @@ -1485,7 +1487,9 @@ fn _asctime_from_tm(tm: &c_tm) -> Result { const MON_NAME: [&str; 12] = [ "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec", ]; - Ok(w_str_new(&format!( + // interp_time.py:_asctime returns the result of ordinary `%` formatting; + // the rendered value is a collectable runtime string. + Ok(w_str_new_managed(&format!( "{} {}{:>3} {:02}:{:02}:{:02} {}", WDAY_NAME[tm.tm_wday as usize], MON_NAME[tm.tm_mon as usize], @@ -1508,25 +1512,13 @@ pub fn ctime(args: &[PyObjectRef]) -> Result { "time.ctime is unavailable on wasm32", )) } - #[cfg(unix)] + #[cfg(not(target_arch = "wasm32"))] { + // interp_time.py:ctime always delegates through localtime + _asctime. + // Keep one result-allocation path on Unix and Windows alike. let tm = _c_localtime(seconds)?; _asctime_from_tm(&tm) } - #[cfg(windows)] - { - unsafe extern "C" { - fn _ctime64(time: *const i64) -> *const libc::c_char; - } - let t = seconds; - let p = unsafe { _ctime64(&t) }; - if p.is_null() { - return Err(crate::PyError::value_error("unconvertible time")); - } - let lossy = unsafe { std::ffi::CStr::from_ptr(p) }.to_string_lossy(); - let s = lossy.trim_end_matches('\n'); - Ok(w_str_new(s)) - } } /// `app_time.py:26-34 strptime` — parse `string` per `format`, delegating to diff --git a/pyre/pyre-interpreter/src/module/unicodedata/mod.rs b/pyre/pyre-interpreter/src/module/unicodedata/mod.rs index 76dcfc8c639..36590c3d2a9 100644 --- a/pyre/pyre-interpreter/src/module/unicodedata/mod.rs +++ b/pyre/pyre-interpreter/src/module/unicodedata/mod.rs @@ -271,7 +271,17 @@ fn normalize(args: &[PyObjectRef]) -> PyResult { } let form = normalize_form("normalize", args[0])?; let text = normalize_text("normalize", args[1])?; - Ok(w_str_from_wtf8(ucd_core::normalize(form, text))) + // interp_ucd.py:175-178 returns `space.newutf8(content, strlen)` for + // ASCII. PyPy's W_UnicodeObject.is_w treats wrappers sharing that UTF-8 + // buffer as identical; returning the exact base str preserves that O(1) + // identity without copying Pyre's owned Wtf8Buf. A subclass must still + // be converted to a base str, just like `space.newutf8`. + if text.as_bytes().is_ascii() + && unsafe { pyre_object::is_exact_type(args[1], &pyre_object::STR_TYPE) } + { + return Ok(args[1]); + } + Ok(w_str_from_wtf8_managed(ucd_core::normalize(form, text))) } fn is_normalized(args: &[PyObjectRef]) -> PyResult { diff --git a/pyre/pyre-interpreter/src/typedef.rs b/pyre/pyre-interpreter/src/typedef.rs index 2cbc4c38e41..0bfea871a89 100644 --- a/pyre/pyre-interpreter/src/typedef.rs +++ b/pyre/pyre-interpreter/src/typedef.rs @@ -4006,7 +4006,7 @@ fn super_descr_repr(args: &[PyObjectRef]) -> Result pyre_object::w_type_get_name(bound_type) }) }; - Ok(w_str_new(&format!( + Ok(pyre_object::w_str_new_managed(&format!( ", {}>", start_name, bound_name ))) @@ -9626,7 +9626,7 @@ fn union_repr_method(args: &[PyObjectRef]) -> crate::PyResult { "descriptor '__repr__' requires a 'types.UnionType' object", )); } - Ok(pyre_object::w_str_from_wtf8(unsafe { + Ok(pyre_object::w_str_from_wtf8_managed(unsafe { crate::display::py_repr_wtf8(self_)? })) } @@ -10376,7 +10376,7 @@ fn init_getset_descriptor_type(ns: PyObjectRef) { "descriptor '__repr__' requires a 'getset_descriptor' object but received a '{received}'" ))); } - Ok(pyre_object::w_str_new(&unsafe { + Ok(pyre_object::w_str_new_managed(&unsafe { getset_descriptor_repr(descr) })) }, @@ -10990,7 +10990,7 @@ fn init_type_type(ns: PyObjectRef) { "", crate::baseobjspace::type_repr_qualified_name(obj) ); - Ok(pyre_object::w_str_new(&rendered)) + Ok(pyre_object::w_str_new_managed(&rendered)) }, 1, ), @@ -13475,7 +13475,7 @@ fn init_function_type(ns: PyObjectRef) { " at {}>", crate::display::repr_addr(function as usize) )); - Ok(pyre_object::w_str_from_wtf8(repr)) + Ok(pyre_object::w_str_from_wtf8_managed(repr)) }, 1, ), @@ -13893,7 +13893,7 @@ fn init_builtin_function_type(ns: PyObjectRef) { } else { unsafe { crate::function::function_get_self_or_none(carrier) } }; - Ok(pyre_object::w_str_new(&unsafe { + Ok(pyre_object::w_str_new_managed(&unsafe { crate::function::builtin_function_repr_text(name, w_self) })) }, @@ -14669,7 +14669,7 @@ fn init_method_wrapper_type(ns: PyObjectRef) { let type_name = crate::typedef::r#type(w_self) .map(|tp| unsafe { pyre_object::w_type_get_name(tp.as_ptr()) }) .unwrap_or("object"); - Ok(pyre_object::w_str_new(&format!( + Ok(pyre_object::w_str_new_managed(&format!( "", crate::display::repr_addr(w_self as usize) ))) @@ -14854,7 +14854,7 @@ fn init_method_descriptor_type(ns: PyObjectRef) { let name = crate::function::function_get_name(descr); let owner = crate::function::fget_func_objclass(descr)?; let owner_name = pyre_object::w_type_get_name(owner); - Ok(pyre_object::w_str_new(&format!( + Ok(pyre_object::w_str_new_managed(&format!( "" ))) }, @@ -15014,7 +15014,7 @@ fn init_classmethod_descriptor_type(ns: PyObjectRef) { let name = crate::function::function_get_name(function); let owner = crate::function::fget_func_objclass(function)?; let owner_name = pyre_object::w_type_get_name(owner); - Ok(pyre_object::w_str_new(&format!( + Ok(pyre_object::w_str_new_managed(&format!( "" ))) }, @@ -15646,7 +15646,7 @@ fn init_member_descriptor_type(ns: PyObjectRef) { "descriptor '__repr__' requires a 'member_descriptor' object", )); } - Ok(pyre_object::w_str_new(&unsafe { + Ok(pyre_object::w_str_new_managed(&unsafe { member_descriptor_repr(member) })) }, @@ -15817,7 +15817,7 @@ fn cell_descr_repr(args: &[PyObjectRef]) -> crate::PyResult { crate::display::repr_addr(value as usize) ) }; - Ok(w_str_new(&text)) + Ok(w_str_new_managed(&text)) } /// `nestedscope.py:934-952 Cell.typedef`, in source order. CPython 3.14 is @@ -26665,7 +26665,7 @@ fn generator_frame(obj: PyObjectRef) -> *mut crate::pyframe::PyFrame { fn generator_descr_repr(args: &[PyObjectRef]) -> crate::PyResult { let name = generator_name_value(args[0], true)?; - Ok(w_str_new(&format!( + Ok(w_str_new_managed(&format!( "", unsafe { pyre_object::w_str_get_value(name) }, crate::display::repr_addr(args[0] as usize) @@ -26674,7 +26674,7 @@ fn generator_descr_repr(args: &[PyObjectRef]) -> crate::PyResult { fn coroutine_descr_repr(args: &[PyObjectRef]) -> crate::PyResult { let name = generator_name_value(args[0], true)?; - Ok(w_str_new(&format!( + Ok(w_str_new_managed(&format!( "", unsafe { pyre_object::w_str_get_value(name) }, crate::display::repr_addr(args[0] as usize)