gc: fix collection fallbacks, shared actions, and heap dumps

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,
     })
 }

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,
     })
 }

majit/majit-gc/src/collector.rs

@@ -60,9 +60,9 @@ struct HeapDumpWriter {
     buffer: Vec<isize>,
 }
 
-// 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::<isize>();
-        // 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::<libc::c_void>(),
-                    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::<libc::c_void>(),
-                    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::<u8>(), 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::<libc::c_void>(),
+                        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::<libc::c_void>(),
+                        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(())
     }
 }
 

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,17 +2462,30 @@ pub fn is_app_level_object(obj: GcRef) -> bool {
 }
 
 pub type DumpRpyHeapFn = fn(i32) -> Result<bool, i32>;
+pub type HeapDumpWriteFn = fn(i32, &[u8]) -> Result<isize, i32>;
 pub type GetTypeidsTextFn = fn() -> Option<Vec<u8>>;
 pub type GetTypeidsListFn = fn() -> Option<Vec<usize>>;
 
 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);
 
 pub fn set_active_dump_rpy_heap(hook: Option<DumpRpyHeapFn>) {
     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<HeapDumpWriteFn>) {
+    HEAP_DUMP_WRITE.set(hook);
+}
+
+pub(crate) fn try_heap_dump_write(fd: i32, bytes: &[u8]) -> Option<Result<isize, i32>> {
+    HEAP_DUMP_WRITE.get().map(|write| write(fd, bytes))
+}
+
 pub fn set_active_get_typeids_text(hook: Option<GetTypeidsTextFn>) {
     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())
 }
 

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

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

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

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

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

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

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

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