Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 0 additions & 1 deletion Lib/test/test_thread.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ def task():
support.gc_collect() # For PyPy or other GCs.
self.assertEqual(thread._count(), orig)

@unittest.expectedFailure # TODO: RUSTPYTHON
def test_unraisable_exception(self):
def task():
started.release()
Expand Down
30 changes: 19 additions & 11 deletions crates/vm/src/stdlib/_thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ pub(crate) mod _thread {

use crate::{
AsObject, Py, PyPayload, PyRef, PyResult, VirtualMachine,
builtins::{PyDictRef, PyIntRef, PyStr, PyTupleRef, PyType, PyTypeRef, PyUtf8StrRef},
builtins::{
PyBaseExceptionRef, PyDictRef, PyIntRef, PyStr, PyTupleRef, PyType, PyTypeRef,
PyUtf8StrRef,
},
common::wtf8::Wtf8Buf,
frame::FrameRef,
function::{ArgCallable, FuncArgs, KwArgs, OptionalArg, PySetterValue, TimeoutSeconds},
Expand Down Expand Up @@ -558,6 +561,19 @@ pub(crate) mod _thread {
.map_err(|_err| vm.new_runtime_error("can't start new thread"))
}

fn report_unraisable_thread_exception(
exc: PyBaseExceptionRef,
func: &ArgCallable,
vm: &VirtualMachine,
) {
let msg = func
.as_ref()
.repr(vm)
.ok()
.map(|repr| format!("Exception ignored in thread started by {}", repr.as_wtf8()));
vm.run_unraisable(exc, msg, vm.ctx.none());
Comment on lines +569 to +574

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Handle repr failures gracefully to ensure the context message is printed.

If func.as_ref().repr(vm) fails, ok() maps the result to None, which suppresses the "Exception ignored in thread started by..." context message entirely. CPython prevents this by using a fallback message when the function's representation cannot be retrieved.

🐛 Proposed fix to provide a fallback
-        let msg = func
-            .as_ref()
-            .repr(vm)
-            .ok()
-            .map(|repr| format!("Exception ignored in thread started by {}", repr.as_wtf8()));
-        vm.run_unraisable(exc, msg, vm.ctx.none());
+        let msg = match func.as_ref().repr(vm) {
+            Ok(repr) => format!("Exception ignored in thread started by {}", repr.as_wtf8()),
+            Err(_) => "Exception ignored in thread started by <function repr() failed>".to_owned(),
+        };
+        vm.run_unraisable(exc, Some(msg), vm.ctx.none());
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let msg = func
.as_ref()
.repr(vm)
.ok()
.map(|repr| format!("Exception ignored in thread started by {}", repr.as_wtf8()));
vm.run_unraisable(exc, msg, vm.ctx.none());
let msg = match func.as_ref().repr(vm) {
Ok(repr) => format!("Exception ignored in thread started by {}", repr.as_wtf8()),
Err(_) => "Exception ignored in thread started by <function repr() failed>".to_owned(),
};
vm.run_unraisable(exc, Some(msg), vm.ctx.none());
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/vm/src/stdlib/_thread.rs` around lines 569 - 574, Update the message
construction in the thread exception path around func.as_ref().repr(vm) so repr
failures produce a fallback context string rather than None. Preserve the
existing formatted representation when repr succeeds, and always pass a context
message to vm.run_unraisable.

}

fn run_thread(func: ArgCallable, args: FuncArgs, vm: &VirtualMachine) {
// Increment thread count when thread actually starts executing
vm.state.thread_count.fetch_add(1);
Expand All @@ -572,11 +588,7 @@ pub(crate) mod _thread {
if let Err(exc) = func.invoke(args, vm)
&& !exc.fast_isinstance(vm.ctx.exceptions.system_exit)
{
vm.run_unraisable(
exc,
Some("Exception ignored in thread started by".to_owned()),
func.into(),
);
report_unraisable_thread_exception(exc, &func, vm);
}
}
for lock in SENTINELS.take() {
Expand Down Expand Up @@ -1755,11 +1767,7 @@ pub(crate) mod _thread {
if let Err(exc) = func.invoke((), vm)
&& !exc.fast_isinstance(vm.ctx.exceptions.system_exit)
{
vm.run_unraisable(
exc,
Some("Exception ignored in thread started by".to_owned()),
func.into(),
);
report_unraisable_thread_exception(exc, &func, vm);
}
}
}))
Expand Down
26 changes: 16 additions & 10 deletions crates/vm/src/vm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1296,18 +1296,24 @@ impl VirtualMachine {
}
};

let msg_str = if let Some(msg) = msg {
format!("{msg}: ")
if self.is_none(object) {
if let Some(msg) = msg {
write_to_stderr(&format!("{msg}:\n"), &stderr, self);
}
} else {
Comment on lines +1299 to 1303

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Remove the trailing colon when formatting msg without an object.

When object is None, the default unraisable hook should write the message followed by a newline, without a trailing colon. Currently, a thread exception will be incorrectly logged as Exception ignored in thread started by <function>: instead of matching CPython's format.

🐛 Proposed fix
         if self.is_none(object) {
             if let Some(msg) = msg {
-                write_to_stderr(&format!("{msg}:\n"), &stderr, self);
+                write_to_stderr(&format!("{msg}\n"), &stderr, self);
             }
         } else {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if self.is_none(object) {
if let Some(msg) = msg {
write_to_stderr(&format!("{msg}:\n"), &stderr, self);
}
} else {
if self.is_none(object) {
if let Some(msg) = msg {
write_to_stderr(&format!("{msg}\n"), &stderr, self);
}
} else {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/vm/src/vm/mod.rs` around lines 1299 - 1303, Update the None-object
branch in the default unraisable hook to write msg followed only by a newline,
removing the trailing colon from the format passed to write_to_stderr. Preserve
the existing conditional behavior and formatting for cases where an object is
present.

"Exception ignored in: ".to_owned()
};
write_to_stderr(&msg_str, &stderr, self);
let msg_str = if let Some(msg) = msg {
format!("{msg}: ")
} else {
"Exception ignored in: ".to_owned()
};
write_to_stderr(&msg_str, &stderr, self);

let repr_result = object.repr(self);
let repr_wtf8 = repr_result
.as_ref()
.map_or_else(|_| "<object repr failed>".as_ref(), |s| s.as_wtf8());
write_to_stderr(&format!("{repr_wtf8}\n"), &stderr, self);
let repr_result = object.repr(self);
let repr_wtf8 = repr_result
.as_ref()
.map_or_else(|_| "<object repr failed>".as_ref(), |s| s.as_wtf8());
write_to_stderr(&format!("{repr_wtf8}\n"), &stderr, self);
}

// Write exception type and message
let exc_type_name = e.class().name();
Expand Down