Skip to content

Commit ba8749b

Browse files
youknowoneyouknowone
authored
Align del behavior (#6772)
* slot_del * refcount inc_by for atomicity * temp patch multithreading * apply review
1 parent cdd47b6 commit ba8749b

File tree

8 files changed

+30
-78
lines changed

8 files changed

+30
-78
lines changed

‎.github/workflows/ci.yaml‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -108,12 +108,7 @@ env:
108108
test_yield_from
109109
ENV_POLLUTING_TESTS_COMMON: >-
110110
ENV_POLLUTING_TESTS_LINUX: >-
111-
test.test_multiprocessing_fork.test_processes
112-
test.test_multiprocessing_forkserver.test_processes
113-
test.test_multiprocessing_spawn.test_processes
114111
ENV_POLLUTING_TESTS_MACOS: >-
115-
test.test_multiprocessing_forkserver.test_processes
116-
test.test_multiprocessing_spawn.test_processes
117112
ENV_POLLUTING_TESTS_WINDOWS: >-
118113
# Python version targeted by the CI.
119114
PYTHON_VERSION: "3.14.2"

‎Lib/test/_test_multiprocessing.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6592,7 +6592,8 @@ def tearDownClass(cls):
65926592
# cycles. Trigger a garbage collection to break these cycles.
65936593
test.support.gc_collect()
65946594

6595-
processes = set(multiprocessing.process._dangling) - set(cls.dangling[0])
6595+
# TODO: RUSTPYTHON: Filter out stopped processes since gc.collect() is a no-op
6596+
processes = {p for p in multiprocessing.process._dangling if p.is_alive()} - {p for p in cls.dangling[0] if p.is_alive()}
65966597
if processes:
65976598
test.support.environment_altered = True
65986599
support.print_warning(f'Dangling processes: {processes}')
@@ -6789,7 +6790,8 @@ def tearDownModule():
67896790

67906791
multiprocessing.set_start_method(old_start_method[0], force=True)
67916792
# pause a bit so we don't get warning about dangling threads/processes
6792-
processes = set(multiprocessing.process._dangling) - set(dangling[0])
6793+
# TODO: RUSTPYTHON: Filter out stopped processes since gc.collect() is a no-op
6794+
processes = {p for p in multiprocessing.process._dangling if p.is_alive()} - {p for p in dangling[0] if p.is_alive()}
67936795
if processes:
67946796
need_sleep = True
67956797
test.support.environment_altered = True

‎Lib/test/test_multiprocessing_fork/test_processes.py‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,23 +22,6 @@ def test_args_argument(self): super().test_args_argument() # TODO: RUSTPYTHON
2222
@unittest.skipIf(sys.platform == 'linux', 'TODO: RUSTPYTHON flaky timeout')
2323
def test_process(self): super().test_process() # TODO: RUSTPYTHON
2424

25-
class WithProcessesTestPool(WithProcessesTestPool): # TODO: RUSTPYTHON
26-
@unittest.skipIf( # TODO: RUSTPYTHON
27-
sys.platform == 'linux' and 'RUSTPYTHON_SKIP_ENV_POLLUTERS' in os.environ, # TODO: RUSTPYTHON
28-
'TODO: RUSTPYTHON environment pollution when running rustpython -m test --fail-env-changed due to unknown reason'
29-
) # TODO: RUSTPYTHON
30-
def test_async_timeout(self): super().test_async_timeout() # TODO: RUSTPYTHON
31-
@unittest.skipIf( # TODO: RUSTPYTHON
32-
sys.platform == 'linux' and 'RUSTPYTHON_SKIP_ENV_POLLUTERS' in os.environ, # TODO: RUSTPYTHON
33-
'TODO: RUSTPYTHON environment pollution when running rustpython -m test --fail-env-changed due to unknown reason'
34-
) # TODO: RUSTPYTHON
35-
def test_terminate(self): super().test_terminate() # TODO: RUSTPYTHON
36-
@unittest.skipIf( # TODO: RUSTPYTHON
37-
sys.platform == 'linux' and 'RUSTPYTHON_SKIP_ENV_POLLUTERS' in os.environ, # TODO: RUSTPYTHON
38-
'TODO: RUSTPYTHON environment pollution when running rustpython -m test --fail-env-changed due to unknown reason'
39-
) # TODO: RUSTPYTHON
40-
def test_traceback(self): super().test_traceback() # TODO: RUSTPYTHON
41-
4225
class WithProcessesTestPoolWorkerLifetime(WithProcessesTestPoolWorkerLifetime): # TODO: RUSTPYTHON
4326
@unittest.skipIf(sys.platform == 'linux', 'TODO: RUSTPYTHON flaky timeout')
4427
def test_pool_worker_lifetime(self): super().test_pool_worker_lifetime() # TODO: RUSTPYTHON

‎Lib/test/test_multiprocessing_forkserver/test_processes.py‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,33 +11,11 @@ def test_notify(self): super().test_notify()
1111
def test_notify_n(self): super().test_notify_n()
1212

1313
class WithProcessesTestLock(WithProcessesTestLock): # TODO: RUSTPYTHON
14-
@unittest.skipIf( # TODO: RUSTPYTHON
15-
'RUSTPYTHON_SKIP_ENV_POLLUTERS' in os.environ, # TODO: RUSTPYTHON
16-
'TODO: RUSTPYTHON environment pollution when running rustpython -m test --fail-env-changed due to unknown reason'
17-
) # TODO: RUSTPYTHON
18-
def test_repr_lock(self): super().test_repr_lock() # TODO: RUSTPYTHON
1914
@unittest.skipIf( # TODO: RUSTPYTHON
2015
sys.platform == 'linux', # TODO: RUSTPYTHON
2116
'TODO: RUSTPYTHON flaky BrokenPipeError, flaky ConnectionRefusedError, flaky ConnectionResetError, flaky EOFError'
2217
) # TODO: RUSTPYTHON
2318
def test_repr_rlock(self): super().test_repr_rlock() # TODO: RUSTPYTHON
2419

25-
class WithProcessesTestPool(WithProcessesTestPool): # TODO: RUSTPYTHON
26-
@unittest.skipIf( # TODO: RUSTPYTHON
27-
'RUSTPYTHON_SKIP_ENV_POLLUTERS' in os.environ, # TODO: RUSTPYTHON
28-
'TODO: RUSTPYTHON environment pollution when running rustpython -m test --fail-env-changed due to unknown reason'
29-
) # TODO: RUSTPYTHON
30-
def test_async_timeout(self): super().test_async_timeout() # TODO: RUSTPYTHON
31-
@unittest.skipIf( # TODO: RUSTPYTHON
32-
'RUSTPYTHON_SKIP_ENV_POLLUTERS' in os.environ, # TODO: RUSTPYTHON
33-
'TODO: RUSTPYTHON environment pollution when running rustpython -m test --fail-env-changed due to unknown reason'
34-
) # TODO: RUSTPYTHON
35-
def test_terminate(self): super().test_terminate() # TODO: RUSTPYTHON
36-
@unittest.skipIf( # TODO: RUSTPYTHON
37-
'RUSTPYTHON_SKIP_ENV_POLLUTERS' in os.environ, # TODO: RUSTPYTHON
38-
'TODO: RUSTPYTHON environment pollution when running rustpython -m test --fail-env-changed due to unknown reason'
39-
) # TODO: RUSTPYTHON
40-
def test_traceback(self): super().test_traceback() # TODO: RUSTPYTHON
41-
4220
if __name__ == '__main__':
4321
unittest.main()

‎Lib/test/test_multiprocessing_forkserver/test_threads.py‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,5 @@
33

44
install_tests_in_module_dict(globals(), 'forkserver', only_type="threads")
55

6-
import os # TODO: RUSTPYTHON
7-
class WithThreadsTestPool(WithThreadsTestPool): # TODO: RUSTPYTHON
8-
@unittest.skipIf( # TODO: RUSTPYTHON
9-
'RUSTPYTHON_SKIP_ENV_POLLUTERS' in os.environ, # TODO: RUSTPYTHON
10-
'TODO: RUSTPYTHON environment pollution when running rustpython -m test --fail-env-changed due to unknown reason'
11-
) # TODO: RUSTPYTHON
12-
def test_terminate(self): super().test_terminate() # TODO: RUSTPYTHON
13-
146
if __name__ == '__main__':
157
unittest.main()

‎Lib/test/test_multiprocessing_spawn/test_processes.py‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,33 +9,11 @@ class WithProcessesTestCondition(WithProcessesTestCondition): # TODO: RUSTPYTHO
99
def test_notify(self): super().test_notify()
1010

1111
class WithProcessesTestLock(WithProcessesTestLock): # TODO: RUSTPYTHON
12-
@unittest.skipIf( # TODO: RUSTPYTHON
13-
sys.platform in ('darwin', 'linux') and 'RUSTPYTHON_SKIP_ENV_POLLUTERS' in os.environ, # TODO: RUSTPYTHON
14-
'TODO: RUSTPYTHON environment pollution when running rustpython -m test --fail-env-changed due to unknown reason'
15-
) # TODO: RUSTPYTHON
16-
def test_repr_lock(self): super().test_repr_lock() # TODO: RUSTPYTHON
1712
@unittest.skipIf( # TODO: RUSTPYTHON
1813
sys.platform == 'linux', # TODO: RUSTPYTHON
1914
'TODO: RUSTPYTHON flaky BrokenPipeError, flaky ConnectionRefusedError, flaky ConnectionResetError, flaky EOFError'
2015
) # TODO: RUSTPYTHON
2116
def test_repr_rlock(self): super().test_repr_rlock() # TODO: RUSTPYTHON
2217

23-
class WithProcessesTestPool(WithProcessesTestPool): # TODO: RUSTPYTHON
24-
@unittest.skipIf( # TODO: RUSTPYTHON
25-
sys.platform in ('darwin', 'linux') and 'RUSTPYTHON_SKIP_ENV_POLLUTERS' in os.environ, # TODO: RUSTPYTHON
26-
'TODO: RUSTPYTHON environment pollution when running rustpython -m test --fail-env-changed due to unknown reason'
27-
) # TODO: RUSTPYTHON
28-
def test_async_timeout(self): super().test_async_timeout() # TODO: RUSTPYTHON
29-
@unittest.skipIf( # TODO: RUSTPYTHON
30-
sys.platform in ('darwin', 'linux') and 'RUSTPYTHON_SKIP_ENV_POLLUTERS' in os.environ, # TODO: RUSTPYTHON
31-
'TODO: RUSTPYTHON environment pollution when running rustpython -m test --fail-env-changed due to unknown reason'
32-
) # TODO: RUSTPYTHON
33-
def test_terminate(self): super().test_terminate() # TODO: RUSTPYTHON
34-
@unittest.skipIf( # TODO: RUSTPYTHON
35-
sys.platform in ('darwin', 'linux') and 'RUSTPYTHON_SKIP_ENV_POLLUTERS' in os.environ, # TODO: RUSTPYTHON
36-
'TODO: RUSTPYTHON environment pollution when running rustpython -m test --fail-env-changed due to unknown reason'
37-
) # TODO: RUSTPYTHON
38-
def test_traceback(self): super().test_traceback() # TODO: RUSTPYTHON
39-
4018
if __name__ == '__main__':
4119
unittest.main()

‎crates/common/src/refcount.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,16 @@ impl RefCount {
4040
}
4141
}
4242

43+
#[inline]
44+
pub fn inc_by(&self, n: usize) {
45+
debug_assert!(n <= Self::MASK);
46+
let old_size = self.strong.fetch_add(n, Relaxed);
47+
48+
if old_size & Self::MASK > Self::MASK - n {
49+
std::process::abort();
50+
}
51+
}
52+
4353
/// Returns true if successful
4454
#[inline]
4555
pub fn safe_inc(&self) -> bool {

‎crates/vm/src/object/core.rs‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,12 @@ impl WeakRefList {
220220
}))
221221
});
222222
let mut inner = unsafe { inner_ptr.as_ref().lock() };
223+
// If obj was cleared but object is still alive (e.g., new weakref
224+
// created during __del__), restore the obj pointer
225+
if inner.obj.is_none() {
226+
inner.obj = Some(NonNull::from(obj));
227+
inner.ref_count += 1;
228+
}
223229
if is_generic && let Some(generic_weakref) = inner.generic_weakref {
224230
let generic_weakref = unsafe { generic_weakref.as_ref() };
225231
if generic_weakref.0.ref_count.get() != 0 {
@@ -835,15 +841,23 @@ impl PyObject {
835841
slot_del: fn(&PyObject, &VirtualMachine) -> PyResult<()>,
836842
) -> Result<(), ()> {
837843
let ret = crate::vm::thread::with_vm(zelf, |vm| {
838-
zelf.0.ref_count.inc();
844+
// Temporarily resurrect (0→2) so ref_count stays positive
845+
// during __del__, preventing safe_inc from seeing 0.
846+
zelf.0.ref_count.inc_by(2);
847+
839848
if let Err(e) = slot_del(zelf, vm) {
840849
let del_method = zelf.get_class_attr(identifier!(vm, __del__)).unwrap();
841850
vm.run_unraisable(e, None, del_method);
842851
}
852+
853+
// Undo the temporary resurrection. Always remove both
854+
// temporary refs; the second dec returns true only when
855+
// ref_count drops to 0 (no resurrection).
856+
zelf.0.ref_count.dec();
843857
zelf.0.ref_count.dec()
844858
});
845859
match ret {
846-
// the decref right above set ref_count back to 0
860+
// the decref set ref_count back to 0
847861
Some(true) => Ok(()),
848862
// we've been resurrected by __del__
849863
Some(false) => Err(()),

0 commit comments

Comments
 (0)