src/lib.rs (1)

176-189: Sandbox fallback design is sound but loses OS error detail.

The sandbox path correctly keeps filesystem I/O in the binary (trusted host) while the VM only sees source strings. Two minor observations:

1. std::fs::read_to_string assumes UTF-8, which is the Python 3 default source encoding — fine for the common case.
2. Line 187: err.to_string() discards the original io::ErrorKind/errno. Consider preserving it if the VM's new_os_error supports it, though this is low priority for a sandbox fallback.

Optional: preserve errno in sandbox error path

-            Err(err) => Err(vm.new_os_error(err.to_string())),
+            Err(err) => {
+                let msg = format!("Can't open file '{}': {}", path, err);
+                Err(vm.new_os_error(msg))
+            }

extra_tests/snippets/sandbox_smoke.py (1)

19-21: Minor: sys.stderr is not tested for writability.

stdin/stdout are tested but stderr is mentioned in the docstring (line 4) and available via SandboxStdio. Consider adding assert sys.stderr.writable() for completeness.

Optional: add stderr assertion

 assert sys.stdout.writable()
 assert sys.stdin.readable()
+assert sys.stderr.writable()
 assert sys.stdout.fileno() == 1

crates/vm/src/vm/python_run.rs (2)

149-155: Read::read may short-read even on regular files.

file.read(&mut buf)? is not guaranteed to fill the 2-byte buffer in a single call. While exceedingly rare for regular files, Read::read_exact would be the robust choice here.

Suggested fix

-        use std::io::Read;
-        if file.read(&mut buf)? != 2 {
+        use std::io::Read;
+        if file.read_exact(&mut buf).is_err() {

---

109-112: Commented-out deprecation on run_script.

The #[deprecated] attribute is commented out. If this is intentional for now, consider adding a brief // TODO explaining when the deprecation should be activated. Otherwise it's easy for this to be forgotten.

crates/vm/src/exceptions.rs (4)

1307-1345: build() relies on expect() for infallible paths — verify they truly can't fail.

Lines 1338, 1341, and 1343 each call .expect("new_os_error usage error"). While py_new always returns Ok(...) for the current implementation and into_ref_with_type should succeed when using the same exc_type, slot_init is more complex and could theoretically fail (e.g., on a subclass that overrides __init__). If any of these paths fail, the panic will be hard to diagnose.

Consider whether slot_init could fail for exotic subclasses and, if so, whether returning a PyResult from build would be more appropriate. If these are truly infallible, a brief comment on why would help future maintainers.

---

1320-1335: On non-Windows, a dummy None is injected at position 3 (winerror slot) into the args vec.

When errno is Some, the args vector is always 5 elements: [errno, strerror, filename, winerror_or_none, filename2]. On non-Windows, winerror is vm.ctx.none(). This means slot_init receives 5 args on all platforms — which works because slot_init only processes args[3] inside a #[cfg(windows)] block. However, this couples the builder's arg layout to slot_init's conditional parsing.

This is fine as-is, but a brief comment noting that the 4th position is always None on non-Windows (and ignored by slot_init) would prevent confusion.

---

1375-1385: libc::strerror is not guaranteed thread-safe.

On Unix, libc::strerror may return a pointer to a shared static buffer (for unknown errno values). While most modern libc implementations use static string literals for known errno values, the general case is not thread-safe. Since the result is immediately copied via to_string_lossy().into_owned(), the risk is low, but it's worth noting.

If thread-safety is a concern, strerror_r (POSIX) or strerror_l would be safer alternatives, though they come with their own portability complications.

---

1926-1926: Minor: Cloning the atomic ref just to check for None.

exc.filename.to_owned() clones the PyObjectRef behind the atomic ref only to check is_none. Consider using a read/deref approach if available to avoid the unnecessary clone:

-let has_filename = exc.filename.to_owned().filter(|f| !vm.is_none(f)).is_some();
+let has_filename = exc.filename.deref().is_some_and(|f| !vm.is_none(f));

(Depends on whether deref() returns a suitable reference — adjust as needed for the PyAtomicRef API.)

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

1375-1385: libc::strerror is not guaranteed thread-safe on all platforms.

On some platforms, strerror returns a pointer to a shared static buffer. While glibc uses thread-local storage, POSIX doesn't guarantee thread-safety. The immediate .to_string_lossy().into_owned() conversion minimizes the risk window, but strerror_r (or Rust's std::io::Error::from_raw_os_error(errno).to_string()) would be a safer alternative.

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

82-83: Nit: inconsistent #[cfg] style with set_interrupt_ex below.

Lines 82–83 use two stacked #[cfg(...)] attributes, while line 103 combines them into a single #[cfg(all(...))]. Both are semantically equivalent, but using a consistent style improves readability.

♻️ Suggested consolidation

-#[cfg(unix)]
-#[cfg(feature = "host_env")]
+#[cfg(all(unix, feature = "host_env"))]
 pub(crate) fn clear_after_fork() {