Skip to content

Commit f3cddc8

Browse files
committed
fix: bound resolve spawn with timeout to prevent indefinite hangs (Fixes #463)
Replace the unbounded `Command::output()` call in `get_interpreter_details` with a `spawn()` + bounded `try_wait()` polling loop. On timeout the child is killed and `None` is returned so the resolve path completes promptly instead of stalling the foreground env-selection flow. Stale cached Python paths on Windows (Store stubs, vanished network shares, EDR-stalled `CreateProcess`) could otherwise block `resolve` for tens to hundreds of seconds, tripping the extension's hang watchdog. Telemetry showed p99 of 122s on Windows and `envSelection` hangs dominating across all three platforms. Adds a Unix regression test that proves a hanging fake executable is killed near the configured timeout rather than blocking the full sleep duration.
1 parent 8c75cf0 commit f3cddc8

1 file changed

Lines changed: 107 additions & 4 deletions

File tree

  • crates/pet-python-utils/src

‎crates/pet-python-utils/src/env.rs‎

Lines changed: 107 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,27 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4-
use log::{error, trace};
4+
use log::{error, trace, warn};
55
use pet_core::{arch::Architecture, env::PythonEnv, python_environment::PythonEnvironment};
66
use serde::{Deserialize, Serialize};
77
use std::{
88
path::{Path, PathBuf},
9-
time::SystemTime,
9+
process::Stdio,
10+
thread,
11+
time::{Duration, Instant, SystemTime},
1012
};
1113

1214
use crate::{cache::create_cache, executable::new_silent_command};
1315

1416
const PYTHON_INFO_JSON_SEPARATOR: &str = "093385e9-59f7-4a16-a604-14bf206256fe";
1517
const PYTHON_INFO_CMD:&str = "import json, sys; print('093385e9-59f7-4a16-a604-14bf206256fe');print(json.dumps({'version': '.'.join(str(n) for n in sys.version_info), 'sys_prefix': sys.prefix, 'executable': sys.executable, 'is64_bit': sys.maxsize > 2**32}))";
1618

19+
/// Maximum wall-clock time to wait for a spawned Python interpreter to print
20+
/// its info JSON before we give up and kill it. Stale cached paths on Windows
21+
/// (Store stubs, vanished network shares, EDR-stalled `CreateProcess`) can
22+
/// otherwise block `resolve` for tens to hundreds of seconds (Fixes #463).
23+
const RESOLVE_SPAWN_TIMEOUT: Duration = Duration::from_secs(15);
24+
1725
#[derive(Debug, Deserialize, Clone)]
1826
pub struct InterpreterInfo {
1927
pub version: String,
@@ -88,13 +96,66 @@ impl ResolvedPythonEnv {
8896
}
8997

9098
fn get_interpreter_details(executable: &Path) -> Option<ResolvedPythonEnv> {
99+
get_interpreter_details_with_timeout(executable, RESOLVE_SPAWN_TIMEOUT)
100+
}
101+
102+
fn get_interpreter_details_with_timeout(
103+
executable: &Path,
104+
timeout: Duration,
105+
) -> Option<ResolvedPythonEnv> {
91106
// Spawn the python exe and get the version, sys.prefix and sys.executable.
92107
let executable = executable.to_str()?;
93108
let start = SystemTime::now();
94109
trace!("Executing Python: {} -c {}", executable, PYTHON_INFO_CMD);
95-
let result = new_silent_command(executable)
110+
let mut child = match new_silent_command(executable)
96111
.args(["-c", PYTHON_INFO_CMD])
97-
.output();
112+
.stdin(Stdio::null())
113+
.stdout(Stdio::piped())
114+
.stderr(Stdio::piped())
115+
.spawn()
116+
{
117+
Ok(child) => child,
118+
Err(err) => {
119+
error!(
120+
"Failed to spawn Python to resolve info {:?}: {}",
121+
executable, err
122+
);
123+
return None;
124+
}
125+
};
126+
127+
// Poll for completion up to the timeout. A stale cached path on Windows
128+
// (Store stub, vanished network share, EDR-stalled `CreateProcess`) can
129+
// otherwise block `wait_with_output()` for tens to hundreds of seconds.
130+
let deadline = Instant::now() + timeout;
131+
loop {
132+
match child.try_wait() {
133+
Ok(Some(_status)) => break,
134+
Ok(None) => {
135+
if Instant::now() >= deadline {
136+
warn!(
137+
"Timed out after {:?} resolving Python via spawn for {:?}; killing child.",
138+
timeout, executable
139+
);
140+
let _ = child.kill();
141+
let _ = child.wait();
142+
return None;
143+
}
144+
thread::sleep(Duration::from_millis(25));
145+
}
146+
Err(err) => {
147+
error!(
148+
"Failed to wait on Python interpreter spawn for {:?}: {}",
149+
executable, err
150+
);
151+
let _ = child.kill();
152+
let _ = child.wait();
153+
return None;
154+
}
155+
}
156+
}
157+
158+
let result = child.wait_with_output();
98159
match result {
99160
Ok(output) => {
100161
let output = String::from_utf8(output.stdout).unwrap().trim().to_string();
@@ -143,3 +204,45 @@ fn get_interpreter_details(executable: &Path) -> Option<ResolvedPythonEnv> {
143204
}
144205
}
145206
}
207+
208+
#[cfg(all(test, unix))]
209+
mod tests {
210+
use super::*;
211+
use std::os::unix::fs::PermissionsExt;
212+
213+
/// Regression test for #463: a spawn that never exits must not block the
214+
/// resolve path indefinitely. We use a shell script that sleeps far longer
215+
/// than the test timeout and assert that the call returns None promptly
216+
/// (well under the script's sleep duration).
217+
#[test]
218+
fn get_interpreter_details_times_out_on_hanging_executable() {
219+
let tmp_dir = std::env::temp_dir().join(format!(
220+
"pet_resolve_timeout_{}_{}",
221+
std::process::id(),
222+
std::time::SystemTime::now()
223+
.duration_since(std::time::UNIX_EPOCH)
224+
.unwrap()
225+
.as_nanos()
226+
));
227+
std::fs::create_dir_all(&tmp_dir).unwrap();
228+
let fake_exe = tmp_dir.join("hangs");
229+
std::fs::write(&fake_exe, "#!/bin/sh\nsleep 60\n").unwrap();
230+
let mut perms = std::fs::metadata(&fake_exe).unwrap().permissions();
231+
perms.set_mode(0o755);
232+
std::fs::set_permissions(&fake_exe, perms).unwrap();
233+
234+
let start = Instant::now();
235+
let result = get_interpreter_details_with_timeout(&fake_exe, Duration::from_millis(200));
236+
let elapsed = start.elapsed();
237+
238+
let _ = std::fs::remove_file(&fake_exe);
239+
let _ = std::fs::remove_dir(&tmp_dir);
240+
241+
assert!(result.is_none(), "hanging spawn must return None");
242+
assert!(
243+
elapsed < Duration::from_secs(5),
244+
"spawn must be killed near the timeout (took {:?})",
245+
elapsed
246+
);
247+
}
248+
}

0 commit comments

Comments
 (0)