Visitar URL original
Fix atexit unraisable exception message format (#7399) · RustPython/RustPython@85eca21 · GitHub
Skip to content

Commit 85eca21

Browse files
youknowoneyouknowone
authored
Fix atexit unraisable exception message format (#7399)
* Fix atexit unraisable exception message format Match PyErr_FormatUnraisable behavior: use "Exception ignored in atexit callback {func!r}" as err_msg and pass None as object instead of the callback function. * Fix atexit unregister deadlock with reentrant __eq__ Release the lock during equality comparison in unregister so that __eq__ can safely call atexit.unregister or atexit._clear. Store callbacks in LIFO order (insert at front) and use identity-based search after comparison to handle list mutations, matching atexitmodule.c behavior. Also pass None as err_msg when func.repr() fails, matching CPython's PyErr_FormatUnraisable fallback.
1 parent d248a04 commit 85eca21

3 files changed

Lines changed: 52 additions & 20 deletions

File tree

‎Lib/test/_test_atexit.py‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,22 +47,19 @@ def func2(*args, **kwargs):
4747
('func2', (), {}),
4848
('func1', (1, 2), {})])
4949

50-
@unittest.expectedFailure # TODO: RUSTPYTHON
5150
def test_badargs(self):
5251
def func():
5352
pass
5453

5554
# func() has no parameter, but it's called with 2 parameters
5655
self.assert_raises_unraisable(TypeError, func, 1 ,2)
5756

58-
@unittest.expectedFailure # TODO: RUSTPYTHON
5957
def test_raise(self):
6058
def raise_type_error():
6159
raise TypeError
6260

6361
self.assert_raises_unraisable(TypeError, raise_type_error)
6462

65-
@unittest.expectedFailure # TODO: RUSTPYTHON
6663
def test_raise_unnormalized(self):
6764
# bpo-10756: Make sure that an unnormalized exception is handled
6865
# properly.
@@ -71,7 +68,6 @@ def div_zero():
7168

7269
self.assert_raises_unraisable(ZeroDivisionError, div_zero)
7370

74-
@unittest.expectedFailure # TODO: RUSTPYTHON
7571
def test_exit(self):
7672
self.assert_raises_unraisable(SystemExit, sys.exit)
7773

@@ -122,7 +118,6 @@ def test_bound_methods(self):
122118
atexit._run_exitfuncs()
123119
self.assertEqual(l, [5])
124120

125-
@unittest.expectedFailure # TODO: RUSTPYTHON
126121
def test_atexit_with_unregistered_function(self):
127122
# See bpo-46025 for more info
128123
def func():
@@ -140,7 +135,6 @@ def func():
140135
finally:
141136
atexit.unregister(func)
142137

143-
@unittest.skip("TODO: RUSTPYTHON; Hangs")
144138
def test_eq_unregister_clear(self):
145139
# Issue #112127: callback's __eq__ may call unregister or _clear
146140
class Evil:
@@ -154,7 +148,6 @@ def __eq__(self, other):
154148
atexit.unregister(Evil())
155149
atexit._clear()
156150

157-
@unittest.skip("TODO: RUSTPYTHON; Hangs")
158151
def test_eq_unregister(self):
159152
# Issue #112127: callback's __eq__ may call unregister
160153
def f1():

‎crates/vm/src/stdlib/atexit.rs‎

Lines changed: 51 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@ mod atexit {
77

88
#[pyfunction]
99
fn register(func: PyObjectRef, args: FuncArgs, vm: &VirtualMachine) -> PyObjectRef {
10-
vm.state.atexit_funcs.lock().push((func.clone(), args));
10+
// Callbacks go in LIFO order (insert at front)
11+
vm.state
12+
.atexit_funcs
13+
.lock()
14+
.insert(0, Box::new((func.clone(), args)));
1115
func
1216
}
1317

@@ -18,27 +22,62 @@ mod atexit {
1822

1923
#[pyfunction]
2024
fn unregister(func: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
21-
let mut funcs = vm.state.atexit_funcs.lock();
22-
23-
let mut i = 0;
24-
while i < funcs.len() {
25-
if vm.bool_eq(&funcs[i].0, &func)? {
26-
funcs.remove(i);
27-
} else {
28-
i += 1;
25+
// Iterate backward (oldest to newest in LIFO list).
26+
// Release the lock during comparison so __eq__ can call atexit functions.
27+
let mut i = {
28+
let funcs = vm.state.atexit_funcs.lock();
29+
funcs.len() as isize - 1
30+
};
31+
while i >= 0 {
32+
let (cb, entry_ptr) = {
33+
let funcs = vm.state.atexit_funcs.lock();
34+
if i as usize >= funcs.len() {
35+
i = funcs.len() as isize;
36+
i -= 1;
37+
continue;
38+
}
39+
let entry = &funcs[i as usize];
40+
(entry.0.clone(), &**entry as *const (PyObjectRef, FuncArgs))
41+
};
42+
// Lock released: __eq__ can safely call atexit functions
43+
let eq = vm.bool_eq(&func, &cb)?;
44+
if eq {
45+
// The entry may have moved during __eq__. Search backward by identity.
46+
let mut funcs = vm.state.atexit_funcs.lock();
47+
let mut j = (funcs.len() as isize - 1).min(i);
48+
while j >= 0 {
49+
if core::ptr::eq(&**funcs.get(j as usize).unwrap(), entry_ptr) {
50+
funcs.remove(j as usize);
51+
i = j;
52+
break;
53+
}
54+
j -= 1;
55+
}
2956
}
57+
{
58+
let funcs = vm.state.atexit_funcs.lock();
59+
if i as usize >= funcs.len() {
60+
i = funcs.len() as isize;
61+
}
62+
}
63+
i -= 1;
3064
}
31-
3265
Ok(())
3366
}
3467

3568
#[pyfunction]
3669
pub fn _run_exitfuncs(vm: &VirtualMachine) {
3770
let funcs: Vec<_> = core::mem::take(&mut *vm.state.atexit_funcs.lock());
38-
for (func, args) in funcs.into_iter().rev() {
71+
// Callbacks stored in LIFO order, iterate forward
72+
for entry in funcs.into_iter() {
73+
let (func, args) = *entry;
3974
if let Err(e) = func.call(args, vm) {
4075
let exit = e.fast_isinstance(vm.ctx.exceptions.system_exit);
41-
vm.run_unraisable(e, Some("Error in atexit._run_exitfuncs".to_owned()), func);
76+
let msg = func
77+
.repr(vm)
78+
.ok()
79+
.map(|r| format!("Exception ignored in atexit callback {}", r.as_wtf8()));
80+
vm.run_unraisable(e, msg, vm.ctx.none());
4281
if exit {
4382
break;
4483
}

‎crates/vm/src/vm/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,7 @@ pub struct PyGlobalState {
585585
pub stacksize: AtomicCell<usize>,
586586
pub thread_count: AtomicCell<usize>,
587587
pub hash_secret: HashSecret,
588-
pub atexit_funcs: PyMutex<Vec<(PyObjectRef, FuncArgs)>>,
588+
pub atexit_funcs: PyMutex<Vec<Box<(PyObjectRef, FuncArgs)>>>,
589589
pub codec_registry: CodecsRegistry,
590590
pub finalizing: AtomicBool,
591591
pub warnings: WarningsState,

0 commit comments

Comments
 (0)