{{ message }}
Replace unsafe pointer cast with PyMutex in PyCode#7055
Merged
youknowone merged 1 commit intoRustPython:mainfrom Feb 9, 2026
Merged
Replace unsafe pointer cast with PyMutex in PyCode#7055youknowone merged 1 commit intoRustPython:mainfrom
youknowone merged 1 commit intoRustPython:mainfrom
Conversation
Contributor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/vm/src/builtins/code.rs`:
- Around line 337-351: Replace direct reads of the CodeObject field
"frame.code.source_path" with the PyCode accessor "frame.code.source_path()" so
the mutex-protected value from PyCode::source_path is used; search for
occurrences of "frame.code.source_path" in faulthandler.rs and update them to
call the accessor (leveraging the existing PyCode::source_path and
set_source_path methods).
Add `source_path: PyMutex<&'static PyStrInterned>` field to `PyCode` for interior mutability, replacing the UB-inducing `#[allow(invalid_reference_casting)]` + `write_volatile` pattern in `update_code_filenames`. Update all read sites across the codebase to use the new `source_path()` accessor method.
youknowone
force-pushed
the
pycode-path
branch
from
66eb96f to
5103e88
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/stdlib/src/faulthandler.rs`:
- Around line 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.
Comment on lines
254
to
256
| 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(); |
Contributor
There was a problem hiding this comment.
🧩 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 -A5Repository: 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 -100Repository: 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 -A8Repository: 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.,
AtomicPtroratomicreference) 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.
|
@youknowone doesn't Since the lifetime is What the update code does is actually an atomic compare and exchange, I think. |
Member
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add
source_path: PyMutex<&'static PyStrInterned>field toPyCodefor interior mutability, replacing the UB-inducing#[allow(invalid_reference_casting)]+write_volatilepattern inupdate_code_filenames. Update all 12 read sites across the codebase to use the newsource_path()method.Summary by CodeRabbit