Repository navigation
IPFS pin task retains its full object list while parked, so retention is unbounded across repos (#174 follow-up) #296
Description
Summary
PR #174 adds a global pin_semaphore that bounds how many post-push pin loops run concurrently. On the local IPFS replication path the per-push object list is materialized and moved into the detached task before that permit is acquired, so a task parked waiting for a permit still holds its full list. EncryptInflight caps parked tasks at one per repo, so nothing bounds the retained memory across distinct repos.
This is a follow-up to #174, not a defect on main: origin/main has no pin_semaphore, no EncryptInflight, and no pin_new_objects_gated.
The branch already states this gap in its own docstrings, deliberately, in four places (pin_new_objects_gated in api/repos.rs, the pin_semaphore and encrypt_inflight field docs in state.rs, and the max_concurrent_pin_tasks doc in config.rs). This issue exists to track that declared residual, not to report it as undisclosed. It is the sibling of #287, which tracks the same pool's hold time on the Pinata side.
Mechanism
At head 807bc93, all in crates/gitlawb-node/src/api/repos.rs unless noted:
- The receive-pack tail materializes
object_listat repos.rs:2154-2170. - It moves that list into the detached task:
tokio::spawn(run_encrypt_pin_task(ctx, inflight_guard, object_list, ...))at repos.rs:2213. run_encrypt_pin_task(repos.rs:880) hands the snapshot topin_and_encrypt_objects(repos.rs:888), which callspin_new_objects_gated(repos.rs:1185) taking the list by value.- The permit is acquired at repos.rs:1160, after the parked future already owns the list.
EncryptInflight(crates/gitlawb-node/src/state.rs:286) keys in-flight tasks by repo id, one per repo. There is no global cap on the parked set.- The pool is
max_concurrent_pin_tasks, default 8 (crates/gitlawb-node/src/config.rs:396, wired atmain.rs:409).
The Pinata task on the same tail orders it the other way: it acquires at repos.rs:2305 and re-derives its list inside the permit via pinata_object_list_for_refs at repos.rs:2308.
Reachability
Pushing is authenticated (git_receive_pack, repos.rs:1640, takes Extension<AuthenticatedDid>). Two things widen that:
GITLAWB_ENFORCE_OWNER_PUSHdefaults to false (config.rs:85), and the field's own doc notes that any party can sign as their own DID, so a self-generated did:key can push to repos it does not own and no repo creation is needed.- The only brake on the route is a per-IP rate limit, default 600/hour (
main.rs:325), on a key the project already documents as rotatable (see TrustedProxy IP trust is config-asserted, not peer-verified: header rotation defeats every per-IP brake #207, gitlawb-node-2 and -3 do not set GITLAWB_TRUSTED_PROXY, collapsing four per-IP limiters into one shared bucket #264, A write guard holds a lock-pool slot for up to 600s, so ~32 concurrent pushes deny every write on the node #282). The request body is unbounded (server.rs:205,DefaultBodyLimit::disable()).
What limits it, stated plainly: accumulation requires the 8-slot pin pool to be saturated and slow, which needs a configured IPFS endpoint under load. With GITLAWB_IPFS_API empty (the default, config.rs:88) each permit is held only long enough to no-op, so lists do not pile up on a stock node. The realistic worst case is a node with IPFS configured and the endpoint degraded: pushes to N distinct repos retain N object-id lists (roughly 64 bytes per object id, so tens of MB for a very large push) for as long as the pool stays full. Authenticated but unprivileged, not anonymous.
What is and is not bounded today
Bounded: how many pin loops run concurrently (pin_semaphore, default 8); in-flight pin tasks per repo (one, via EncryptInflight); coalesced pending tip pairs per repo (MAX_PENDING_TIP_PAIRS = 1024, state.rs:300, over fixed-width 40-hex pairs, so about 80 KB); and the IPFS pin loop's own hold, via the batch budget added in #174.
Not bounded: the object-list memory held by tasks parked on pin_semaphore, summed across repos.
Also not bounded, and worth naming because it sits on the path usually cited as the good example: the Pinata task captures full ref names (ref_updates_clone, repos.rs:2251), parse_ref_updates (repos.rs:2755) applies no length check to the ref name, a pkt-line permits one near 65 KB, and that task is deliberately not coalesced (see the comment at repos.rs:2222-2231), so its parked count is one per push with no cap. The Pinata ordering avoids retaining the object list; it does not make that path free of retention.
Three fix shapes that were tried and do not work
- Capture the ref tuples instead of the list. Does not bound retention.
parse_ref_updates(repos.rs:2755) checks that both SHAs are 40 characters and applies no bound to the ref name, and the request body is unlimited, so one captured tuple can carry a name near 65 KB. - Cap that capture at
MAX_PENDING_TIP_PAIRS(1024). Bounds count, not bytes. That constant is byte-safe only because its current contents are fixed-width 40-hex SHA pairs (PendingWork::Tips, state.rs:305). Applied to a capture containing ref names it is a count bound with nothing behind it. - Derive the list after the park, mirroring the Pinata twin. Needs a fresh repo-identity re-read, because the disk path is keyed on the owner/name slug and a delete plus re-create between spawn and drain gives the row a new id (see the comment at repos.rs:1023). Abandoning on an id mismatch then drops a pin the coalesced drain would otherwise have kept, and there is no reconciliation sweep to recover it (Implement the reconciliation sweep the replication path already assumes as a durability backstop #218). That is the failure class Coalesced drain drops pending pin work permanently on a transient DB error #289 already documents on the drain path.
What a real fix has to bound
A byte bound on what a parked task retains, not a count bound, and not a bound that trades retention for a dropped pin. It needs to hold under all three of: a ref name near the pkt-line maximum, a push whose object list is large, and pushes spread across many distinct repos so the per-repo cap does not bind.
Acceptance should be a retention assertion (total retained bytes across parked tasks stays under a stated ceiling while the pool is saturated), not a task count, since a count assertion passes today while the bytes are unbounded. Whatever lands must not reintroduce the drop-on-mismatch path from shape 3.
Refs #174. Sibling of #287 (same pool, hold time rather than retention). Related to #217, which moves the same spawn for a different reason, and to #218, whose absence is what makes dropping work in shape 3 permanent.
Activity