mirror of
https://github.com/Twigpine/node.git
synced 2026-11-09 13:31:25 -10:00
fix(node): spawn the replication tail at the durability boundary
The post-receive replication tail was spawned at the end of git_receive_pack, after touch_repo, the trust-score update, per-ref certificate issuance and webhook fan-out. A client or proxy disconnect anywhere in that window dropped the tail, and with it this push's pins, recovery copy, and announcements - even though the push was already durable on disk. Spawn it immediately after the receive_result ? instead. That is the first point where the push is known to have landed: guard.release() a few lines up runs on failure too, so anchoring there would spawn a tail for a push git rejected, pinning and announcing a half-applied repo - the state release() deliberately refuses to upload to Tigris, and one a hostile pusher can produce on demand by aborting a pack mid-transfer. RefUpdate gains Clone so the tail takes the parsed updates while the certificate and webhook loops keep iterating their own copy. This makes the tail run concurrently with certificate issuance rather than after it, so a ref can be announced before its signed certificate exists. Accepted, and stated here rather than left for a reviewer to find: cert issuance already fails open (errors are logged and skipped) and the gossip event carries cert_id: None regardless, so no announce consumer reads a certificate out of it. Evidence is an ordering gate in inv22_gates.rs rather than a cancellation-race test, matching the F3 gate's instrument: forcing a disconnect to land inside the cert/webhook window is timing-dependent and would be flaky, while the ordering is precisely what changed. The gate is mutation-verified in both directions - moving the spawn back below the webhooks fails the upper bound, hoisting it above the ? fails the lower one.
This commit is contained in:
1 parent
19291dfd64
commit
ade53cbf47
2 files changed
+88
-18
No files matched your search
@@ -1823,6 +1823,34 @@ pub async fn git_receive_pack(
|
||||
app
|
||||
})?;
|
||||
|
||||
// #174 F2/U5: the post-receive replication tail runs in an independently owned
|
||||
// task. It parks on `git_encrypt_semaphore` (withheld / candidate / full-scan
|
||||
// resolution), so leaving it in the request future means a client/proxy disconnect
|
||||
// while parked silently drops this push's pins, recovery copy, and announcements.
|
||||
//
|
||||
// Spawned HERE, at the durability boundary, rather than at the end of the handler.
|
||||
// This is the first point where the push is known to have landed: `guard.release()`
|
||||
// above runs on failure too, so anchoring on it would spawn a tail that pins and
|
||||
// announces a half-applied repo — the state release() deliberately refuses to
|
||||
// upload, and one a hostile pusher can produce on demand by aborting a pack
|
||||
// mid-transfer. Everything below (touch_repo, metrics, trust score, certificates,
|
||||
// webhooks) stays in the cancellable handler, so a disconnect during that window no
|
||||
// longer takes the tail with it.
|
||||
//
|
||||
// The tail now runs CONCURRENTLY with certificate issuance rather than after it, so
|
||||
// a ref can be announced before its signed certificate exists. That window is
|
||||
// accepted: cert issuance already fails open (errors are logged and skipped) and the
|
||||
// gossip event carries `cert_id: None` regardless, so no announce consumer reads a
|
||||
// certificate out of it. Each push owns its own tail, including its own
|
||||
// always-spawned announce, so per-push announcements are never coalesced away.
|
||||
tokio::spawn(post_receive_replication_tail(
|
||||
state.clone(),
|
||||
record.clone(),
|
||||
ref_updates.clone(),
|
||||
disk_path.clone(),
|
||||
auth.0.to_string(),
|
||||
));
|
||||
|
||||
// Update the repo's updated_at timestamp after a successful push
|
||||
let _ = state.db.touch_repo(&record.id).await;
|
||||
|
||||
@@ -1912,24 +1940,6 @@ pub async fn git_receive_pack(
|
||||
}
|
||||
}
|
||||
|
||||
// #174 F2: move the whole post-receive replication tail into an independently owned
|
||||
// task. It parks on `git_encrypt_semaphore` (withheld / candidate / full-scan
|
||||
// resolution), so leaving it in the request future means a client/proxy disconnect
|
||||
// while parked silently drops this push's pins, recovery copy, and announcements —
|
||||
// the residual `state.rs` documented, because the durable `try_begin` gate sat after
|
||||
// the park. The request future now returns the git response immediately and the tail
|
||||
// runs detached; a disconnect can no longer drop it. Each push owns its own tail,
|
||||
// including its own always-spawned announce, so per-push announcements are never
|
||||
// coalesced away (the announce spawn stays out of the per-repo encrypt coalescing).
|
||||
let did = did.to_string();
|
||||
tokio::spawn(post_receive_replication_tail(
|
||||
state,
|
||||
record,
|
||||
ref_updates,
|
||||
disk_path,
|
||||
did,
|
||||
));
|
||||
|
||||
Ok(result)
|
||||
}
|
||||
|
||||
@@ -2638,6 +2648,10 @@ pub async fn get_icaptcha_proof(
|
||||
|
||||
// ── Pkt-line parsing ──────────────────────────────────────────────────────
|
||||
|
||||
/// `Clone` so `git_receive_pack` can hand the parsed updates to the detached
|
||||
/// replication tail at the durability boundary while the certificate and webhook
|
||||
/// loops below still iterate their own copy (#174 U5).
|
||||
#[derive(Clone)]
|
||||
struct RefUpdate {
|
||||
old_sha: String,
|
||||
new_sha: String,
|
||||
|
||||
@@ -413,3 +413,59 @@ fn inv22_ipfs_walk_admission_reaches_every_blocking_site() {
|
||||
give any new blocking walk its own Arc::clone(&admission) and update this gate"
|
||||
);
|
||||
}
|
||||
|
||||
/// #174 U5: the post-receive replication tail is spawned at the DURABILITY BOUNDARY,
|
||||
/// not at the end of the handler.
|
||||
///
|
||||
/// The tail owes this push its pins, recovery copy, and announcements. Spawned below
|
||||
/// the certificate and webhook work, a client disconnect anywhere in that window took
|
||||
/// the tail with it. Spawned above it, the tail is already detached and survives.
|
||||
///
|
||||
/// The lower bound matters just as much as the upper one: `guard.release()` runs on
|
||||
/// failure too, so a spawn anchored there would fire for a push git rejected, pinning
|
||||
/// and announcing a half-applied repo. The `?` on `receive_result` is the first point
|
||||
/// where the push is known to have landed, which is why the spawn must sit between
|
||||
/// that `?` and `touch_repo`.
|
||||
///
|
||||
/// This is an ordering check rather than a cancellation-race test on purpose: forcing
|
||||
/// a disconnect to land inside the cert/webhook window is timing-dependent and would
|
||||
/// be flaky, while the ordering is exactly what the fix changes. Same instrument the
|
||||
/// F3 gate above uses.
|
||||
///
|
||||
/// MUTATION (RED): move the `tokio::spawn(post_receive_replication_tail` call back
|
||||
/// below the webhook block and the ordering assertion fails; move it above the `?`
|
||||
/// and the failed-push assertion fails.
|
||||
#[test]
|
||||
fn inv22_replication_tail_spawns_at_the_durability_boundary() {
|
||||
let repos = src("api/repos.rs");
|
||||
// Production half only — the tests below name these identifiers too.
|
||||
let production = repos
|
||||
.split("#[cfg(test)]")
|
||||
.next()
|
||||
.expect("split always yields a first chunk");
|
||||
|
||||
let receive_ok = production
|
||||
.find("let result = receive_result.map_err(")
|
||||
.expect("U5 gate stale: git_receive_pack no longer maps receive_result with ?");
|
||||
let spawn = production
|
||||
.find("tokio::spawn(post_receive_replication_tail(")
|
||||
.expect("U5 gate missing: the replication tail must be spawned by git_receive_pack");
|
||||
let touch = production
|
||||
.find("state.db.touch_repo(")
|
||||
.expect("U5 gate stale: git_receive_pack no longer calls touch_repo");
|
||||
let webhook = production
|
||||
.find("webhooks::fire_event(")
|
||||
.expect("U5 gate stale: git_receive_pack no longer fires push webhooks");
|
||||
|
||||
assert!(
|
||||
receive_ok < spawn,
|
||||
"U5 gate bypassed: the tail must be spawned AFTER the receive_result `?`, or a \
|
||||
rejected push spawns a tail that pins and announces a half-applied repo"
|
||||
);
|
||||
assert!(
|
||||
spawn < touch && spawn < webhook,
|
||||
"U5 gate bypassed: the tail must be spawned BEFORE touch_repo and the webhook \
|
||||
fan-out, so a disconnect in that window cannot drop this push's pins, \
|
||||
recovery copy, and announcements"
|
||||
);
|
||||
}
|
||||
Reference in new issue
Block a user