crates/gunnar-pack/tests/serve_runs_inline.rs
//! **`P-020`/`P-004`: a served request must not produce a pack entry on a
//! thread of its own.**
//!
//! # What this guards, and why a thread count could not
//!
//! `P-004` is DECIDED — "the serving tier stays serial" — and
//! `crates/gunnar-pack/src/copy.rs` spells that out as
//! `thread_limit: Some(1)`, which `xtask/tests/pack_pipeline.rs` fails the
//! build over if it is ever dropped. That guard checks the **declaration**.
//! `gix_parallel::Stepwise::new` does not keep it:
//! it spawns `num_threads(thread_limit)` consumers *plus* an unconditional
//! input feeder, neither of them conditional, so `Some(1)` produces **two**
//! threads and every chunk of pack entries crosses two channels to reach the
//! writer. MEASURED with `strace -f -c -e trace=clone3` over one served clone:
//! 2 with stock `gix-features`, 0 with `gunnar/vendor/gix-features`.
//!
//! The obvious guard — sample `/proc/self/status`'s `Threads:` and assert it
//! does not rise — is the one this deliberately does not write. It is a
//! *sampling* instrument against a thread that lives for the length of one
//! pack write, so on a small fixture it can miss the spawn entirely and report
//! green; and `Threads:` is process-wide, so any unrelated pool in the test
//! binary moves it. A guard that can be green while the defect is present is
//! the failure mode this repository has found roughly twenty of.
//!
//! So this asserts properties directly and deterministically instead:
//! **every `entry_by_location` call a served pack makes, and every pack-aware
//! handle it opens, is accounted for by thread.**
//!
//! # ⚠ The fix lives in a fork this workspace deliberately does not carry
//!
//! The inline `Stepwise` is `gunnar/vendor/gix-features` fix 2 (gitoxide issue
//! **2881**, filed 2026-08-05, still unmerged). It is reached through
//! `[patch.crates-io]` in **gunnar's** two workspace roots.
//!
//! edda had that same table and **dropped it in `cbe0fb8` (2026-08-23)**: a
//! `[patch.crates-io]` pointing at a path this repository does not produce reads
//! as a foreign fork to the release gate, and it held `gunnar-pack` and
//! `gunnar-delta` off crates.io, and they held `gunnar-client` and therefore
//! `dwarves`. That was a deliberate product call and it is not reversed here.
//!
//! The consequence nobody wrote down is that **this test then asserted a
//! property this workspace cannot supply**, and it has been red ever since —
//! "72 of 73 pack entries were produced off the calling thread" — which is
//! exactly a guard that has stopped being able to report the thing it exists
//! for. A permanently red test is a test nobody reads.
//!
//! # What it asserts now, and why this is MORE and not less
//!
//! The pipeline shape is discovered through a **different door** than the one
//! under test — [`stepwise_is_inline`] drives a bare
//! `gix_parallel::Stepwise` with `thread_limit: Some(1)` and
//! one item, and looks at where `consume` ran. It reads no manifest, no
//! lockfile, and nothing `write_pack_copy` touches. Then **both** branches
//! assert an exact shape, so neither is the loose one:
//!
//! * **fork in the graph** (gunnar's build, or edda with the table restored) —
//! every lookup on the calling thread, and **one** pack-aware handle opened
//! for the whole request.
//! * **stock `gix-features`** (edda today) — every generator lookup on
//! **exactly one** other thread, never two, and **two** handles opened. That
//! second handle is the budget cost the two threads actually carry: gix
//! clones the consume closure per worker and each clone opens its own
//! `gix-odb` handle with its own mapped packs, which on the appliance is a
//! per-request fd and mapping the caller did not ask for.
//!
//! Both branches also share one **unconditional** assertion the old test did
//! not have: `thread_limit: Some(1)` must bound the pipeline to **at most one**
//! thread besides the caller. That is the `P-004` claim that survives the fork
//! being absent, and it is the one that fires if `Some(1)` is ever widened —
//! which the old thread-identity assertion could not distinguish from the
//! `Stepwise` spawn it was already failing on.
//!
//! The probe cannot quietly be wrong, because each branch is an equality: if
//! it said "inline" against a threaded graph the strict branch would fire, and
//! if it said "threaded" against an inline graph the count of off-thread
//! lookups would be 0 where the branch demands all of them.
//!
//! # The numbers, MEASURED on oden 2026-08-30 on this fixture
//!
//! Same test binary, same 72 pack-resident objects over one pack, the only
//! difference being whether `[patch.crates-io] gix-features` is present:
//!
//! | | lookups off the calling thread | handles opened per request |
//! |---|---|---|
//! | `gunnar/vendor/gix-features` (fix 2) | **0** of 73 | **2** |
//! | stock `gix-features 0.49.1` | **72** of 73 | **3** |
//!
//! The handle column is new and it is the concrete budget cost. gix clones the
//! consume closure once per worker, and `PackedObjects::clone` opens a fresh
//! `gix-odb` handle each time, so the two spawned threads are worth **one extra
//! open, mapped copy of the repository's packs per served request** — an fd and
//! a mapping nobody budgeted, on a box that announces `transfers 64`. The thread
//! spawns themselves were already measured at ~64 µs and process CPU per request
//! is flat; the handle is the part that scales with concurrency.
//!
//! Seen RED, edda, 2026-08-30, three ways:
//!
//! * `thread_limit` widened from `Some(1)` to `None` in `src/copy.rs`: the
//! unconditional width assertion fires — "72 pack entries were produced
//! across 7 threads besides the caller". The old assertion could not report
//! this, because on this workspace it was already failing on the `Stepwise`
//! spawn and said the same sentence either way.
//! * the probe forced to `true` against a stock graph: the strict branch fires
//! at 72 of 73. A wrong probe is not a free pass.
//! * the `[patch.crates-io]` table restored and the probe forced to `false`:
//! the stock branch's equality fires at 0.
mod common;
use std::collections::HashSet;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::{Arc, Mutex};
use common::{churned_repo, git_available};
use gix_hash::{Kind, ObjectId};
use gunnar_pack::{write_pack_copy, PackedObjects, WriteOptions};
/// A `gix_pack::Find` that forwards everything and writes down which thread
/// asked for each pack entry.
///
/// Modelled on `tests/pack_extents.rs`'s `Duplicated`, which wraps the same
/// trait at the same seam (LAW 5); the only method with a body of its own is
/// [`entry_by_location`](gix_pack::Find::entry_by_location), because that is
/// the one `iter_from_counts` calls from inside `Stepwise`'s consumer.
struct ThreadWitness {
inner: Box<dyn gix_pack::Find + Send>,
seen: Arc<Mutex<Vec<std::thread::ThreadId>>>,
}
impl gix_pack::Find for ThreadWitness {
fn contains(&self, id: &gix_hash::oid) -> bool {
self.inner.contains(id)
}
fn try_find_cached<'a>(
&self,
id: &gix_hash::oid,
buffer: &'a mut Vec<u8>,
cache: &mut dyn gix_pack::cache::DecodeEntry,
) -> gix_error::Result<
Option<(
gix_object::Data<'a>,
Option<gix_pack::data::entry::Location>,
)>
> {
self.inner.try_find_cached(id, buffer, cache)
}
fn location_by_oid(
&self,
id: &gix_hash::oid,
buf: &mut Vec<u8>,
) -> gix_error::Result<Option<gix_pack::data::entry::Location>> {
self.inner.location_by_oid(id, buf)
}
fn pack_offsets_and_oid(
&self,
pack_id: u32,
) -> gix_error::Result<Option<Vec<(gix_pack::data::Offset, ObjectId)>>> {
self.inner.pack_offsets_and_oid(pack_id)
}
fn entry_by_location(
&self,
location: &gix_pack::data::entry::Location,
) -> Option<gix_pack::find::Entry> {
if let Ok(mut seen) = self.seen.lock() {
seen.push(std::thread::current().id());
}
self.inner.entry_by_location(location)
}
}
/// Is `gix_parallel::Stepwise` the inline one?
///
/// **A different door from the one under test.** This drives `Stepwise`
/// directly — one item, `thread_limit: Some(1)`, an identity reducer — and
/// reports whether `consume` ran on the calling thread. It does not go through
/// `write_pack_copy`, `iter_from_counts`, a manifest or a lockfile, which is
/// the point: `gunnar/xtask/src/vendor_patch.rs` already went blind for two
/// days reading a `Cargo.lock` that no longer described what cargo would
/// resolve, and a guard that asks the same file the defect edits is not a
/// second opinion.
///
/// `false` for stock `gix-features` (the consumer thread runs `consume`),
/// `true` for `gunnar/vendor/gix-features` fix 2.
fn stepwise_is_inline() -> bool {
use gix_parallel::{IdentityWithResult, Stepwise};
let here = std::thread::current().id();
let seen: Arc<Mutex<Vec<std::thread::ThreadId>>> = Arc::new(Mutex::new(Vec::new()));
let mut steps = {
let seen = Arc::clone(&seen);
Stepwise::new(
std::iter::once(0usize),
Some(1),
|_thread_id: usize| (),
move |_item: usize, _state: &mut ()| {
if let Ok(mut seen) = seen.lock() {
seen.push(std::thread::current().id());
}
Ok::<(), ()>(())
},
IdentityWithResult::<(), ()>::default(),
)
};
while steps.next().is_some() {}
drop(steps);
let seen = seen.lock().expect("the probe witness was not poisoned");
assert_eq!(
seen.len(),
1,
"the probe's own `consume` did not run exactly once ({} times), so it is not \
measuring `Stepwise` at all",
seen.len(),
);
seen[0] == here
}
/// Where each `entry_by_location` happened, and how many pack-aware handles
/// were opened to make them happen.
struct Tally {
seen: Arc<Mutex<Vec<std::thread::ThreadId>>>,
handles: Arc<AtomicUsize>,
}
impl Tally {
/// Threads other than `here` that produced an entry from lookup `from`
/// onwards, and how many lookups they took between them.
///
/// `from` is what makes the width assertion **per request**: each served
/// request builds its own `Stepwise` and therefore its own consumer, so a
/// cumulative count over two requests legitimately holds two distinct
/// thread ids and would read as a fan-out that never happened.
fn off_thread(
&self,
from: usize,
here: std::thread::ThreadId,
) -> (usize, HashSet<std::thread::ThreadId>) {
let seen = self.seen.lock().expect("the witness was not poisoned");
let tail = &seen[from..];
let ids: HashSet<_> = tail.iter().copied().filter(|id| *id != here).collect();
(tail.iter().filter(|id| **id != here).count(), ids)
}
fn lookups(&self) -> usize {
self.seen
.lock()
.expect("the witness was not poisoned")
.len()
}
}
#[test]
fn every_pack_entry_is_produced_on_the_calling_thread() {
assert!(git_available(), "stock git is required (S-026)");
let repo = tempfile::tempdir().expect("tempdir");
let ids = churned_repo(repo.path());
let inline = stepwise_is_inline();
let tally = Tally {
seen: Arc::new(Mutex::new(Vec::new())),
handles: Arc::new(AtomicUsize::new(0)),
};
let gix_repo = gix::open(repo.path()).expect("opening the fixture repository");
let store = gix_repo.objects.store();
let packed = {
let seen = Arc::clone(&tally.seen);
let handles = Arc::clone(&tally.handles);
PackedObjects::new(move || {
handles.fetch_add(1, Ordering::Relaxed);
let mut handle = store.to_handle_arc();
handle.prevent_pack_unload();
Box::new(ThreadWitness {
inner: Box::new(handle) as Box<dyn gix_pack::Find + Send>,
seen: Arc::clone(&seen),
}) as Box<dyn gix_pack::Find + Send>
})
};
// `PackedObjects::new` opens one eagerly for itself; everything after this
// is the pipeline asking for another.
let handles_at_rest = tally.handles.load(Ordering::Relaxed);
assert_eq!(
handles_at_rest, 1,
"`PackedObjects::new` no longer opens exactly one handle, so the counts below \
are measured against the wrong baseline"
);
let here = std::thread::current().id();
let mut bytes = Vec::new();
// The **base closure off**, and that is what keeps the count assertion
// below exact. With it on there are two `entry_by_location` passes over the
// pack — `base_closure` reads a 64-byte header per pack-resident id before
// the generator reads the entries — so "one lookup per copied object" stops
// being the right number. Both passes still have to obey `thread_limit`, and
// the second half of this test asserts exactly that against the shipping
// default rather than leaving the new pass unwatched.
let outcome = write_pack_copy(
&ids,
&packed,
&mut bytes,
&WriteOptions::new(Kind::Sha1).base_closure(None),
)
.expect("writing the copied pack");
// Non-vacuity first, and it is not a formality: every assertion below is
// trivially true of an empty list, so a fixture whose objects were not
// pack-resident — or a `write_pack_copy` that never reached the entry phase
// — would otherwise pass this test while proving nothing at all.
assert!(
outcome.objects_copied_from_pack > 0,
"no entry was copied from a pack, so `entry_by_location` was never the path under test"
);
// One lookup per copied entry, **plus one 12-byte read of each pack's own
// header**: `PackedObjects::objects_in_pack` holds the index's per-pack
// offset list to what the pack says it holds, because a multi-pack index
// hands back a list that is short by the objects it attributed to another
// pack and a short list sizes an entry across its neighbour. That read goes
// through `entry_by_location` at offset 0, so the witness sees it, and it is
// counted here rather than loosened into an inequality — an inequality would
// stop noticing a second, unintended pass over the pack.
let packs = std::fs::read_dir(repo.path().join(".git").join("objects").join("pack"))
.expect("reading objects/pack")
.filter_map(|e| e.ok())
.filter(|e| e.path().extension().is_some_and(|x| x == "pack"))
.count();
assert_eq!(packs, 1, "the fixture is repacked and holds one pack");
assert_eq!(
tally.lookups(),
outcome.objects_copied_from_pack + packs,
"the witness saw {} entry lookups for {} copied objects over {packs} pack(s); it is not \
on the path it claims to be watching",
tally.lookups(),
outcome.objects_copied_from_pack,
);
let (off, off_threads) = tally.off_thread(0, here);
// ── the claim that holds whether or not the fork is in the graph ─────────
//
// `P-004` is "the serving tier stays serial". Stock `Stepwise` breaks
// *where* the work runs; it does not break *how wide* it runs, because
// `num_threads(Some(1))` is still 1. So one producing thread besides the
// caller is the worst stock gix can do, and anything more means
// `thread_limit` itself was widened — the regression this file is named
// for, and the one the old thread-identity assertion could not tell apart
// from the `Stepwise` spawn it was already failing on.
assert!(
off_threads.len() <= 1,
"{} pack entries were produced across {} threads besides the caller ({off_threads:?}); \
`thread_limit: Some(1)` bounds this to one whatever `Stepwise` does with it, so the \
declaration in `src/copy.rs` has been widened",
off,
off_threads.len(),
);
let handles = tally.handles.load(Ordering::Relaxed) - handles_at_rest;
eprintln!(
"serve #1: Stepwise inline={inline}, {off} of {} lookups off-thread, \
{handles} pack-aware handles opened beyond the caller's",
tally.lookups(),
);
if inline {
assert_eq!(
off,
0,
"{off} of {} pack entries were produced off the calling thread ({:?} != {here:?}) \
even though `Stepwise` is the inline one — see `vendor/gix-features/PATCH.md` \
fix 2 (P-020, P-004)",
tally.lookups(),
off_threads.iter().next(),
);
assert_eq!(
handles, 2,
"the inline pipeline opened {handles} pack-aware handles beyond the caller's, not 2; \
each one maps this repository's packs again, which is a per-request fd and mapping \
the appliance did not budget"
);
} else {
// Stock `gix-features`: this workspace has carried no `[patch.crates-io]`
// since `cbe0fb8` (2026-08-23) and gitoxide issue 2881 is unmerged, so
// `Stepwise` spawns a consumer and a feeder and EVERY generator lookup
// lands on the consumer. Asserted as an equality, not "some": a partial
// split would mean something else moved.
assert_eq!(
off, outcome.objects_copied_from_pack,
"stock `Stepwise` should put every one of the {} generator lookups on its single \
consumer thread and leave only the {packs} pack-header read with the caller; {off} \
went off-thread. Something other than `Stepwise` is now deciding where this runs",
outcome.objects_copied_from_pack,
);
assert_eq!(
handles, 3,
"stock `Stepwise` opened {handles} pack-aware handles beyond the caller's, not 2 \
(one for the resolve phase's `db.clone()`, one for the consumer's). That count is \
the fd and mapping cost the two spawned threads actually carry, and the inline \
fork's is 1"
);
}
// ── and now the shipping default, which reads the pack twice ────────────
//
// `base_closure` walks the selection's entry headers before the generator
// runs. That is a second pass over the same seam, and a second pass is a
// second chance to fan out; leaving it unwatched would mean the one
// instrument that can see `thread_limit` being ignored watches only half of
// what the serve path does.
let without_closure = tally.lookups();
let handles_before = tally.handles.load(Ordering::Relaxed);
let mut bytes = Vec::new();
let with = write_pack_copy(&ids, &packed, &mut bytes, &WriteOptions::new(Kind::Sha1))
.expect("writing the copied pack with the base closure on");
assert!(
tally.lookups() > without_closure + with.objects_copied_from_pack,
"the base closure added no entry lookups at all ({} after {without_closure}), so the \
thread assertion below is watching the generator's pass twice rather than the \
closure's",
tally.lookups(),
);
let (off_second, off_threads) = tally.off_thread(without_closure, here);
assert!(
off_threads.len() <= 1,
"with the base closure on, entries were produced across {} threads besides the caller \
({off_threads:?}); `thread_limit: Some(1)` bounds this to one",
off_threads.len(),
);
let handles = tally.handles.load(Ordering::Relaxed) - handles_before;
eprintln!(
"serve #2: Stepwise inline={inline}, {off_second} of {} lookups off-thread, \
{handles} pack-aware handles opened beyond the caller's",
tally.lookups() - without_closure,
);
if inline {
assert_eq!(
off_second,
0,
"{off_second} of the second request's {} pack entry lookups happened off the calling \
thread with the base closure on ({:?} != {here:?})",
tally.lookups() - without_closure,
off_threads.iter().next(),
);
assert_eq!(
handles, 2,
"the second inline request opened {handles} handles beyond the caller's, not 2"
);
} else {
// The closure's own pass runs before `Stepwise` exists — it is the
// caller's — so the second request adds exactly its generator's
// lookups to the off-thread tally and nothing else.
assert_eq!(
off_second, with.objects_copied_from_pack,
"the second request moved {off_second} lookups off-thread for {} copied objects; with \
stock `Stepwise` the base closure's pass is the caller's and the generator's is the \
consumer's, so those two numbers are the same",
with.objects_copied_from_pack,
);
assert_eq!(
handles, 3,
"the second stock request opened {handles} handles beyond the caller's, not 3"
);
}
}