From 84766517fffd7e664a332ccdfdc182774f8ac4cb Mon Sep 17 00:00:00 2001 From: zackees Date: Sun, 23 Aug 2026 10:36:53 -0700 Subject: [PATCH] fix(dylint): give each boundary-lint process a unique observation identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Dylint (windows)` failed on an fbuild-cli-only change with: platform-boundary: actual Dylint observations disagree for pid=2636 source=crates/fbuild-toolchain/src/toolchain/esp_qemu.rs: expected={('attr_cfg', 'windows'): 6} actual={('attr_cfg', 'windows'): 12} Nothing in that PR touched fbuild-toolchain, both counts were exactly double, and the Linux leg of the same commit was green. `cargo dylint --all-targets` runs one driver process per crate-target, so a crate's lib and lib-test targets compile the same sources twice. Each process stamped its observations with `std::process::id()` alone — and Windows hands out a recycled PID readily enough that the second process can be given the first one's number after it exits. The checker keys findings by that PID, so two runs merged into one bucket and every count for the shared sources read double. The same collision on Linux is rare enough that the gate looked stable. Replayed against a real observation file, collapsing every identity to a single PID reproduces the CI failure exactly — same two files, same 6 -> 12 and 3 -> 6: awk -F'\t' 'BEGIN{OFS="\t"} {$1="2636"; print}' observed.tsv > collide.tsv uv run --no-project python ci/enforce_platform_boundary.py \ --dylint-observed collide.tsv The identity is now `-`, taken once per process. Reusing a PID requires the original holder to have exited first, so two processes cannot share a start nanosecond. The checker needs no change: it already treats the first field as an opaque key. Crate version bumped so the cached `.so` is rebuilt with the new emitter. Co-Authored-By: Claude Opus 5 (1M context) --- dylints/enforce_platform_boundary/Cargo.lock | 2 +- dylints/enforce_platform_boundary/Cargo.toml | 2 +- dylints/enforce_platform_boundary/src/lib.rs | 30 ++++++++++++++++---- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/dylints/enforce_platform_boundary/Cargo.lock b/dylints/enforce_platform_boundary/Cargo.lock index 97d7cee5..3ded4772 100644 --- a/dylints/enforce_platform_boundary/Cargo.lock +++ b/dylints/enforce_platform_boundary/Cargo.lock @@ -300,7 +300,7 @@ dependencies = [ [[package]] name = "enforce_platform_boundary" -version = "0.1.1" +version = "0.1.2" dependencies = [ "dylint_linting", "dylint_testing", diff --git a/dylints/enforce_platform_boundary/Cargo.toml b/dylints/enforce_platform_boundary/Cargo.toml index 62eaf59c..c46d7603 100644 --- a/dylints/enforce_platform_boundary/Cargo.toml +++ b/dylints/enforce_platform_boundary/Cargo.toml @@ -2,7 +2,7 @@ name = "enforce_platform_boundary" # Bump whenever the embedded baseline changes: setup-soldr's Dylint cache # hashes the manifest but not src/baseline.txt. -version = "0.1.1" +version = "0.1.2" description = "Enforce fbuild's single host-platform boundary" edition = "2021" publish = false diff --git a/dylints/enforce_platform_boundary/src/lib.rs b/dylints/enforce_platform_boundary/src/lib.rs index 164c61e2..f0f5d0dc 100644 --- a/dylints/enforce_platform_boundary/src/lib.rs +++ b/dylints/enforce_platform_boundary/src/lib.rs @@ -130,6 +130,29 @@ static COUNTS: LazyLock>> = LazyLock::new(|| Mutex::new( static OBSERVED_SOURCES: LazyLock>> = LazyLock::new(|| Mutex::new(HashSet::new())); +/// Identity stamped on every observation line so the checker can tell one +/// driver process's findings from another's. +/// +/// A bare PID is not that identity. `cargo dylint --all-targets` runs one +/// driver process per crate-target, so a crate's lib and lib-test targets +/// compile the same sources twice — and Windows reuses PIDs aggressively +/// enough that the second process can be handed the first one's number once +/// it has exited. The two runs' findings then merge under a single key and +/// every count for the shared sources reads exactly double, which surfaced +/// as `expected={('attr_cfg', 'windows'): 6} actual={...: 12}` on +/// `fbuild-toolchain` while the Linux leg of the same commit was green. +/// +/// Appending a nanosecond stamp taken once per process separates them: +/// reusing a PID requires the original holder to have exited first, so the +/// two processes cannot have started in the same nanosecond. +static PROCESS_IDENTITY: LazyLock = LazyLock::new(|| { + let nonce = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0); + format!("{}-{nonce:x}", std::process::id()) +}); + fn append_observation(line: &str) { let Some(path) = std::env::var(OBSERVATION_PATH_ENV) .ok() @@ -156,7 +179,7 @@ fn observe_source(context: &EarlyContext<'_>, span: Span) { .lock() .expect("platform lint source counter poisoned"); if sources.insert(path.clone()) { - append_observation(&format!("{}\t{path}\tsource_seen\t-\n", std::process::id())); + append_observation(&format!("{}\t{path}\tsource_seen\t-\n", *PROCESS_IDENTITY)); } } @@ -250,10 +273,7 @@ fn record(context: &EarlyContext<'_>, span: Span, kind: Kind, normalized: &str) let key = (path, kind.as_str().to_owned(), normalized.to_owned()); append_observation(&format!( "{}\t{}\t{}\t{}\n", - std::process::id(), - key.0, - key.1, - key.2 + *PROCESS_IDENTITY, key.0, key.1, key.2 )); let mut counts = COUNTS.lock().expect("platform lint counter poisoned"); if exceeds_baseline(&BASELINE.counts, &mut counts, &key) {