Skip to content

Replace unsafe pointer cast with PyMutex in PyCode #7055

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 1 commit into from
Feb 9, 2026
+33 −26
Merged

Replace unsafe pointer cast with PyMutex in PyCode #7055

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/stdlib/src/faulthandler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ mod decl {
/// Dump a single frame's info to fd (signal-safe), reading live data.
#[cfg(any(unix, windows))]
fn dump_frame_from_raw(fd: i32, frame: &Frame) {
let filename = frame.code.source_path.as_str();
let filename = frame.code.source_path().as_str();
let funcname = frame.code.obj_name.as_str();
Comment on lines 254 to 256
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find PyMutex definition
echo "=== PyMutex Definition ==="
rg -n 'type PyMutex|struct PyMutex' --type rust -A3 -B1

echo ""
echo "=== source_path method signature and impl ==="
# Search for source_path method definition
rg -n 'fn source_path' --type rust -A8

echo ""
echo "=== Check signal handler context ==="
# Find faulthandler_fatal_error to confirm it's a signal handler
rg -n 'faulthandler_fatal_error|extern "C"' crates/stdlib/src/faulthandler.rs -B2 -A5

Repository: RustPython/RustPython

Length of output: 3633

🏁 Script executed:

#!/bin/bash
# Let's check the specific file to understand the context better
echo "=== Checking faulthandler.rs structure ==="
cat -n crates/stdlib/src/faulthandler.rs | head -300 | tail -100

Repository: RustPython/RustPython

Length of output: 4032

🏁 Script executed:

#!/bin/bash
# Check what the Frame structure looks like and where source_path comes from
echo "=== Frame structure and code field ==="
rg -n 'struct Frame|pub struct Frame' --type rust -A10

echo ""
echo "=== PyCode structure related to source_path ==="
rg -n 'pub struct PyCode|source_path' --type rust -B2 -A8

Repository: RustPython/RustPython

Length of output: 48965

source_path() acquires a mutex inside a signal handler — potential deadlock.

dump_frame_from_raw (line 254) is reachable from faulthandler_fatal_error, which is a signal handler. Calling source_path() at line 255 locks a PyMutex. If the signal fires while the same PyCode's source_path mutex is held elsewhere, this causes a deadlock.

The comment on line 252 explicitly documents this function as "(signal-safe)", which is now violated. Consider either:

  • Storing a separate signal-safe copy (e.g., AtomicPtr or atomic reference) for signal-handler reads, or
  • Using try_lock() with a fallback (e.g., print "<unknown>" if the lock is contended).
🤖 Prompt for AI Agents 
In `@crates/stdlib/src/faulthandler.rs` around lines 254 - 256,
dump_frame_from_raw is called from the signal handler faulthandler_fatal_error
but calls frame.code.source_path() which acquires a PyMutex and can deadlock;
make this signal-safe by avoiding any mutex acquisition: either add a
signal-safe cached string pointer on PyCode (e.g., an atomic/AtomicPtr holding a
nul-terminated C string updated outside signal handlers and read here) and read
that in dump_frame_from_raw, or replace the call to source_path() with a
non-blocking attempt (call try_lock() on the PyCode/source_path mutex and if it
fails print a fallback like "<unknown>" or the raw pointer/address), updating
references to Frame/ PyCode/ source_path accordingly so dump_frame_from_raw
never blocks in a signal context.

let lasti = frame.lasti();
let lineno = if lasti == 0 {
Expand Down Expand Up @@ -328,7 +328,7 @@ mod decl {
#[cfg(any(unix, windows))]
fn dump_frame_from_ref(fd: i32, frame: &crate::vm::PyRef<Frame>) {
let funcname = frame.code.obj_name.as_str();
let filename = frame.code.source_path.as_str();
let filename = frame.code.source_path().as_str();
let lineno = if frame.lasti() == 0 {
frame.code.first_line_number.map(|n| n.get()).unwrap_or(1) as u32
} else {
Expand Down
24 changes: 19 additions & 5 deletions crates/vm/src/builtins/code.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use alloc::fmt;
use core::{borrow::Borrow, ops::Deref};
use malachite_bigint::BigInt;
use num_traits::Zero;
use rustpython_common::lock::PyMutex;
use rustpython_compiler_core::{OneIndexed, bytecode::CodeUnits, bytecode::PyCodeLocationInfoKind};

/// State for iterating through code address ranges
Expand Down Expand Up @@ -323,6 +324,7 @@ impl<B: AsRef<[u8]>> IntoCodeObject for frozen::FrozenCodeObject<B> {
#[pyclass(module = false, name = "code")]
pub struct PyCode {
pub code: CodeObject,
source_path: PyMutex<&'static PyStrInterned>,
}

impl Deref for PyCode {
Expand All @@ -333,8 +335,20 @@ impl Deref for PyCode {
}

impl PyCode {
pub const fn new(code: CodeObject) -> Self {
Self { code }
pub fn new(code: CodeObject) -> Self {
let sp = code.source_path;
Self {
code,
source_path: PyMutex::new(sp),
}
}

pub fn source_path(&self) -> &'static PyStrInterned {
*self.source_path.lock()
}

pub fn set_source_path(&self, new: &'static PyStrInterned) {
*self.source_path.lock() = new;
}
pub fn from_pyc_path(path: &std::path::Path, vm: &VirtualMachine) -> PyResult<PyRef<Self>> {
let name = match path.file_stem() {
Expand Down Expand Up @@ -397,7 +411,7 @@ impl Representable for PyCode {
"<code object {} at {:#x} file {:?}, line {}>",
code.obj_name,
zelf.get_id(),
code.source_path.as_str(),
zelf.source_path().as_str(),
code.first_line_number.map_or(-1, |n| n.get() as i32)
))
}
Expand Down Expand Up @@ -572,7 +586,7 @@ impl PyCode {

#[pygetset]
pub fn co_filename(&self) -> PyStrRef {
self.code.source_path.to_owned()
self.source_path().to_owned()
}

#[pygetset]
Expand Down Expand Up @@ -906,7 +920,7 @@ impl PyCode {

let source_path = match co_filename {
OptionalArg::Present(source_path) => source_path,
OptionalArg::Missing => self.code.source_path.to_owned(),
OptionalArg::Missing => self.source_path().to_owned(),
};

let first_line_number = match co_firstlineno {
Expand Down
2 changes: 1 addition & 1 deletion crates/vm/src/builtins/traceback.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ impl serde::Serialize for PyTraceback {
let mut struc = s.serialize_struct("PyTraceback", 3)?;
struc.serialize_field("name", self.frame.code.obj_name.as_str())?;
struc.serialize_field("lineno", &self.lineno.get())?;
struc.serialize_field("filename", self.frame.code.source_path.as_str())?;
struc.serialize_field("filename", self.frame.code.source_path().as_str())?;
struc.end()
}
}
2 changes: 1 addition & 1 deletion crates/vm/src/exceptions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ fn write_traceback_entry<W: Write>(
output: &mut W,
tb_entry: &Py<PyTraceback>,
) -> Result<(), W::Error> {
let filename = tb_entry.frame.code.source_path.as_str();
let filename = tb_entry.frame.code.source_path().as_str();
writeln!(
output,
r##" File "{}", line {}, in {}"##,
Expand Down
2 changes: 1 addition & 1 deletion crates/vm/src/frame.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3223,7 +3223,7 @@ impl ExecutingFrame<'_> {
self.lasti(),
self.code.obj_name,
op_name,
self.code.source_path
self.code.source_path()
);
}
self.state.stack.drain(stack_len - count..).map(|obj| {
Expand Down
4 changes: 2 additions & 2 deletions crates/vm/src/import.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ pub fn import_code_obj(
if set_file_attr {
attrs.set_item(
identifier!(vm, __file__),
code_obj.source_path.to_object(),
code_obj.source_path().to_object(),
vm,
)?;
}
Expand All @@ -195,7 +195,7 @@ fn remove_importlib_frames_inner(
return (None, false);
};

let file_name = traceback.frame.code.source_path.as_str();
let file_name = traceback.frame.code.source_path().as_str();

let (inner_tb, mut now_in_importlib) =
remove_importlib_frames_inner(vm, traceback.next.lock().clone(), always_trim);
Expand Down
15 changes: 4 additions & 11 deletions crates/vm/src/stdlib/imp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ mod _imp {

#[pyfunction]
fn _fix_co_filename(code: PyRef<PyCode>, path: PyStrRef, vm: &VirtualMachine) {
let old_name = code.code.source_path;
let old_name = code.source_path();
let new_name = vm.ctx.intern_str(path.as_str());
super::update_code_filenames(&code, old_name, new_name);
}
Expand Down Expand Up @@ -288,18 +288,11 @@ fn update_code_filenames(
old_name: &'static PyStrInterned,
new_name: &'static PyStrInterned,
) {
if !core::ptr::eq(code.code.source_path, old_name)
&& code.code.source_path.as_str() != old_name.as_str()
{
let current = code.source_path();
if !core::ptr::eq(current, old_name) && current.as_str() != old_name.as_str() {
return;
}
// SAFETY: called during import before the code object is shared.
// Mutates co_filename in place.
#[allow(invalid_reference_casting)]
unsafe {
let source_path_ptr = &code.code.source_path as *const _ as *mut &'static PyStrInterned;
core::ptr::write_volatile(source_path_ptr, new_name);
}
code.set_source_path(new_name);
for constant in code.code.constants.iter() {
let obj: &crate::PyObject = constant.borrow();
if let Some(inner_code) = obj.downcast_ref::<PyCode>() {
Expand Down
2 changes: 1 addition & 1 deletion crates/vm/src/stdlib/io.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4971,7 +4971,7 @@ mod _io {
&& let Some(frame) = vm.current_frame()
&& let Some(stdlib_dir) = vm.state.config.paths.stdlib_dir.as_deref()
{
let path = frame.code.source_path.as_str();
let path = frame.code.source_path().as_str();
if !path.starts_with(stdlib_dir) {
stacklevel = stacklevel.saturating_sub(1);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/vm/src/vm/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ impl Context {

pub fn new_code(&self, code: impl code::IntoCodeObject) -> PyRef<PyCode> {
let code = code.into_code_object(self);
PyRef::new_ref(PyCode { code }, self.types.code_type.to_owned(), None)
PyRef::new_ref(PyCode::new(code), self.types.code_type.to_owned(), None)
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/vm/src/warn.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ fn setup_context(
}

let (globals, filename, lineno) = if let Some(f) = f {
(f.globals.clone(), f.code.source_path, f.f_lineno())
(f.globals.clone(), f.code.source_path(), f.f_lineno())
} else if let Some(frame) = vm.current_frame() {
// We have a frame but it wasn't found during stack walking
(frame.globals.clone(), vm.ctx.intern_str("<sys>"), 1)
Expand Down
Loading