Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

crates/vm/src/frame.rs (1)

395-407: ⚠️ Potential issue | 🔴 Critical

Avoid unsynchronized fastlocals access when try_lock() fails.

Line 399 allows None from try_lock(), but Line 407 still performs unsafe { self.fastlocals.borrow() }. If another thread owns the mutex, this is a data race/UB.

🐛 Proposed safety fix

 pub fn locals(&self, vm: &VirtualMachine) -> PyResult<ArgMapping> {
-    // Acquire the state mutex to synchronize with frame execution.
-    // If try_lock fails, the frame is executing on this thread (e.g.
-    // trace callback accessing f_locals), so fastlocals access is safe.
-    let _guard = self.state.try_lock();
+    // Acquire the state mutex to synchronize with frame execution.
+    // If unavailable, avoid unsynchronized fastlocals reads.
+    let _guard = match self.state.try_lock() {
+        Some(guard) => guard,
+        None => return Ok(self.locals.clone()),
+    };
@@
-        // SAFETY: Either _guard holds the state mutex (frame not executing),
-        // or we're in a trace callback on the same thread that holds it.
+        // SAFETY: `_guard` holds the state mutex, so fastlocals access is exclusive.
         let fastlocals = unsafe { self.fastlocals.borrow() };

Use this to verify there is no explicit same-thread ownership check before the unsafe borrow:

#!/bin/bash
set -euo pipefail

echo "1) locals() lock/unsafe flow"
rg -n "pub fn locals\\(|try_lock\\(|fastlocals\\.borrow\\(" crates/vm/src/frame.rs -C4

echo
echo "2) Any thread-identity guard in frame path"
rg -n "ThreadId|current_thread|owner\\.load|owner" crates/vm/src/frame.rs -C3

echo
echo "3) Mutex backend and try_lock definitions/usages"
fd lock.rs | xargs rg -n "PyMutex|try_lock|parking_lot|Mutex" -C2

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/vm/src/frame.rs` around lines 395 - 407, The locals() method currently
calls self.state.try_lock() and ignores a failed lock (None), but always
proceeds to unsafely access self.fastlocals.borrow(), which can race if another
thread owns the mutex; modify locals() so that when try_lock() returns None you
either (a) acquire the lock with self.state.lock() (or spin until safe) before
doing the unsafe borrow, or (b) check a same-thread/owner marker on the frame
(e.g. an owner ThreadId) and only perform unsafe { self.fastlocals.borrow() }
when the current thread is the owner; update references to _guard, try_lock(),
state, locals(), and fastlocals.borrow() accordingly to ensure the
unsynchronized path cannot perform the unsafe borrow.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro