Use portable_atomic for every 64-bit atomic outside tests, and fail CI on std ones

std::sync::atomic::AtomicU64 exists only on targets with 64-bit atomics.
The 32-bit MIPS targets have none, so these uses stopped the fips crate
building for mips-unknown-linux-musl and mipsel-unknown-linux-musl, even
with the Nostr dependency's own atomics fixed. The transport stats
already used portable_atomic::AtomicU64; the metrics counters, the worker
backpressure counters, the native API, the Nostr runtime and the
profiling code now do too. On targets with 64-bit atomics portable_atomic
uses the std type, so nothing changes there.

No CI leg builds for MIPS, so a new std use would only show up as a
failed router build. check-portable-atomics.py reads the committed src/
tree and flags plain, grouped, nested and renamed imports of AtomicU64 and
AtomicI64, glob imports of std::sync::atomic, path-qualified uses, and uses
reached through a renamed atomic module (use std::sync::atomic as sa, or
{self as sa}). Test-only files are skipped, and the Android BLE file is
exempt because every Android target has 64-bit atomics. It runs in local
CI and in the GitHub CI parity job.
This commit is contained in:
Johnathan Corgan
2026-10-01 14:21:40 +00:00
parent 044d467387
commit 38685e8422
12 changed files with 241 additions and 21 deletions
+2
View File
@@ -77,6 +77,8 @@ jobs:
run: bash testing/check-action-pins.sh run: bash testing/check-action-pins.sh
- name: Check every source comment resolves in-repo - name: Check every source comment resolves in-repo
run: bash testing/check-comment-refs.sh run: bash testing/check-comment-refs.sh
- name: Check no non-test code uses std 64-bit atomics
run: python3 testing/check-portable-atomics.py
# Hermetic: synthetic ping functions, no containers, ~45s. Lives beside # Hermetic: synthetic ping functions, no containers, ~45s. Lives beside
# the other two so both runners gate on it identically — putting it in # the other two so both runners gate on it identically — putting it in
# only one would create exactly the drift check-ci-parity.sh exists to # only one would create exactly the drift check-ci-parity.sh exists to
+2 -1
View File
@@ -10,11 +10,12 @@
//! connection is served by its own spawned task, so two simultaneous `on` //! connection is served by its own spawned task, so two simultaneous `on`
//! requests are genuinely concurrent and must not both create a writer. //! requests are genuinely concurrent and must not both create a writer.
use portable_atomic::AtomicU64;
use std::fs::File; use std::fs::File;
use std::io::Write; use std::io::Write;
use std::path::{Component, Path, PathBuf}; use std::path::{Component, Path, PathBuf};
use std::sync::Mutex; use std::sync::Mutex;
use std::sync::atomic::{AtomicBool, AtomicU8, AtomicU64, Ordering}; use std::sync::atomic::{AtomicBool, AtomicU8, Ordering};
use std::time::{Duration, SystemTime, UNIX_EPOCH}; use std::time::{Duration, SystemTime, UNIX_EPOCH};
use super::recorder; use super::recorder;
+1 -1
View File
@@ -9,8 +9,8 @@
//! `swap(0)`, so there are no "previous value" arrays to carry and the counters //! `swap(0)`, so there are no "previous value" arrays to carry and the counters
//! are per-interval by construction. //! are per-interval by construction.
use portable_atomic::{AtomicU64, Ordering::Relaxed};
use std::sync::LazyLock; use std::sync::LazyLock;
use std::sync::atomic::{AtomicU64, Ordering::Relaxed};
use std::time::{Duration, Instant}; use std::time::{Duration, Instant};
/// Measurement domain. Structural only: one variant today. /// Measurement domain. Structural only: one variant today.
+2 -1
View File
@@ -81,11 +81,12 @@ mod unix_impl {
use crate::config::NativeApiConfig; use crate::config::NativeApiConfig;
use crate::control::protocol::{Request, Response}; use crate::control::protocol::{Request, Response};
use crate::identity::{NodeAddr, decode_npub, encode_npub}; use crate::identity::{NodeAddr, decode_npub, encode_npub};
use portable_atomic::AtomicU64;
use secp256k1::XOnlyPublicKey; use secp256k1::XOnlyPublicKey;
use std::collections::HashMap; use std::collections::HashMap;
use std::os::fd::{AsFd, OwnedFd}; use std::os::fd::{AsFd, OwnedFd};
use std::path::PathBuf; use std::path::PathBuf;
use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::{Arc, Mutex}; use std::sync::{Arc, Mutex};
use tokio::io::BufReader; use tokio::io::BufReader;
use tokio::net::{UnixListener, UnixStream}; use tokio::net::{UnixListener, UnixStream};
+1 -1
View File
@@ -34,7 +34,7 @@ use crate::node::Node;
#[cfg(any(target_os = "linux", target_os = "macos"))] #[cfg(any(target_os = "linux", target_os = "macos"))]
use crate::transport::TransportHandle; use crate::transport::TransportHandle;
#[cfg(any(target_os = "linux", target_os = "macos"))] #[cfg(any(target_os = "linux", target_os = "macos"))]
use std::sync::atomic::{AtomicU64, Ordering::Relaxed}; use portable_atomic::{AtomicU64, Ordering::Relaxed};
#[cfg(any(target_os = "linux", target_os = "macos"))] #[cfg(any(target_os = "linux", target_os = "macos"))]
use tracing::{debug, warn}; use tracing::{debug, warn};
+1 -1
View File
@@ -39,10 +39,10 @@
use crate::NodeAddr; use crate::NodeAddr;
use crate::transport::{TransportAddr, TransportId}; use crate::transport::{TransportAddr, TransportId};
use crossbeam_channel::{Receiver, Sender, TrySendError, bounded}; use crossbeam_channel::{Receiver, Sender, TrySendError, bounded};
use portable_atomic::{AtomicU64, Ordering};
use ring::aead::{Aad, LessSafeKey, Nonce}; use ring::aead::{Aad, LessSafeKey, Nonce};
use std::collections::HashMap; use std::collections::HashMap;
use std::sync::Arc; use std::sync::Arc;
use std::sync::atomic::{AtomicU64, Ordering};
use tokio::sync::mpsc::UnboundedSender; use tokio::sync::mpsc::UnboundedSender;
use tracing::{debug, trace, warn}; use tracing::{debug, trace, warn};
+11 -13
View File
@@ -513,8 +513,7 @@ impl EncryptWorkerPool {
match self.senders[idx].try_push(job) { match self.senders[idx].try_push(job) {
Ok(()) => {} Ok(()) => {}
Err(MacWorkerTryPushError::Full(job)) => { Err(MacWorkerTryPushError::Full(job)) => {
static FULL_COUNT: std::sync::atomic::AtomicU64 = static FULL_COUNT: portable_atomic::AtomicU64 = portable_atomic::AtomicU64::new(0);
std::sync::atomic::AtomicU64::new(0);
let n = FULL_COUNT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); let n = FULL_COUNT.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
if n < 8 || n.is_multiple_of(10000) { if n < 8 || n.is_multiple_of(10000) {
warn!( warn!(
@@ -538,8 +537,7 @@ impl EncryptWorkerPool {
match self.senders[idx].try_send(job) { match self.senders[idx].try_send(job) {
Ok(()) => {} Ok(()) => {}
Err(TrySendError::Full(job)) => { Err(TrySendError::Full(job)) => {
static FULL_COUNT: std::sync::atomic::AtomicU64 = static FULL_COUNT: portable_atomic::AtomicU64 = portable_atomic::AtomicU64::new(0);
std::sync::atomic::AtomicU64::new(0);
let n = FULL_COUNT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); let n = FULL_COUNT.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
if n < 8 || n.is_multiple_of(10000) { if n < 8 || n.is_multiple_of(10000) {
warn!( warn!(
@@ -571,7 +569,7 @@ struct MacSendFlowKey {
#[derive(Default)] #[derive(Default)]
struct MacSequencedSendFlows { struct MacSequencedSendFlows {
flows: Mutex<HashMap<MacSendFlowKey, Arc<MacSequencedSendFlow>>>, flows: Mutex<HashMap<MacSendFlowKey, Arc<MacSequencedSendFlow>>>,
last_prune_ms: std::sync::atomic::AtomicU64, last_prune_ms: portable_atomic::AtomicU64,
} }
#[cfg(target_os = "macos")] #[cfg(target_os = "macos")]
@@ -719,8 +717,8 @@ struct MacSequencedSendFlow {
socket: AsyncUdpSocket, socket: AsyncUdpSocket,
connected_socket: Option<std::sync::Arc<crate::transport::udp::ConnectedPeerSocket>>, connected_socket: Option<std::sync::Arc<crate::transport::udp::ConnectedPeerSocket>>,
dest_addr: SocketAddr, dest_addr: SocketAddr,
next_seq: std::sync::atomic::AtomicU64, next_seq: portable_atomic::AtomicU64,
last_used_ms: std::sync::atomic::AtomicU64, last_used_ms: portable_atomic::AtomicU64,
state: Mutex<MacSendFlowState>, state: Mutex<MacSendFlowState>,
ready_cv: Condvar, ready_cv: Condvar,
space_cv: Condvar, space_cv: Condvar,
@@ -763,8 +761,8 @@ impl MacSequencedSendFlow {
socket, socket,
connected_socket, connected_socket,
dest_addr, dest_addr,
next_seq: std::sync::atomic::AtomicU64::new(0), next_seq: portable_atomic::AtomicU64::new(0),
last_used_ms: std::sync::atomic::AtomicU64::new(now_ms), last_used_ms: portable_atomic::AtomicU64::new(now_ms),
state: Mutex::new(MacSendFlowState::default()), state: Mutex::new(MacSendFlowState::default()),
ready_cv: Condvar::new(), ready_cv: Condvar::new(),
space_cv: Condvar::new(), space_cv: Condvar::new(),
@@ -1386,8 +1384,8 @@ impl SendBackpressurePacer {
return false; return false;
} }
static SEND_BACKPRESSURE_COUNT: std::sync::atomic::AtomicU64 = static SEND_BACKPRESSURE_COUNT: portable_atomic::AtomicU64 =
std::sync::atomic::AtomicU64::new(0); portable_atomic::AtomicU64::new(0);
let n = SEND_BACKPRESSURE_COUNT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); let n = SEND_BACKPRESSURE_COUNT.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
if n < 8 || n.is_multiple_of(100_000) { if n < 8 || n.is_multiple_of(100_000) {
warn!( warn!(
@@ -1493,8 +1491,8 @@ fn default_send_backpressure_drop_after() -> u32 {
#[cfg(all(unix, not(target_os = "linux")))] #[cfg(all(unix, not(target_os = "linux")))]
fn record_udp_send_backpressure_drop(err: &std::io::Error) { fn record_udp_send_backpressure_drop(err: &std::io::Error) {
static SEND_BACKPRESSURE_DROP_COUNT: std::sync::atomic::AtomicU64 = static SEND_BACKPRESSURE_DROP_COUNT: portable_atomic::AtomicU64 =
std::sync::atomic::AtomicU64::new(0); portable_atomic::AtomicU64::new(0);
let n = SEND_BACKPRESSURE_DROP_COUNT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); let n = SEND_BACKPRESSURE_DROP_COUNT.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
if n < 8 || n.is_multiple_of(100_000) { if n < 8 || n.is_multiple_of(100_000) {
warn!( warn!(
+1 -1
View File
@@ -11,7 +11,7 @@
//! The remaining families (session, handshake, mmp, transport) stay on //! The remaining families (session, handshake, mmp, transport) stay on
//! `NodeStats`. //! `NodeStats`.
use std::sync::atomic::{AtomicU64, Ordering}; use portable_atomic::{AtomicU64, Ordering};
use crate::cache::HintOutcome; use crate::cache::HintOutcome;
use crate::node::reject::{BloomReject, DiscoveryReject, ForwardingReject, TreeReject}; use crate::node::reject::{BloomReject, DiscoveryReject, ForwardingReject, TreeReject};
+2 -1
View File
@@ -1,7 +1,8 @@
use portable_atomic::AtomicU64;
use std::collections::{HashMap, HashSet}; use std::collections::{HashMap, HashSet};
use std::net::SocketAddr; use std::net::SocketAddr;
use std::sync::Arc; use std::sync::Arc;
use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::atomic::{AtomicBool, Ordering};
use std::time::{Duration, Instant}; use std::time::{Duration, Instant};
use nostr::nips::nip17; use nostr::nips::nip17;
+1 -1
View File
@@ -36,8 +36,8 @@
//! * `FMP_WORKER_QUEUE_WAIT` — rx_loop FMP job dispatch → worker //! * `FMP_WORKER_QUEUE_WAIT` — rx_loop FMP job dispatch → worker
//! * `ENDPOINT_EVENT_WAIT` — rx_loop endpoint delivery → endpoint recv //! * `ENDPOINT_EVENT_WAIT` — rx_loop endpoint delivery → endpoint recv
use portable_atomic::{AtomicU64, Ordering::Relaxed};
use std::sync::OnceLock; use std::sync::OnceLock;
use std::sync::atomic::{AtomicU64, Ordering::Relaxed};
use std::time::Instant; use std::time::Instant;
/// Number of measurement buckets. Indices match `Stage`. /// Number of measurement buckets. Indices match `Stage`.
+206
View File
@@ -0,0 +1,206 @@
#!/usr/bin/env python3
"""Fail when non-test code under src/ uses the std 64-bit atomics.
`std::sync::atomic::AtomicU64` and `AtomicI64` exist only on targets with
64-bit atomics. The 32-bit MIPS targets the OpenWrt packages are meant to
cover (mips-unknown-linux-musl, mipsel-unknown-linux-musl) have none, so one
such use anywhere in the crate stops the whole crate building there. Code
that needs a 64-bit atomic uses `portable_atomic::AtomicU64` instead, which is
the std type where the target has one and a lock-based fallback where it does
not. No CI leg builds for MIPS yet, so without this check the first anyone
would hear of a new std use is a failed build on a router target.
What is flagged, read from the files committed at HEAD:
* a `use` tree rooted at `std` or `core` that reaches
`sync::atomic::AtomicU64` or `sync::atomic::AtomicI64`, in any form:
a plain path, a grouped `{...}` list over one or several lines, a nested
group such as `std::sync::{Arc, atomic::{AtomicU64, Ordering}}`, or a
rename with `as`;
* a glob import of `std::sync::atomic::*` or `core::sync::atomic::*`,
because it brings both types in unnamed;
* a path-qualified use such as `std::sync::atomic::AtomicU64::new(0)`, and
`atomic::AtomicU64` after `use std::sync::atomic;`, anywhere in the text;
* `sa::AtomicU64` where the file renames the module with
`use std::sync::atomic as sa;` or `use std::sync::{atomic::{self as sa}}`.
Scope is src/, minus files under a `tests/` directory and files named
`tests.rs` or `*_tests.rs`, which are only ever built for the host. A
`#[cfg(test)]` module inside an ordinary file is NOT exempt: telling it apart
needs a Rust parser, and holding test code in those files to the same rule
costs nothing today (no such module uses the std types).
One file is exempt by name, with its reason: src/transport/ble/io_android.rs
is compiled only for Android, and every Android target Rust supports has
64-bit atomics.
Known gaps, recorded rather than discovered: a std 64-bit atomic reached
through a re-export from another crate, or through a type alias defined
outside src/, is not seen; nor is one written with a macro that assembles the
path from pieces.
Exit codes:
0 - no non-test file under src/ uses a std 64-bit atomic
1 - at least one does; every hit is printed
2 - the check could not look (not a git work tree, git failed, or src/
matched no Rust files); never a pass
"""
from __future__ import annotations
import re
import subprocess
import sys
EXEMPT = {
"src/transport/ble/io_android.rs": "Android targets all have 64-bit atomics",
}
WIDE = ("AtomicU64", "AtomicI64")
USE_RE = re.compile(r"\buse\s+([^;]+);", re.S)
# Any `atomic::AtomicU64` whose `atomic` is not the tail of a longer name, so
# `portable_atomic::AtomicU64` is not matched and every std spelling is.
PATH_RE = re.compile(r"(?<![\w])atomic\s*::\s*(AtomicU64|AtomicI64)\b")
def git(*args: str) -> str:
"""Run a git command and return its stdout, exiting 2 if it fails."""
proc = subprocess.run(["git", *args], capture_output=True, text=True)
if proc.returncode != 0:
print(f"check-portable-atomics: git {' '.join(args)} failed: {proc.stderr.strip()}",
file=sys.stderr)
sys.exit(2)
return proc.stdout
def is_test_path(path: str) -> bool:
"""True for files only ever built as part of the test harness."""
parts = path.split("/")
name = parts[-1]
return "tests" in parts[:-1] or name == "tests.rs" or name.endswith("_tests.rs")
def split_top(text: str) -> list[str]:
"""Split a use-tree list on the commas that are not inside braces."""
items, depth, cur = [], 0, []
for ch in text:
if ch == "{":
depth += 1
elif ch == "}":
depth -= 1
if ch == "," and depth == 0:
items.append("".join(cur))
cur = []
else:
cur.append(ch)
items.append("".join(cur))
return [i.strip() for i in items if i.strip()]
def flatten(tree: str, prefix: tuple[str, ...] = ()) -> list[tuple[str, ...]]:
"""Expand a use tree into the full paths it imports.
A rename, written `name@alias` by the caller, stays on the last segment.
"""
tree = re.sub(r"\s+", "", tree)
brace = tree.find("{")
if brace == -1:
segs = tuple(s for s in tree.split("::") if s)
return [prefix + segs]
head = tuple(s for s in tree[:brace].split("::") if s)
inner = tree[brace + 1:tree.rfind("}")]
out = []
for item in split_top(inner):
if item == "self" or item.startswith("self@"):
alias = item[len("self"):]
out.append(prefix + head[:-1] + (head[-1] + alias,))
else:
out.extend(flatten(item, prefix + head))
return out
def use_hits(text: str) -> list[tuple[int, str]]:
"""Return (line, imported path) for each std/core wide-atomic import."""
hits = []
for m in USE_RE.finditer(text):
body = re.sub(r"\s+as\s+(\w+)", r"@\1", m.group(1))
line = text.count("\n", 0, m.start()) + 1
for path in flatten(body):
if not path:
continue
last, _, alias = path[-1].partition("@")
path = path[:-1] + (last,)
if path[0] not in ("std", "core") or path[1:3] != ("sync", "atomic"):
continue
if len(path) == 3 and alias:
hits.extend(alias_hits(text, alias, "::".join(path)))
elif len(path) >= 4 and (path[3] in WIDE or path[3] == "*"):
hits.append((line, "::".join(path[:4])))
return hits
def alias_hits(text: str, alias: str, module: str) -> list[tuple[int, str]]:
"""Return (line, use) for each wide atomic named through a module alias."""
hits = []
pat = re.compile(r"(?<![\w])" + re.escape(alias) + r"\s*::\s*(AtomicU64|AtomicI64)\b")
for m in pat.finditer(text):
line = text.count("\n", 0, m.start()) + 1
hits.append((line, f"{module}::{m.group(1)} as {alias}::{m.group(1)}"))
return hits
def path_hits(text: str) -> list[tuple[int, str]]:
"""Return (line, matched text) for each path-qualified wide atomic."""
hits = []
for m in PATH_RE.finditer(text):
line = text.count("\n", 0, m.start()) + 1
hits.append((line, re.sub(r"\s+", "", m.group(0))))
return hits
def main() -> int:
"""Scan the committed src/ tree and report every std wide-atomic use."""
root = git("rev-parse", "--show-toplevel").strip()
if not root:
print("check-portable-atomics: empty work-tree root", file=sys.stderr)
return 2
files = [
f for f in git("-C", root, "ls-tree", "-r", "--name-only", "HEAD", "--", "src/").splitlines()
if f.endswith(".rs")
]
if not files:
print("check-portable-atomics: src/ matched no Rust files at HEAD", file=sys.stderr)
return 2
scanned = 0
findings = []
for path in files:
if is_test_path(path) or path in EXEMPT:
continue
text = git("-C", root, "show", f"HEAD:{path}")
scanned += 1
seen = set()
for line, what in use_hits(text) + path_hits(text):
if (line, what) not in seen:
seen.add((line, what))
findings.append(f"{path}:{line}: {what}")
if scanned == 0:
print("check-portable-atomics: every file under src/ was excluded", file=sys.stderr)
return 2
if findings:
for f in findings:
print(f)
print("", file=sys.stderr)
print("check-portable-atomics: std 64-bit atomics do not exist on 32-bit MIPS, so the",
file=sys.stderr)
print("crate stops building there. Use portable_atomic::AtomicU64 (or AtomicI64).",
file=sys.stderr)
return 1
return 0
if __name__ == "__main__":
sys.exit(main())
+11
View File
@@ -1554,6 +1554,16 @@ run_comment_refs() {
record "comment-refs" $rc record "comment-refs" $rc
} }
# No non-test code may use std's 64-bit atomics. They do not exist on 32-bit
# MIPS, so one such use stops the crate building for the OpenWrt MIPS targets,
# and no leg builds for MIPS to notice. Static, and it needs nothing built.
run_portable_atomics() {
local rc=0
info "[portable-atomics] Checking that no non-test code uses std 64-bit atomics"
python3 "$SCRIPT_DIR/check-portable-atomics.py" || rc=$?
record "portable-atomics" $rc
}
# Every daemon log string a test matches on must still be emitted by src/. # Every daemon log string a test matches on must still be emitted by src/.
# A stale one does not fail — it stops observing, and an expect-zero assertion # A stale one does not fail — it stops observing, and an expect-zero assertion
# built on it then passes for the wrong reason. # built on it then passes for the wrong reason.
@@ -1636,6 +1646,7 @@ main() {
run_image_scoping run_image_scoping
run_action_pins run_action_pins
run_comment_refs run_comment_refs
run_portable_atomics
run_wait_converge run_wait_converge
run_deb_version run_deb_version
run_nextest_flaky run_nextest_flaky