You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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_list at 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 to pin_and_encrypt_objects (repos.rs:888), which calls pin_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 at main.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_PUSH defaults 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.
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.
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.
Summary
PR #174 adds a global
pin_semaphorethat 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.EncryptInflightcaps 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/mainhas nopin_semaphore, noEncryptInflight, and nopin_new_objects_gated.The branch already states this gap in its own docstrings, deliberately, in four places (
pin_new_objects_gatedinapi/repos.rs, thepin_semaphoreandencrypt_inflightfield docs instate.rs, and themax_concurrent_pin_tasksdoc inconfig.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 incrates/gitlawb-node/src/api/repos.rsunless noted:object_listat repos.rs:2154-2170.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.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.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_refsat repos.rs:2308.Reachability
Pushing is authenticated (
git_receive_pack, repos.rs:1640, takesExtension<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.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_APIempty (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, viaEncryptInflight); 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
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.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.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.