Skip to content

Commit 6bba625

Browse files
Bound crawler and tool probes through one spawn deadline (#845) (#886)
* Start refactor for #845 Assisted-by: Claude Code:claude-opus-5-5 * Bound crawler and tool probes by one deadline A version-manager shim that never answers (`gem`, `python3`, `npm`, `composer` behind rbenv/asdf, a Ruby waiting on a network gem home) used to hang `scan`, `apply`, `vex` and every other crawling command forever with no output: the crawler probes waited on `output()` with no deadline. Every probe now runs through one `utils::process::output_within` primitive: null stdin, captured stdout, dropped stderr, and the child killed and reaped at the deadline without waiting on a grandchild that still holds the pipe. Crawler probes get the same 10 s budget that the Pipenv and Hatch version probes and the self-update `--version` check already used, and those three sites drop their hand-rolled `tokio::time::timeout` + `kill_on_drop` blocks for it. Refs #845. Assisted-by: Claude Code:claude-opus-5-5 * Port #878: Gradle digests through utils::digest `main` fails `utils::digest::tests::production_digests_go_through_the_ helpers` because #646 left inline sha1/sha256 calls in `gradle_cache.rs`, `jvm_jar.rs` and `sidecars/maven.rs`, which turns `test`, `test-release` and `coverage` red on every PR. This is #878's change verbatim; it no-ops once #878 merges. Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 19f5f38 commit 6bba625

4 files changed

Lines changed: 229 additions & 34 deletions

File tree

‎crates/socket-patch-core/src/update/download.rs‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -265,19 +265,21 @@ async fn sanity_exec(
265265
const ETXTBSY_ATTEMPTS: u64 = 10;
266266
let mut attempt = 0u64;
267267
let output = loop {
268-
let mut cmd = tokio::process::Command::new(staged);
269-
cmd.arg("--version")
270-
.stdin(std::process::Stdio::null())
271-
.stdout(std::process::Stdio::piped())
272-
.stderr(std::process::Stdio::null())
273-
.kill_on_drop(true);
274-
let result = tokio::time::timeout(std::time::Duration::from_secs(10), cmd.output())
275-
.await
276-
.map_err(|_| {
277-
UpdateError::VerifyFailed(
268+
let mut cmd = std::process::Command::new(staged);
269+
cmd.arg("--version");
270+
let result = match crate::utils::fs::run_blocking(move || {
271+
crate::utils::process::output_within(cmd, crate::utils::process::PROBE_TIMEOUT)
272+
})
273+
.await
274+
{
275+
Ok(output) => Ok(output),
276+
Err(crate::utils::process::BoundedError::Spawn(e)) => Err(e),
277+
Err(crate::utils::process::BoundedError::TimedOut) => {
278+
return Err(UpdateError::VerifyFailed(
278279
"downloaded binary hung during its --version self-check".to_string(),
279-
)
280-
})?;
280+
))
281+
}
282+
};
281283
match result {
282284
Ok(output) => break output,
283285
Err(e)

‎crates/socket-patch-core/src/utils/pipenv.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ pub async fn installed_major(root: &Path) -> Option<u32> {
6565
// The RESOLVED path is spawned; a Windows `.bat` / `.cmd` shim is run by
6666
// `std` itself through cmd.exe with correct quoting (see the shared
6767
// launcher's docs).
68-
let mut command = tokio::process::Command::from(crate::utils::process::command_for(&program));
68+
let mut command = crate::utils::process::command_for(&program);
6969
// The version banner does not depend on a project, so the probe runs in a
7070
// NEUTRAL directory: with the scanned repository as cwd, Pipenv would read
7171
// its `.env`, `Pipfile` and `.venv` pointer — committed, attacker-shaped
@@ -76,12 +76,12 @@ pub async fn installed_major(root: &Path) -> Option<u32> {
7676
.current_dir(std::env::temp_dir())
7777
.env("PIPENV_DONT_LOAD_ENV", "1")
7878
.env("PIPENV_NOSPIN", "1")
79-
.env("PIPENV_IGNORE_VIRTUALENVS", "1")
80-
.kill_on_drop(true);
81-
let output = tokio::time::timeout(std::time::Duration::from_secs(10), command.output())
82-
.await
83-
.ok()?
84-
.ok()?;
79+
.env("PIPENV_IGNORE_VIRTUALENVS", "1");
80+
let output = crate::utils::fs::run_blocking(move || {
81+
crate::utils::process::output_within(command, crate::utils::process::PROBE_TIMEOUT)
82+
})
83+
.await
84+
.ok()?;
8585
if !output.status.success() {
8686
return None;
8787
}

‎crates/socket-patch-core/src/utils/process.rs‎

Lines changed: 170 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,81 @@
1717
//! runner or thread a singleton.
1818
1919
use std::ffi::OsString;
20+
use std::io::Read;
2021
use std::path::{Path, PathBuf};
21-
use std::process::Command;
22+
use std::process::{Command, Output, Stdio};
23+
use std::time::{Duration, Instant};
24+
25+
/// How long a crawler probe (`gem env gemdir`, `npm root -g`, `python3
26+
/// --version`, ...) may run before it is killed and answers "no
27+
/// information". The same budget the Pipenv and Hatch version probes and the
28+
/// self-update `--version` check use.
29+
pub const PROBE_TIMEOUT: Duration = Duration::from_secs(10);
30+
31+
/// Why [`output_within`] produced no [`Output`].
32+
#[derive(Debug)]
33+
pub enum BoundedError {
34+
/// The child could not be spawned (missing program, ETXTBSY, ...).
35+
Spawn(std::io::Error),
36+
/// The child did not exit and close stdout within the budget; it was
37+
/// killed.
38+
TimedOut,
39+
}
40+
41+
/// Run `command` to completion within `budget`: the one bounded spawn every
42+
/// probe goes through.
43+
///
44+
/// stdin is null (the child can't wait for input), stdout is captured and
45+
/// stderr is discarded. When the child has not exited and closed stdout by
46+
/// the deadline it is killed and reaped, and the call returns
47+
/// [`BoundedError::TimedOut`] without waiting for a grandchild that still
48+
/// holds the pipe (a `#!/bin/sh` shim's `sleep`). A wedged toolchain shim (a
49+
/// version manager prompting for an install, a Ruby waiting on a network
50+
/// gem home) therefore costs at most `budget`, never the whole run.
51+
///
52+
/// Blocking: async callers run it through `utils::fs::run_blocking`.
53+
pub fn output_within(mut command: Command, budget: Duration) -> Result<Output, BoundedError> {
54+
let deadline = Instant::now() + budget;
55+
let mut child = command
56+
.stdin(Stdio::null())
57+
.stdout(Stdio::piped())
58+
.stderr(Stdio::null())
59+
.spawn()
60+
.map_err(BoundedError::Spawn)?;
61+
let mut stdout = child.stdout.take().expect("stdout is piped");
62+
let (sender, receiver) = std::sync::mpsc::channel();
63+
std::thread::spawn(move || {
64+
let mut bytes = Vec::new();
65+
let _ = stdout.read_to_end(&mut bytes);
66+
let _ = sender.send(bytes);
67+
});
68+
let timed_out = |child: &mut std::process::Child| {
69+
let _ = child.kill();
70+
let _ = child.wait();
71+
Err(BoundedError::TimedOut)
72+
};
73+
let Ok(stdout) = receiver.recv_timeout(deadline.saturating_duration_since(Instant::now()))
74+
else {
75+
return timed_out(&mut child);
76+
};
77+
loop {
78+
match child.try_wait() {
79+
Ok(Some(status)) => {
80+
return Ok(Output {
81+
status,
82+
stdout,
83+
stderr: Vec::new(),
84+
})
85+
}
86+
Ok(None) if Instant::now() < deadline => std::thread::sleep(Duration::from_millis(5)),
87+
Ok(None) => return timed_out(&mut child),
88+
Err(error) => {
89+
let _ = child.kill();
90+
return Err(BoundedError::Spawn(error));
91+
}
92+
}
93+
}
94+
}
2295

2396
/// The executable `name` on ABSOLUTE `PATH` entries only, or `None` when
2497
/// no entry holds one.
@@ -212,6 +285,16 @@ pub(crate) fn neutral_probe_dir_with(var: &impl Fn(&str) -> Option<OsString>) ->
212285
/// names a path is spawned as given) with `args`, optionally from `cwd`,
213286
/// and return its trimmed stdout under the [`CommandRunner`] contract.
214287
fn run_resolved(bin: &str, args: &[&str], cwd: Option<&Path>) -> Option<String> {
288+
run_resolved_within(bin, args, cwd, PROBE_TIMEOUT)
289+
}
290+
291+
/// [`run_resolved`] under an explicit budget (tests).
292+
fn run_resolved_within(
293+
bin: &str,
294+
args: &[&str],
295+
cwd: Option<&Path>,
296+
budget: Duration,
297+
) -> Option<String> {
215298
let program = if Path::new(bin).components().count() > 1 {
216299
PathBuf::from(bin)
217300
} else {
@@ -233,7 +316,19 @@ fn run_resolved(bin: &str, args: &[&str], cwd: Option<&Path>) -> Option<String>
233316
if let Some(cwd) = cwd {
234317
command.current_dir(cwd);
235318
}
236-
let output = command.output().ok()?;
319+
let output = match output_within(command, budget) {
320+
Ok(output) => output,
321+
Err(BoundedError::TimedOut) => {
322+
if crate::utils::env_compat::is_debug_enabled() {
323+
eprintln!(
324+
"[socket-patch debug] probe `{bin} {}` did not answer within {budget:?}; treating it as absent",
325+
args.join(" ")
326+
);
327+
}
328+
return None;
329+
}
330+
Err(BoundedError::Spawn(_)) => return None,
331+
};
237332
if !output.status.success() {
238333
return None;
239334
}
@@ -260,6 +355,79 @@ mod tests {
260355
assert_eq!(out, "hello");
261356
}
262357

358+
/// A probe that never answers (a wedged `gem` shim) is killed at the
359+
/// budget and answers "no information" instead of hanging the crawl.
360+
/// On `main` `run_resolved` waited on `output()` with no deadline.
361+
#[cfg(unix)]
362+
#[test]
363+
fn a_hung_probe_answers_none_within_its_budget() {
364+
let tmp = tempfile::tempdir().unwrap();
365+
let shim = tmp.path().join("gem");
366+
std::fs::write(&shim, "#!/bin/sh\nexec sleep 30\n").unwrap();
367+
set_executable(&shim);
368+
let start = Instant::now();
369+
let out = run_resolved_within(
370+
shim.to_str().unwrap(),
371+
&["env", "gemdir"],
372+
None,
373+
Duration::from_millis(300),
374+
);
375+
assert_eq!(out, None);
376+
assert!(
377+
start.elapsed() < Duration::from_secs(10),
378+
"the budget must bound the probe, took {:?}",
379+
start.elapsed()
380+
);
381+
}
382+
383+
/// A shim that forks its child (no `exec`) leaves a grandchild holding
384+
/// stdout after the shim is killed: the deadline still returns.
385+
#[cfg(unix)]
386+
#[test]
387+
fn output_within_does_not_wait_for_a_grandchild_holding_stdout() {
388+
let mut command = Command::new("sh");
389+
command.args(["-c", "sleep 30; echo late"]);
390+
let start = Instant::now();
391+
let result = output_within(command, Duration::from_millis(300));
392+
assert!(matches!(result, Err(BoundedError::TimedOut)), "{result:?}");
393+
assert!(
394+
start.elapsed() < Duration::from_secs(10),
395+
"{:?}",
396+
start.elapsed()
397+
);
398+
}
399+
400+
/// The bounded spawn keeps the `output()` contract the former callers
401+
/// relied on: stdout captured, stderr dropped, exit status reported,
402+
/// stdin null, a missing program surfaced as a spawn error.
403+
#[cfg(unix)]
404+
#[test]
405+
fn output_within_reports_status_stdout_and_spawn_errors() {
406+
let mut ok = Command::new("sh");
407+
ok.args(["-c", "printf out; printf err >&2"]);
408+
let output = output_within(ok, PROBE_TIMEOUT).unwrap();
409+
assert!(output.status.success());
410+
assert_eq!(output.stdout, b"out");
411+
assert!(output.stderr.is_empty());
412+
413+
let mut failing = Command::new("sh");
414+
failing.args(["-c", "printf partial; exit 3"]);
415+
let output = output_within(failing, PROBE_TIMEOUT).unwrap();
416+
assert_eq!(output.status.code(), Some(3));
417+
assert_eq!(output.stdout, b"partial");
418+
419+
let mut reads_stdin = Command::new("sh");
420+
reads_stdin.args(["-c", "cat; printf done"]);
421+
let output = output_within(reads_stdin, PROBE_TIMEOUT).unwrap();
422+
assert_eq!(output.stdout, b"done", "stdin is null, so `cat` sees EOF");
423+
424+
let missing = Command::new("/definitely/not/a/real/binary-1234567");
425+
assert!(matches!(
426+
output_within(missing, PROBE_TIMEOUT),
427+
Err(BoundedError::Spawn(_))
428+
));
429+
}
430+
263431
/// Spawn failure → None. The binary name is intentionally one
264432
/// that should never be on PATH.
265433
#[test]

‎crates/socket-patch-core/src/vendor/pypi_hatch.rs‎

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -129,21 +129,18 @@ async fn require_environment_context_support_with(
129129
var: &impl Fn(&str) -> Option<std::ffi::OsString>,
130130
) -> Result<(), Failure> {
131131
let output = match crate::utils::process::resolve_tool_with("hatch", var) {
132-
Some(program) => Some(
133-
tokio::time::timeout(
134-
std::time::Duration::from_secs(10),
135-
tokio::process::Command::from(crate::utils::process::command_for(&program))
136-
.arg("--version")
137-
.current_dir(root)
138-
.stdin(std::process::Stdio::null())
139-
.kill_on_drop(true)
140-
.output(),
141-
)
142-
.await,
143-
),
132+
Some(program) => {
133+
let mut command = crate::utils::process::command_for(&program);
134+
command.arg("--version").current_dir(root);
135+
crate::utils::fs::run_blocking(move || {
136+
crate::utils::process::output_within(command, crate::utils::process::PROBE_TIMEOUT)
137+
})
138+
.await
139+
.ok()
140+
}
144141
None => None,
145142
};
146-
if let Some(Ok(Ok(output))) = output {
143+
if let Some(output) = output {
147144
if output.status.success()
148145
&& String::from_utf8_lossy(&output.stdout)
149146
.split_whitespace()
@@ -922,4 +919,32 @@ mod tests {
922919
.unwrap();
923920
assert!(marker.exists(), "the resolved hatch was not run");
924921
}
922+
923+
/// A `hatch` that never answers is killed at the shared probe budget
924+
/// and takes the same refusal as one too old.
925+
#[cfg(unix)]
926+
#[tokio::test]
927+
async fn a_hung_hatch_is_refused_within_the_probe_budget() {
928+
use std::os::unix::fs::PermissionsExt;
929+
let temp = tempfile::tempdir().unwrap();
930+
let root = temp.path().join("project");
931+
let bin = temp.path().join("bin");
932+
std::fs::create_dir(&root).unwrap();
933+
std::fs::create_dir(&bin).unwrap();
934+
let hatch = bin.join("hatch");
935+
std::fs::write(&hatch, "#!/bin/sh\nexec sleep 60\n").unwrap();
936+
std::fs::set_permissions(&hatch, std::fs::Permissions::from_mode(0o755)).unwrap();
937+
let path = std::env::join_paths([bin.as_path()]).unwrap();
938+
let start = std::time::Instant::now();
939+
let result = require_environment_context_support_with(&root, &|var| {
940+
(var == "PATH").then(|| path.clone())
941+
})
942+
.await;
943+
assert_eq!(result.unwrap_err().0, "pypi_hatch_unsupported");
944+
assert!(
945+
start.elapsed() < std::time::Duration::from_secs(40),
946+
"the probe budget must bound hatch, took {:?}",
947+
start.elapsed()
948+
);
949+
}
925950
}

0 commit comments

Comments
 (0)