Suppress fence-synchronised dependency reports by default

CHANGELOG.md

@@ -31,6 +31,14 @@
   exits `3` ("pytest stopped after X of N collected tests"). A JUnit file left by an
   earlier run is removed first, so a session that dies before writing one is never read
   with the previous run's counts.
+- **Reports inside dependencies' fence-based synchronisation no longer fail runs.**
+  TSan does not model standalone fences, and cannot see inside glibc. The default
+  suppressions now cover `oneshot`'s message handover, crossbeam-epoch's reclamation
+  (0.9.18 and older) and glibc freeing a finished thread's TLS block
+  (`_dl_deallocate_tls`); each was filed as a race "in your extension". Each entry names
+  only the dependency's own function, so races in your code that the dependency runs (a
+  message's `Drop`, a deferred closure, a thread-local destructor) are still reported.
+  Pinned by `clean/clean-dependency-fences` and `racy/race-in-dependency-callbacks`.
 
 ### Added
 

docs/ci.md

@@ -163,12 +163,23 @@ killed after a short grace period and the crash is still reported.
 
 Suppressions, applied in this order:
 
-1. **ftcheck's own** (`ftcheck/ci/suppressions/rust.supp`) — known deliberate races in
-   common Rust dependencies, each justified by that dependency's own source and written
-   as `race_top:`, so a real race in your code that merely runs inside the dependency is
-   still reported. Today: crossbeam-deque's buffer, as used by rayon's job queue. The
-   `clean/clean-rayon` fixture proves both that it is needed (without it: exit 1) and that
-   it holds. `FTCHECK_NO_DEFAULT_SUPPRESSIONS=1` turns them off, to audit what they hide.
+1. **ftcheck's own** (`ftcheck/ci/suppressions/rust.supp`) — reports inside common
+   Rust dependencies that are not races, each justified by that dependency's own source:
+   - crossbeam-deque's buffer, as used by rayon's job queue — a documented deliberate race;
+   - `oneshot`'s message handover, ordered by a standalone `fence(Acquire)`;
+   - crossbeam-epoch's reclamation (0.9.18 and older), ordered by `SeqCst` fences;
+   - glibc freeing a finished thread's TLS block (`_dl_deallocate_tls`), ordered inside
+     glibc, which is not instrumented.
+
+   Entries are `race_top:` where TSan allows, matching only the frame where the access
+   happened. Where that frame is an interceptor (`free`, `memcpy`) they are `race:` on a
+   function of the dependency that runs none of your code. Either way, a real race in
+   your code that runs inside the dependency — a message's `Drop` run by `oneshot`, a
+   closure deferred to crossbeam-epoch, a thread-local destructor — is still reported.
+   `clean/clean-rayon` and `clean/clean-dependency-fences` prove the entries are needed
+   (without them: exit 1) and that they hold; `racy/race-in-dependency-callbacks` proves
+   they stop at the dependency's code. `FTCHECK_NO_DEFAULT_SUPPRESSIONS=1` turns them
+   off, to audit what they hide.
 2. **The image's CPython list** (`/work/tsan_suppressions/cpython.txt`), through
    `$FTCHECK_TSAN_SUPPRESSIONS`.
 3. **Yours**, with `--suppressions FILE`.

docs/limitations.md

@@ -60,10 +60,21 @@ Known gaps, each still open:
   single-threaded baseline never sees the mutator's transient state. The finding says
   when mutators were running, but does not check whether one caused it. A mutator must
   keep the shared inputs valid at every instant.
-- **Some dependency-internal TSan reports still fail runs.** Beyond the suppressed
-  crossbeam-deque race, reports inside dependencies' fence-based synchronisation (which
-  TSan does not model) and glibc's thread-local teardown can be filed as "in your
-  extension" with no frame of yours on either access.
+- **Other dependencies' fence-based synchronisation can still fail runs.** TSan does
+  not model standalone fences. ftcheck suppresses the reports it knows of in common
+  dependencies — crossbeam-deque, `oneshot`, crossbeam-epoch 0.9.18 and older, and
+  glibc's thread-local teardown (see [ci.md](ci.md#attribution-yours-reached-from-yours-or-not-yours)) — but a dependency
+  that synchronises the same way and is not on that list is still filed as "in your
+  extension". Add a `--suppressions` file for it, scoped to the dependency's frames.
+- **The glibc teardown suppression can hide a use-after-free.** The report's own frame
+  is the `free` interceptor, so the entry matches `_dl_deallocate_tls` anywhere on the
+  stack. A genuine race on a finished thread's thread-local, reached by another thread
+  through a pointer that escaped it, is hidden with the false report. Safe Rust cannot
+  express that; `unsafe` code can.
+- **Your own accesses ordered only by such a fence are reported.** Data you write before
+  a `oneshot` send and read after the receive is ordered by the channel's fence, which
+  TSan cannot see, so the access looks unordered in your code. Suppressing it would hide
+  real races, so ftcheck does not.
 
 ## The lint on public projects
 

fixtures/clean/clean-dependency-fences/Cargo.toml

@@ -0,0 +1,17 @@
+[package]
+name = "clean-dependency-fences"
+version = "0.0.0"
+edition = "2021"
+publish = false
+
+[lib]
+name = "clean_dependency_fences"
+crate-type = ["cdylib", "rlib"]
+
+[dependencies]
+pyo3 = { version = "0.29", features = ["extension-module"] }
+oneshot = "0.1"
+# Pinned to the last release before crossbeam-epoch modelled its fences for
+# ThreadSanitizer (0.9.19 and 0.9.21 did, under `crossbeam_sanitize_thread`).
+# Projects that lock an older release still get these reports.
+crossbeam-epoch = "=0.9.18"

fixtures/clean/clean-dependency-fences/expected.toml

@@ -0,0 +1,3 @@
+race = false
+rules = []
+justification = "Values handed over by oneshot, memory reclaimed by crossbeam-epoch 0.9.18, and short-lived threads detached after they finish using a thread-local with a destructor, all called from many Python threads, each call on its own data. Every access is ordered, by fences TSan does not model (oneshot, crossbeam-epoch) or inside uninstrumented glibc (the finished thread's TLS block freed in _dl_deallocate_tls). Without ftcheck's default suppressions TSan reports all three; this fixture pins that the suppressions hold."

fixtures/clean/clean-dependency-fences/ftcheck.toml

@@ -0,0 +1,5 @@
+# Small counts: every call spawns threads, and the stress budget is shared.
+[stress.args]
+"clean_dependency_fences.oneshot_roundtrip" = ["4", "16"]
+"clean_dependency_fences.epoch_churn" = ["2", "4"]
+"clean_dependency_fences.short_lived_threads" = ["2", "8"]

fixtures/clean/clean-dependency-fences/src/lib.rs

@@ -0,0 +1,127 @@
+//! Correct: three patterns whose synchronisation ThreadSanitizer cannot see.
+//!
+//! Every access below is ordered, but not in a way TSan models, so without
+//! ftcheck's default suppressions each one is reported as a data race:
+//!
+//! - `oneshot` hands the message over with a relaxed load of the channel state
+//!   followed by a standalone `fence(Acquire)`. TSan does not model standalone
+//!   fences, so the sender's write of the message and the receiver's read (and
+//!   its free of the channel) look unordered.
+//! - `crossbeam-epoch` reclaims memory once every pinned thread has moved past
+//!   an epoch, which it proves with `SeqCst` fences. TSan sees the collector
+//!   free a garbage bag, or a finished thread's `Local`, that another thread
+//!   read without a happens-before edge.
+//! - glibc frees a finished, detached thread's TLS block in `_dl_deallocate_tls`,
+//!   on the thread that drops the handle. glibc is not instrumented, so nothing
+//!   TSan sees orders the finished thread's own thread-local accesses before
+//!   that free.
+//!
+//! None of it is the extension's code. Each call works on its own data.
+
+use crossbeam_epoch::{self as epoch, Atomic, Owned};
+use pyo3::prelude::*;
+use std::cell::RefCell;
+use std::sync::atomic::Ordering::AcqRel;
+use std::sync::{mpsc, Arc};
+use std::thread;
+use std::time::Duration;
+
+/// Large enough that moving it through the channel is a `memcpy`.
+type Payload = [u64; 32];
+
+/// A worker thread sends one value back; the caller polls, then blocks.
+#[pyfunction]
+fn oneshot_roundtrip(py: Python<'_>, n: usize) -> u64 {
+    py.detach(|| {
+        let mut total = 0;
+        for i in 0..n {
+            let (tx, rx) = oneshot::channel::<Payload>();
+            let worker = thread::spawn(move || tx.send([i as u64; 32]).unwrap());
+            let value = if i % 2 == 0 {
+                // try_recv: relaxed load, then fence(Acquire) once it sees MESSAGE.
+                loop {
+                    match rx.try_recv() {
+                        Ok(v) => break v,
+                        Err(oneshot::TryRecvError::Empty) => thread::yield_now(),
+                        Err(e) => panic!("{e}"),
+                    }
+                }
+            } else {
+                rx.recv().unwrap()
+            };
+            worker.join().unwrap();
+            total += value[31];
+        }
+        total
+    })
+}
+
+/// Threads swap a shared pointer and retire the old value through the epoch
+/// collector, then exit, so their `Local`s are reclaimed by whoever collects.
+#[pyfunction]
+fn epoch_churn(py: Python<'_>, n: usize) -> usize {
+    py.detach(|| {
+        let shared = Arc::new(Atomic::new([0u64; 8]));
+        let workers: Vec<_> = (0..n)
+            .map(|t| {
+                let shared = Arc::clone(&shared);
+                thread::spawn(move || {
+                    for i in 0..64u64 {
+                        let guard = epoch::pin();
+                        let old = shared.swap(Owned::new([t as u64 ^ i; 8]), AcqRel, &guard);
+                        // SAFETY: `old` is unlinked, and only reachable by
+                        // threads pinned before the swap.
+                        unsafe { guard.defer_destroy(old) };
+                    }
+                    epoch::pin().flush();
+                })
+            })
+            .collect();
+        let count = workers.len();
+        for w in workers {
+            w.join().unwrap();
+        }
+        let guard = epoch::pin();
+        // SAFETY: every worker has been joined; nothing else holds the pointer.
+        unsafe { drop(shared.swap(epoch::Shared::null(), AcqRel, &guard).into_owned()) };
+        count
+    })
+}
+
+thread_local! {
+    static SCRATCH: RefCell<Vec<u64>> = const { RefCell::new(Vec::new()) };
+}
+
+/// Short-lived threads that use a thread-local with a destructor. Each handle is
+/// dropped once its thread has finished, which detaches it: glibc then frees the
+/// finished thread's TLS block here, in `_dl_deallocate_tls`.
+#[pyfunction]
+fn short_lived_threads(py: Python<'_>, n: usize) -> u64 {
+    py.detach(|| {
+        let (done, finished) = mpsc::channel();
+        let workers: Vec<_> = (0..n)
+            .map(|i| {
+                let done = done.clone();
+                thread::spawn(move || {
+                    let sum = SCRATCH.with(|s| {
+                        s.borrow_mut().push(i as u64);
+                        s.borrow().iter().sum::<u64>()
+                    });
+                    done.send(sum).unwrap();
+                })
+            })
+            .collect();
+        let total = (0..n).map(|_| finished.recv().unwrap()).sum();
+        // Give each thread time to run its TLS destructors and exit.
+        thread::sleep(Duration::from_millis(20));
+        drop(workers);
+        total
+    })
+}
+
+#[pymodule]
+fn clean_dependency_fences(m: &Bound<'_, PyModule>) -> PyResult<()> {
+    m.add_function(wrap_pyfunction!(oneshot_roundtrip, m)?)?;
+    m.add_function(wrap_pyfunction!(epoch_churn, m)?)?;
+    m.add_function(wrap_pyfunction!(short_lived_threads, m)?)
+}

fixtures/clean/clean-dependency-fences/tests/test_threads.py

@@ -0,0 +1,9 @@
+"""Run by `ftcheck ci` under pytest-run-parallel: this body runs in N threads at once."""
+import clean_dependency_fences as m
+
+
+def test_dependency_synchronisation_from_many_threads():
+    for _ in range(5):
+        assert m.oneshot_roundtrip(8) == sum(range(8))
+        assert m.epoch_churn(4) == 4
+        assert m.short_lived_threads(4) == sum(range(4))