diff --git a/.cspell.dict/rust-more.txt b/.cspell.dict/rust-more.txt index c4457723c6c..e7fb056b0c7 100644 --- a/.cspell.dict/rust-more.txt +++ b/.cspell.dict/rust-more.txt @@ -13,6 +13,7 @@ bstr byteorder byteset caseless +cdpt chrono consts cranelift diff --git a/Cargo.lock b/Cargo.lock index e6a4194a056..b053406245a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -144,6 +144,12 @@ version = "1.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c3d036a3c4ab069c7b410a2ce876bd74808d2d0888a82667669f8e783a898bf1" +[[package]] +name = "arrayvec" +version = "0.7.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f02882884d3e1bc524fb12c79f107f6ad0e1cfd498c536ffb494301740995dfe" + [[package]] name = "ascii" version = "1.1.0" @@ -460,6 +466,34 @@ dependencies = [ "shlex", ] +[[package]] +name = "cdpt" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d0eb396bf1dacfc5f92972e6d9d5dec3b11ef6e8e6fa84f161554158662aafe" +dependencies = [ + "arrayvec", + "cdpt-derive", + "cfg-if", + "crossbeam", + "fastrand", + "libc", + "rustc-hash", + "static_assertions", + "windows-sys 0.60.2", +] + +[[package]] +name = "cdpt-derive" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "83bb6b4d5cec861158c8e0e2037f0a94afd01a7739dc118510c854b8f31f14b7" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "cexpr" version = "0.6.0" @@ -962,6 +996,28 @@ dependencies = [ "itertools 0.13.0", ] +[[package]] +name = "crossbeam" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1137cd7e7fc0fb5d3c5a8678be38ec56e819125d8d7907411fe24ccb943faca8" +dependencies = [ + "crossbeam-channel", + "crossbeam-deque", + "crossbeam-epoch", + "crossbeam-queue", + "crossbeam-utils", +] + +[[package]] +name = "crossbeam-channel" +version = "0.5.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "82b8f8f868b36967f9606790d1903570de9ceaf870a7bf9fbbd3016d636a2cb2" +dependencies = [ + "crossbeam-utils", +] + [[package]] name = "crossbeam-deque" version = "0.8.6" @@ -981,6 +1037,15 @@ dependencies = [ "crossbeam-utils", ] +[[package]] +name = "crossbeam-queue" +version = "0.3.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0f58bbc28f91df819d0aa2a2c00cd19754769c2fad90579b3592b1c9ba7a3115" +dependencies = [ + "crossbeam-utils", +] + [[package]] name = "crossbeam-utils" version = "0.8.21" @@ -3803,6 +3868,7 @@ dependencies = [ "ascii", "bitflags 2.13.0", "bstr", + "cdpt", "chrono", "constant_time_eq", "crossbeam-utils", diff --git a/Cargo.toml b/Cargo.toml index 4781c95e101..0f296d5f64f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -201,6 +201,7 @@ bitflagset = "0.0.3" bstr = "1" bzip2 = "0.6" chrono = { version = "0.4.44", default-features = false, features = ["clock", "std"] } +cdpt = "0.1.0" console_error_panic_hook = "0.1" constant_time_eq = "0.5" cranelift = "0.132.0" diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index 855e1c79cc8..0fe63332d15 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -236,7 +236,6 @@ def test_function(self): # is 3 because it includes f's code object. self.assertIn(gc.collect(), (2, 3)) - @unittest.expectedFailure # TODO: RUSTPYTHON; - weakref clear ordering differs from 3.15+ def test_function_tp_clear_leaves_consistent_state(self): # https://github.com/python/cpython/issues/91636 code = """if 1: @@ -831,7 +830,6 @@ def __del__(self): rc, out, err = assert_python_ok(TESTFN) self.assertEqual(out.strip(), b'__del__ called') - @unittest.expectedFailure # TODO: RUSTPYTHON def test_get_stats(self): stats = gc.get_stats() self.assertEqual(len(stats), 3) diff --git a/crates/vm/Cargo.toml b/crates/vm/Cargo.toml index b3479e017a1..cc15e380c0b 100644 --- a/crates/vm/Cargo.toml +++ b/crates/vm/Cargo.toml @@ -90,6 +90,7 @@ writeable = { workspace = true } exitcode = { workspace = true } [target.'cfg(not(target_arch = "wasm32"))'.dependencies] +cdpt = { workspace = true } rustyline = { workspace = true } which = { workspace = true } widestring = { workspace = true } diff --git a/crates/vm/src/gc_state.rs b/crates/vm/src/gc_state.rs index 93596a9dad7..9d762b11e4f 100644 --- a/crates/vm/src/gc_state.rs +++ b/crates/vm/src/gc_state.rs @@ -136,6 +136,21 @@ impl GcGeneration { #[derive(Clone, Copy, PartialEq, Eq, Hash)] struct GcPtr(NonNull); +fn clear_weakrefs_and_invoke_callbacks(objects: &[PyObjectRef]) { + let mut all_callbacks = Vec::new(); + for obj_ref in objects { + let callbacks = obj_ref.gc_clear_weakrefs_collect_callbacks(); + all_callbacks.extend(callbacks); + } + for (wr, cb) in all_callbacks { + if let Some(Err(e)) = crate::vm::thread::with_vm(&cb, |vm| cb.call((wr.clone(),), vm)) { + crate::vm::thread::with_vm(&cb, |vm| { + vm.run_unraisable(e.clone(), Some("weakref callback".to_owned()), cb.clone()); + }); + } + } +} + /// Global GC state pub struct GcState { /// 3 generations (0 = youngest, 2 = oldest) @@ -398,6 +413,11 @@ impl GcState { _ => std::time::Instant::now(), }; + // Keep a CDPT guard during collection, following origin/gc's + // coarse-grained guarded collection experiment. + #[cfg(not(target_arch = "wasm32"))] + let _cdpt_guard = cdpt::pin(); + // Memory barrier to ensure visibility of all reference count updates // from other threads before we start analyzing the object graph. core::sync::atomic::fence(Ordering::SeqCst); @@ -602,20 +622,10 @@ impl GcState { }) .collect(); - // 6c: Clear existing weakrefs BEFORE calling __del__ - let mut all_callbacks: Vec<(crate::PyRef, crate::PyObjectRef)> = - Vec::new(); - for obj_ref in &unreachable_refs { - let callbacks = obj_ref.gc_clear_weakrefs_collect_callbacks(); - all_callbacks.extend(callbacks); - } - for (wr, cb) in all_callbacks { - if let Some(Err(e)) = crate::vm::thread::with_vm(&cb, |vm| cb.call((wr.clone(),), vm)) { - crate::vm::thread::with_vm(&cb, |vm| { - vm.run_unraisable(e.clone(), Some("weakref callback".to_owned()), cb.clone()); - }); - } - } + // 6c: Clear weakrefs that existed before finalizers. This prevents a + // later tp_clear side effect from observing callback-free weakrefs to + // garbage in this generation. + clear_weakrefs_and_invoke_callbacks(&unreachable_refs); // 6d: Call __del__ on unreachable objects (skip already-finalized). // try_call_finalizer() internally checks gc_finalized() and sets it, @@ -624,6 +634,11 @@ impl GcState { obj_ref.try_call_finalizer(); } + // 6e: Clear weakrefs after finalizers, but before tp_clear. Finalizers + // can create weakrefs to other unreachable objects, and those must not + // reveal an object while its clear function is running. + clear_weakrefs_and_invoke_callbacks(&unreachable_refs); + // Detect resurrection let mut resurrected_set: HashSet = HashSet::new(); let unreachable_set: HashSet = unreachable.iter().copied().collect(); diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs index 88ca646a4f1..b557ca2e6a6 100644 --- a/crates/vm/src/object/core.rs +++ b/crates/vm/src/object/core.rs @@ -1770,7 +1770,8 @@ impl PyObject { } /// Clear weakrefs but collect callbacks instead of calling them. - /// This is used by GC to ensure ALL weakrefs are cleared BEFORE any callbacks run. + /// GC uses this while handling a garbage set so callbacks run only after all + /// weakrefs in the current pass have been invalidated. /// Returns collected callbacks as (PyRef, callback) pairs. // = handle_weakrefs pub fn gc_clear_weakrefs_collect_callbacks(&self) -> Vec<(PyRef, PyObjectRef)> { diff --git a/crates/vm/src/stdlib/gc.rs b/crates/vm/src/stdlib/gc.rs index 00eea1b39d5..a6492fedf85 100644 --- a/crates/vm/src/stdlib/gc.rs +++ b/crates/vm/src/stdlib/gc.rs @@ -142,8 +142,6 @@ mod gc { vm.ctx.new_int(stat.uncollectable).into(), vm, )?; - dict.set_item("candidates", vm.ctx.new_int(stat.candidates).into(), vm)?; - dict.set_item("duration", vm.ctx.new_float(stat.duration).into(), vm)?; result.push(dict.into()); }