From 05de2aaa3fb30d61b06d1310e2a7138b629cac09 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 10 Mar 2026 08:05:57 +0000 Subject: [PATCH 1/3] Initial plan From 25dbf2dfac7a1a077d996f758b10709ff917325a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 10 Mar 2026 08:14:12 +0000 Subject: [PATCH 2/3] Fix Arc::relocated() storage corruption when source == destination; add debug_assert for inconsistent source affinities Co-authored-by: Darksecond <291582+Darksecond@users.noreply.github.com> --- crates/thread_aware/src/cell/mod.rs | 17 ++++++++++++++- crates/thread_aware/src/cell/tests.rs | 30 +++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/crates/thread_aware/src/cell/mod.rs b/crates/thread_aware/src/cell/mod.rs index 92efc0194..ce4f2e2d2 100644 --- a/crates/thread_aware/src/cell/mod.rs +++ b/crates/thread_aware/src/cell/mod.rs @@ -499,6 +499,16 @@ impl ThreadAware for Arc { Factory::Closure(factory, factory_source_affinity) => { let factory_clone = (**factory).clone(); + // Detect inconsistent source affinities across multiple relocated() calls: + // once factory_source_affinity is recorded, all subsequent calls must + // supply the same source so that factory data is relocated correctly. + debug_assert!( + factory_source_affinity.is_none() || *factory_source_affinity == Some(source), + "Caller passed source affinity {source:?} but the factory was originally created from \ + {factory_source_affinity:?}; all calls to relocated() on the same Arc must use the \ + same source affinity to ensure factory data is relocated consistently" + ); + // In case factory source is stored in factory, use that - it means we already transferred the factory // once, so we know the original source affinity. Otherwise, use source as that means this is the first // time we're transferring the Arc, so source is the source affinity of the factory as well. @@ -532,7 +542,12 @@ impl ThreadAware for Arc { }; if let MemoryAffinity::Pinned(source) = source { - guard.replace(source, self.value); + // Only restore the value to the source slot when source and destination differ. + // If they are the same slot, the replacement above already stored the new value + // there; overwriting it here would corrupt storage with the stale pre-relocation value. + if source != destination { + guard.replace(source, self.value); + } } drop(guard); diff --git a/crates/thread_aware/src/cell/tests.rs b/crates/thread_aware/src/cell/tests.rs index d2a2794b9..116c14bf0 100644 --- a/crates/thread_aware/src/cell/tests.rs +++ b/crates/thread_aware/src/cell/tests.rs @@ -458,3 +458,33 @@ fn test_strong_count_independent_across_affinities() { // arc_b on affinity2 is unaffected by the clone on affinity1 assert_eq!(PerCore::strong_count(&arc_b), 1); // Still 1; unaffected by clone on affinity1 } + +#[test] +fn test_relocated_source_equals_destination_does_not_corrupt_storage() { + // Regression test: when source == destination, Arc::relocated() must NOT overwrite the + // newly-created value in storage with the stale pre-relocation value. + let affinities = pinned_affinities(&[2]); + let affinity = affinities[0]; + + // Create an Arc whose Counter starts at zero, then advance it. + let arc = PerCore::new(Counter::new); + arc.increment_by(42); + assert_eq!(arc.value(), 42); + + // Relocate with source == destination. The ThreadAware impl always creates a *new* + // Counter (value resets to 0), so `relocated` must return 0 and must also leave + // storage holding 0 (not the stale 42). + let relocated = arc.relocated(affinity.into(), affinity); + assert_eq!(relocated.value(), 0, "relocated value should come from factory"); + + // A second relocation from the same slot must find the factory-created value (0) in + // storage, not the stale pre-relocation value (42). Before the bug fix, the first + // relocated() call wrote the stale Arc back into the storage slot, + // so the second call's `get_clone` fast-path would return 42 instead of 0. + let relocated_again = relocated.relocated(affinity.into(), affinity); + assert_eq!( + relocated_again.value(), + 0, + "subsequent relocation must not see stale pre-relocation value from storage" + ); +} From a8df600a98b2adddd9d1f22b7d67ce7c628e694e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 10 Mar 2026 09:31:08 +0000 Subject: [PATCH 3/3] Remove debug_assert for inconsistent source affinity Co-authored-by: Darksecond <291582+Darksecond@users.noreply.github.com> --- crates/thread_aware/src/cell/mod.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/crates/thread_aware/src/cell/mod.rs b/crates/thread_aware/src/cell/mod.rs index ce4f2e2d2..e9a7677ae 100644 --- a/crates/thread_aware/src/cell/mod.rs +++ b/crates/thread_aware/src/cell/mod.rs @@ -499,16 +499,6 @@ impl ThreadAware for Arc { Factory::Closure(factory, factory_source_affinity) => { let factory_clone = (**factory).clone(); - // Detect inconsistent source affinities across multiple relocated() calls: - // once factory_source_affinity is recorded, all subsequent calls must - // supply the same source so that factory data is relocated correctly. - debug_assert!( - factory_source_affinity.is_none() || *factory_source_affinity == Some(source), - "Caller passed source affinity {source:?} but the factory was originally created from \ - {factory_source_affinity:?}; all calls to relocated() on the same Arc must use the \ - same source affinity to ensure factory data is relocated consistently" - ); - // In case factory source is stored in factory, use that - it means we already transferred the factory // once, so we know the original source affinity. Otherwise, use source as that means this is the first // time we're transferring the Arc, so source is the source affinity of the factory as well.