gc: root objects across collection points;

majit/majit-ir/src/eval_breaker_word.rs

@@ -1,8 +1,9 @@
 //! The single process-global eval-breaker word polled by JIT-compiled loop
 //! back-edges. One `AtomicUsize` whose bits fold the two former back-edge
 //! polls into one load + one nonzero branch:
-//!   bit0 EB_ASYNC — mirrors a negative async ticker (`ActionFlag._ticker < 0`);
-//!                   OR'd in by the OS signal handler and the action dispatcher.
+//!   bit0 EB_ASYNC — an async ticker request; OR'd in by the OS signal handler
+//!                   and action dispatcher, then copied into
+//!                   `ActionFlag._ticker` at a safe interpreter checkpoint.
 //!   bit1 EB_STW   — mirrors `GC_SYNC.stw_requested`; OR'd in by the collector
 //!                   while it drains mutators to safepoints.
 //!   bit2 EB_FINALIZING — mirrors interpreter finalization; once armed,

pyre/extra_tests/snippets/stdlib_re.py

@@ -62,6 +62,15 @@
 assert re.compile("(a)(bc)").match("abc")[1] == "a"
 assert re.compile("a(b)(?P<a>c)d").match("abcd").groupdict() == {"a": "c"}
 
+# Keep later dynamically-created selectors alive while each earlier group
+# slice allocates.  Repetition crosses the moving nursery boundary, exercising
+# the gateway argument-rooting path rather than only immortal integer indices.
+named_match = re.compile("(?P<left>a)(?P<right>bc)").match("abc")
+for i in range(4096):
+    left = ("left" + str(i))[:4]
+    right = ("right" + str(i))[:5]
+    assert named_match.group(left, right) == ("a", "bc")
+
 # test op branch
 assert re.compile(r"((?=\d|\.\d)(?P<int>\d*)|a)").match("123.2132").group() == "123"
 

pyre/extra_tests/snippets/stdlib_signal.py

@@ -22,7 +22,7 @@ def handler(signum, frame):
 
 
 # unix
-if "win" not in sys.platform:
+if not sys.platform.startswith("win"):
     signal.signal(signal.SIGALRM, handler)
     assert signal.getsignal(signal.SIGALRM) is handler
 

pyre/pyre-interpreter/src/baseobjspace.rs

@@ -16261,18 +16261,44 @@ pub fn next(obj: PyObjectRef) -> PyResult {
         //         self.w_prev = w_next
         //         return space.newtuple2(w_prev, w_next)
         if pyre_object::interp_itertools::is_pairwise(obj) {
-            let it = &mut *(obj as *mut pyre_object::interp_itertools::W_Pairwise);
-            let mut w_prev = it.w_prev;
-            if w_prev.is_null() {
-                w_prev = next(it.w_iterator)?;
+            use pyre_object::interp_itertools as pairwise;
+
+            // `space.next` may allocate and move either yielded item. RPython's
+            // shadow-stack transform keeps `self` and `w_prev` live across the
+            // second call and reloads both before the final stores/newtuple2.
+            // Keep the same livevar set explicitly; in particular, the
+            // barriered `self.w_prev` field is forwarded by a minor collection
+            // but a raw Rust local copied before that collection is not.
+            // The bracket claims its whole livevar set up front, so a slot's
+            // index does not depend on which arm ran. `w_prev` is PY_NULL until
+            // the first result and `w_next` has no value yet, and reserving a
+            // slot for each is what lets the reloads below be plain `get`s; a
+            // null slot is what the root walkers already read as "no root".
+            const SELF: usize = 0;
+            const ITERATOR: usize = 1;
+            const PREV: usize = 2;
+            const NEXT: usize = 3;
+
+            let roots = pyre_object::gc_roots::push_roots();
+            let base = roots.base();
+            roots.pin_root(obj);
+            roots.pin_root(pairwise::w_pairwise_get_iterator(roots.get(base + SELF)));
+            roots.pin_root(pairwise::w_pairwise_get_prev(roots.get(base + SELF)));
+            roots.pin_root(std::ptr::null_mut());
+
+            if roots.get(base + PREV).is_null() {
+                let w_prev = next(roots.get(base + ITERATOR))?;
+                roots.set(base + PREV, w_prev);
                 // set before fetching w_next to handle reentrancy
-                it.w_prev = w_prev;
-                pyre_object::gc_hook::try_gc_write_barrier(obj as *mut u8);
+                pairwise::w_pairwise_set_prev(roots.get(base + SELF), roots.get(base + PREV));
             }
-            let w_next = next(it.w_iterator)?;
-            it.w_prev = w_next;
-            pyre_object::gc_hook::try_gc_write_barrier(obj as *mut u8);
-            return Ok(pyre_object::w_tuple_new(vec![w_prev, w_next]));
+            let w_next = next(roots.get(base + ITERATOR))?;
+            roots.set(base + NEXT, w_next);
+            pairwise::w_pairwise_set_prev(roots.get(base + SELF), roots.get(base + NEXT));
+            return Ok(pyre_object::w_tuple_new(vec![
+                roots.get(base + PREV),
+                roots.get(base + NEXT),
+            ]));
         }
         // itertools.cycle — interp_itertools.py W_Cycle.next_w
         //

pyre/pyre-interpreter/src/eval.rs

@@ -1186,6 +1186,8 @@ fn walk_interpreter_global_roots(visitor: &mut dyn FnMut(&mut majit_ir::GcRef))
         crate::cpyext::walk_gc_roots(&mut forward);
     }
     crate::executioncontext::walk_space_user_del_action_roots(visitor);
+    #[cfg(not(target_arch = "wasm32"))]
+    crate::module::signal::interp_signal::walk_check_signal_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")))]

pyre/pyre-interpreter/src/executioncontext.rs

@@ -341,8 +341,8 @@ pub struct ExecutionContext {
     /// `pypy/interpreter/baseobjspace.py` `space.check_signal_action` —
     /// the `CheckSignalAction` registered when the `signal` module loads.
     /// `checksignals` calls its `perform` directly.  Stored as a trait
-    /// pointer into the leaked action owned by `module::_signal`
-    /// (`install_signal_handling`); `None` until installed.
+    /// pointer into the process object-space singleton owned by
+    /// `module::_signal` (`install_signal_handling`); `None` until installed.
     pub check_signal_action: Option<*mut dyn AsyncActionOps>,
     /// `executioncontext.py sys_exc_operror` — the active exception for
     /// `sys.exc_info()` / bare `raise`, saved/restored across handler
@@ -764,6 +764,10 @@ impl ExecutionContext {
                 let w_exc = crate::builtins::exc_exception_new(&[w_async_exception_type])?;
                 return Err(unsafe { crate::PyError::from_exc_object(w_exc) });
             }
+            // The OS signal handler may only touch atomics.  Copy its breaker
+            // request into the ordinary ActionFlag ticker here, under the GIL,
+            // before the upstream decrement-and-dispatch sequence below.
+            self.actionflag.sync_async_ticker();
         }
         // executioncontext.py:158-165 bytecode_trace:
         //   def bytecode_trace(self, frame, decr_by=TICK_COUNTER_STEP):
@@ -954,6 +958,9 @@ impl ExecutionContext {
         frame: *mut PyFrame,
     ) -> Result<(), crate::PyError> {
         self.bytecode_only_trace(frame)?;
+        if majit_ir::eval_breaker_word::load() & majit_ir::eval_breaker_word::EB_ASYNC != 0 {
+            self.actionflag.sync_async_ticker();
+        }
         if self.actionflag.get_ticker() < 0 {
             // executioncontext.py:207-208 — `if actionflag.get_ticker()
             // < 0: actionflag.action_dispatcher(self, frame)`.  Routed
@@ -1374,7 +1381,7 @@ impl ExecutionContext {
         if let Some(action) = self.check_signal_action {
             let self_ptr = self as *mut ExecutionContext;
             unsafe {
-                (*action).perform(&mut *self_ptr, std::ptr::null_mut())?;
+                perform_async_action(action, self_ptr, std::ptr::null_mut())?;
             }
         }
         Ok(())
@@ -1515,6 +1522,16 @@ pub fn disarm_async_eval_breaker() {
     majit_ir::eval_breaker_word::clear_async();
 }
 
+#[cfg(not(target_arch = "wasm32"))]
+fn has_pending_signal_action() -> bool {
+    crate::module::signal::signalstate::has_pending_signals()
+}
+
+#[cfg(target_arch = "wasm32")]
+fn has_pending_signal_action() -> bool {
+    false
+}
+
 /// `dont_look_inside` so the tracer treats it as an opaque call and never
 /// follows the action machinery's trait-object virtual dispatch +
 /// `Result<(), PyError>` propagation (which the JIT codewriter cannot
@@ -1537,6 +1554,28 @@ pub extern "C" fn perform_pending_actions(ec_ptr: i64, frame_ptr: i64) -> i64 {
     }
 }
 
+/// Invoke one action, ending the action object's exclusive borrow before a
+/// requested GIL hand-off.  `GILReleaseAction` is process-owned, so the next
+/// thread can dispatch this same object as soon as it acquires the GIL.  A
+/// direct `perform()` implementation that yields would leave the first
+/// thread's `&mut dyn AsyncActionOps` live across that second mutable borrow.
+///
+/// # Safety
+///
+/// `action` and `ec` must point to live objects for the duration of the
+/// action call.  The caller must hold the GIL on entry.
+unsafe fn perform_async_action(
+    action: *mut dyn AsyncActionOps,
+    ec: *mut ExecutionContext,
+    frame: *mut PyFrame,
+) -> Result<(), crate::PyError> {
+    let control = unsafe { (*action).perform(&mut *ec, frame)? };
+    if control == AsyncActionControl::YieldGil {
+        majit_gc::rgil::yield_thread();
+    }
+    Ok(())
+}
+
 #[derive(Clone)]
 pub struct AbstractActionFlag {
     _periodic_actions: Vec<*mut dyn AsyncActionOps>,
@@ -1773,7 +1812,7 @@ pub trait ActionFlagOps {
                 continue;
             }
             unsafe {
-                (*action_ptr).perform(&mut *ec, frame)?;
+                perform_async_action(action_ptr, ec, frame)?;
             }
         }
         // executioncontext.py:543-556 — nonperiodic bit-mask scan.
@@ -1788,7 +1827,7 @@ pub trait ActionFlagOps {
                     self.abstract_flag_mut()._fired_bitmask &= !mask;
                     if !action_ptr.is_null() && !ec.is_null() {
                         unsafe {
-                            (*action_ptr).perform(&mut *ec, frame)?;
+                            perform_async_action(action_ptr, ec, frame)?;
                         }
                     }
                 }
@@ -1809,16 +1848,12 @@ pub trait ActionFlagOps {
 ///
 /// PyPy starts with a plain `ActionFlag` (its ticker is a Python field)
 /// and, when the `signal` module loads, rebinds `space.actionflag` to a
-/// `SignalActionFlag` whose ticker IS the C `pypysig_counter` cell so the
-/// OS signal handler can force it negative.  pyre merges the two: the
-/// ticker stays a plain `_ticker` field (a plain field read is what the
-/// JIT codewriter can model in the per-bytecode hot path — an atomic /
-/// volatile global read is not), and the OS signal handler writes -1 into
-/// it through a pointer registered at startup
-/// (`signalstate::register_ticker` ← `ticker_addr`).  This is the same
-/// arrangement as upstream's volatile `pypysig_counter.value`: the
-/// handler stores through the cell's address while the interpreter reads
-/// the field directly.
+/// `SignalActionFlag` whose ticker is the C `pypysig_counter` cell.  pyre
+/// merges the two while respecting Rust's memory model: `_ticker` remains a
+/// plain field for the translated per-bytecode hot path, the OS handler arms
+/// the atomic process eval breaker, and `sync_async_ticker` makes the field
+/// negative at the next safe interpreter checkpoint.  No asynchronous code
+/// reads or writes `_ticker`.
 #[derive(Clone)]
 pub struct ActionFlag {
     base: AbstractActionFlag,
@@ -1843,14 +1878,23 @@ impl ActionFlag {
         let _ = (ec, frame);
     }
 
-    /// Address of the ticker cell, handed to `signalstate::register_ticker`
-    /// so the OS signal handler can force the ticker negative.  Stable for
-    /// the process lifetime — the `ExecutionContext` is created once and
-    /// never moved (held behind an `Rc` in pyrex).
+    /// Address used to identify this as the signal-driving ticker.  The OS
+    /// handler never dereferences it; it is stable for the process lifetime
+    /// because the owning process `ActionFlag` allocation never moves.
     pub fn ticker_addr(&mut self) -> *mut isize {
         &mut self._ticker
     }
 
+    /// Transfer an asynchronous eval-breaker request into the registered
+    /// ticker.  Called only by the owning interpreter thread while it holds
+    /// the GIL and after observing `EB_ASYNC`; the OS signal handler never
+    /// accesses `_ticker` itself.
+    pub(crate) fn sync_async_ticker(&mut self) {
+        if self.is_registered_ticker() {
+            self._ticker = -1;
+        }
+    }
+
     /// True when `self._ticker` is the signal-registered ticker cell — the
     /// single cell compiled-loop back-edges poll. Only that ticker drives the
     /// shared async bit; per-EC flags that are not the registered breaker
@@ -1887,8 +1931,8 @@ impl ActionFlagOps for ActionFlag {
     }
 
     /// interp_signal.py:30-32 `SignalActionFlag.get_ticker` — `p.c_value`.
-    /// The cell is the `_ticker` field; the OS handler writes it through
-    /// the registered pointer (`ticker_addr`).
+    /// The safe-checkpoint synchronization above is the Rust equivalent of
+    /// the signal handler making this cell negative.
     fn get_ticker(&self) -> isize {
         self._ticker
     }
@@ -1901,17 +1945,18 @@ impl ActionFlagOps for ActionFlag {
             if value < 0 {
                 arm_async_eval_breaker();
             } else {
-                if !crate::module::thread::has_pending_async_exception() {
+                let async_work_pending = crate::module::thread::has_pending_async_exception()
+                    || has_pending_signal_action();
+                if !async_work_pending {
                     disarm_async_eval_breaker();
                 }
-                // A signal delivered between the ticker store and this clear
-                // rearms the ticker to -1 and re-sets the async bit; the clear
-                // would then drop it, leaving a negative ticker with the bit
-                // clear so a non-allocating compiled loop misses the signal.
-                // Re-read the ticker — the handler writes it through the
-                // registered pointer, so force a fresh load — and restore the
-                // bit if it was rearmed.
-                if unsafe { std::ptr::read_volatile(&self._ticker) } < 0 {
+                // A signal/async exception published between the first
+                // pending check and the clear above must re-arm the bit.  If
+                // it arrives after this second check, its publisher runs
+                // after the clear and leaves the bit armed itself.
+                if crate::module::thread::has_pending_async_exception()
+                    || has_pending_signal_action()
+                {
                     arm_async_eval_breaker();
                 }
             }
@@ -1982,8 +2027,8 @@ impl SpaceActionFlag {
 
     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.
+        // Interpreter calls are serialized by the GIL; the signal handler
+        // touches only process-global atomics, never this allocation.
         unsafe { &*self.ptr }
     }
 
@@ -2005,6 +2050,10 @@ impl SpaceActionFlag {
     pub fn ticker_addr(&mut self) -> *mut isize {
         self.inner_mut().ticker_addr()
     }
+
+    pub(crate) fn sync_async_ticker(&mut self) {
+        self.inner_mut().sync_async_ticker()
+    }
 }
 
 impl ActionFlagOps for SpaceActionFlag {
@@ -2144,18 +2193,27 @@ impl AsyncAction {
 /// `*mut dyn AsyncActionOps` so the dispatcher reaches the override
 /// through the embedded vtable, and `fire` / `action_dispatcher` can
 /// reach the base bitmask through the accessor.
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub enum AsyncActionControl {
+    Continue,
+    /// Complete the action call first, then hand the GIL to a waiter.
+    YieldGil,
+}
+
 pub trait AsyncActionOps {
     /// pypy/interpreter/executioncontext.py:608-609 `AsyncAction.perform`:
     /// `def perform(self, executioncontext, frame): "To be overridden."`
     ///
     /// Returns `Result` so an overriding action (e.g. `CheckSignalAction`)
     /// can raise — PyPy's `perform` propagates an `OperationError` up
-    /// through `action_dispatcher` to the eval loop.
+    /// through `action_dispatcher` to the eval loop.  The control value lets
+    /// the process-owned GIL action defer its hand-off until this method's
+    /// exclusive receiver has ended.
     fn perform(
         &mut self,
         executioncontext: &mut ExecutionContext,
         frame: *mut PyFrame,
-    ) -> Result<(), crate::PyError>;
+    ) -> Result<AsyncActionControl, crate::PyError>;
 
     /// Composition accessor: shared `AsyncAction` state.
     fn async_action(&self) -> &AsyncAction;
@@ -2245,8 +2303,8 @@ impl AsyncActionOps for AsyncAction {
         &mut self,
         _executioncontext: &mut ExecutionContext,
         _frame: *mut PyFrame,
-    ) -> Result<(), crate::PyError> {
-        Ok(())
+    ) -> Result<AsyncActionControl, crate::PyError> {
+        Ok(AsyncActionControl::Continue)
     }
 
     fn async_action(&self) -> &AsyncAction {
@@ -2335,8 +2393,8 @@ impl AsyncActionOps for PeriodicAsyncAction {
         &mut self,
         _executioncontext: &mut ExecutionContext,
         _frame: *mut PyFrame,
-    ) -> Result<(), crate::PyError> {
-        Ok(())
+    ) -> Result<AsyncActionControl, crate::PyError> {
+        Ok(AsyncActionControl::Continue)
     }
 
     fn async_action(&self) -> &AsyncAction {
@@ -2557,13 +2615,13 @@ impl AsyncActionOps for UserDelAction {
         &mut self,
         _executioncontext: &mut ExecutionContext,
         _frame: *mut PyFrame,
-    ) -> Result<(), crate::PyError> {
+    ) -> Result<AsyncActionControl, crate::PyError> {
         if self.collect_oldgen_before_run {
             self.collect_oldgen_before_run = false;
             pyre_object::gc_hook::try_gc_collect_oldgen();
         }
         self._run_finalizers();
-        Ok(())
+        Ok(AsyncActionControl::Continue)
     }
 
     fn async_action(&self) -> &AsyncAction {