Repository navigation
Read/fork handlers run blocking git subprocesses on the tokio runtime (no spawn_blocking, no timeout) #204
Description
Surfaced during review of #196 (pre-existing; git/store.rs and the fork clone are untouched by that PR's diff, so splitting here rather than widening #196's scope).
fork_repo — crates/gitlawb-node/src/api/repos.rs:1567 runs std::process::Command::new("git").args(["clone","--mirror",…]).output() inside the async handler. output() blocks the worker thread; a git clone --mirror of a non-trivial repo can take minutes and there is no timeout, unlike other git walks in this file that use tokio::task::spawn_blocking (repos.rs:67,110,661,1143) or tokio::process::Command.
Read handlers — list_commits/get_tree/get_blob call store::{resolve_head,log,ls_tree,read_file} (api/repos.rs:360-361,397,443,481), which are synchronous std::process::Command::output() calls (git/store.rs:92-231). A slow disk or large repo blocks a worker for seconds; with a finite worker pool a handful of concurrent /commits, /tree, /blob, or one /fork can starve the runtime, including /health and /ready.
Impact: availability/reliability regression; git_service_timeout_secs is bypassed on these paths.
Fix direction: wrap the blocking store calls (and the fork clone) in spawn_blocking bounded by tokio::time::timeout(git_service_timeout_secs, …), returning AppError::Timeout. Verified real by execution during the #196 review.
Activity
beardthelion commentedon Aug 15, 2026
A third site, on the background path rather than a request handler, so it escapes the
git_service_timeout_secsframing above in a way worth noting separately.encrypt_and_pinispub async fn(crates/gitlawb-node/src/encrypted_pin.rs:109) and calls thesynchronous read bare inside its loop (
:155):No
spawn_blockingand noblock_in_placeanywhere in the file.read_object(
crates/gitlawb-node/src/git/store.rs:681-688) composesobject_typeandread_object_content,both plain
std::process::Command::output()with no deadline and no process-group reaping(
store.rs:277-281,store.rs:309-313). The bounded twin's own docstring states the rule thisbreaks (
store.rs:643-645): "This is SYNCHRONOUS blocking work ... Async callers must run it undertokio::task::spawn_blocking... calling it directly from a runtime task blocks a worker."What makes it worth adding here rather than leaving to a later sweep is that it is residue from a fix
that already named it. Commit
03dde22e(#174) touchedencrypted_pin.rsby exactly one line,bounding
pin_git_objectonly, and its message says: "Not bounded: the git read, sincestore::read_object shells two bare Command::output() calls with no timeout or process-group reaping
and this loop does not run it under spawn_blocking." Both sibling pin loops got the bounded read
afterwards (
ipfs_pin.rs:299-306,pinata.rs:166-173, eachspawn_blocking(|| read_object_bounded(..., read_deadline))).This loop did not.
Reachability differs from the sites above. Post-receive only:
run_encrypt_pin_taskintopin_and_encrypt_objects(repos.rs:1181) intoencrypt_and_pin(repos.rs:1224), gated on the repocarrying a path-scoped rule with withheld blobs. So it needs push rights plus rule setup, though on an
open node any registered user can arrange both.
The impact shape is also different, and is why the request-path framing does not cover it: the push
response has already returned, so there is no slow request to observe. Per withheld blob, two blocking
git cat-filechildren hold a runtime worker for their full duration with no ceiling.EncryptInflightcaps one task per repo, so concurrency across repos is bounded by nothing, and N concurrent repos hold
N workers, which starves the shared runtime including
/healthand/readyexactly as describedabove.
Fix is the same shape as the one already proposed here, with one wrinkle:
encrypt_and_pincurrentlytakes neither a
git_binnor a deadline, so both have to be threaded in fromEncryptTaskCtxbeforeit can call
read_object_bounded.Not verified by execution; the regression test needs a live Postgres and a full node build. The static
path is unambiguous, since both constituents are visibly untimed
Command::output()and no wrapperexists in the file.
fix(node): Hide Git diagnostics from blob error responses.
fix(node): close REST blob admission and error gaps
docs(config): describe the blob authorization deadline
test(api): bound the blob authorization timeout regression