Skip to content

Commit de7611f

Browse files
fix: shut down JSONRPC cleanly on EOF (Fixes #529) (#559)
Treat frame-boundary stdin EOF as normal server shutdown instead of repeatedly logging empty headers. Cancel admitted subprocess probes through ownership cleanup, without waiting indefinitely for blocked transport I/O. - Distinguish clean EOF from truncated frames and terminal transport failures; preserve errors that race shutdown. - Use bounded FIFO output with a dedicated owned stdout handle, explicit saturation errors, and documented cancellation semantics. - Supervise probe admission and cleanup; report cleanup failures or the three-second cleanup deadline explicitly. - Close test-client stdin and bound normal exit, forced termination, and reader teardown. - Add deterministic transport/output/supervisor coverage and native EOF, unread-output, broken-pipe, and active-descendant regressions. Validation: Windows and Linux affected suites and CI-feature native tests pass; 80 concurrent lifecycle regression cases pass. Workspace formatting/Clippy and targeted all-target Clippy pass. Independent Reviewer returned no findings. macOS and hosted quality results remain pending. Multi-header framing and envelope validation remain separate work under #532. Fixes #529 --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 094217e commit de7611f

17 files changed

Lines changed: 2638 additions & 125 deletions

File tree

‎.github/skills/rust-coding-skill/SKILL.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,7 @@ Before every Rust commit, run targeted tests and invoke the `rust-precommit` ski
9898
## Learnings
9999

100100
Do not execute freshly written scripts as concurrent Unix subprocess fixtures: spawning can fail with `ETXTBSY` (Text file busy). Prefer an existing interpreter such as `/bin/sh -c` with an inline script, or the existing test executable. Assert the typed runner outcome before checking an optional parsed result, so a spawn failure cannot masquerade as a successful negative parsing or timeout test.
101+
102+
For real-pipe EOF/EPIPE tests, create the pipe inside an isolated test subprocess when other test threads spawn children. Unix `CLOEXEC` closes descriptors at exec, not fork: a concurrent child can temporarily retain a reader, allowing the only write to succeed before the final reader disappears. A readiness handshake alone does not prevent this race. Keep the operation's measured deadline separate from setup, and make an outer fixture deadline cover readiness, waits both before and after forced termination, reader joins, and fallback `Drop` cleanup.
103+
104+
Use a per-worktree Cargo target directory when validating stacked changes so native fixtures cannot execute another worktree's stale binary. On WSL, run timing-sensitive Linux binaries from the native Linux filesystem rather than a Windows mount, where page faults can stall in filesystem RPC. When launching instrumented PET with `env_clear()`, retain `LLVM_PROFILE_FILE` exactly so child coverage reaches the collector instead of an uncollected default profile.

‎crates/pet-conda/src/conda_info.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,13 @@ impl CondaInfo {
107107
None
108108
}
109109
}
110+
Err(ProcessError::Cancelled) => {
111+
trace!(
112+
"Cancelled Conda probe during process shutdown: {:?}",
113+
executable
114+
);
115+
None
116+
}
110117
Err(err) => {
111118
if !is_missing_default_conda(using_default, &err) {
112119
warn!(
@@ -191,6 +198,7 @@ mod tests {
191198
);
192199
}
193200
for error in [
201+
ProcessError::Cancelled,
194202
ProcessError::Spawn(io::Error::from(io::ErrorKind::NotFound)),
195203
ProcessError::Io(io::Error::from(io::ErrorKind::BrokenPipe)),
196204
ProcessError::Timeout(Duration::from_secs(15)),

‎crates/pet-jsonrpc/src/lib.rs‎

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,12 @@
22
// Licensed under the MIT License.
33

44
use serde::{Deserialize, Serialize};
5-
use std::io::{self, Write};
65

6+
mod output;
77
pub mod server;
88

9+
pub use output::{initialize_output, output_error, shutdown_output};
10+
911
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
1012
#[serde(untagged)]
1113
pub enum RequestId {
@@ -24,51 +26,52 @@ impl From<u32> for RequestId {
2426
#[serde(rename_all = "camelCase")]
2527
#[derive(Debug)]
2628
struct AnyMethodMessage<T> {
27-
pub jsonrpc: String,
29+
pub jsonrpc: &'static str,
2830
pub method: &'static str,
2931
pub params: Option<T>,
3032
}
3133

3234
pub fn send_message<T: serde::Serialize>(method: &'static str, params: Option<T>) {
3335
let payload = AnyMethodMessage {
34-
jsonrpc: "2.0".to_string(),
36+
jsonrpc: "2.0",
3537
method,
3638
params,
3739
};
38-
let message = serde_json::to_string(&payload).unwrap();
39-
print!(
40-
"Content-Length: {}\r\nContent-Type: application/vscode-jsonrpc; charset=utf-8\r\n\r\n{}",
41-
message.len(),
42-
message
43-
);
44-
let _ = io::stdout().flush();
40+
output::send(&payload);
4541
}
42+
4643
pub fn send_reply<T: serde::Serialize>(id: &RequestId, payload: Option<T>) {
47-
let payload = serde_json::json!({
48-
"jsonrpc": "2.0",
49-
"result": payload,
50-
"id": id
44+
#[derive(Serialize)]
45+
struct Reply<'a, T> {
46+
jsonrpc: &'static str,
47+
result: Option<T>,
48+
id: &'a RequestId,
49+
}
50+
51+
output::send(&Reply {
52+
jsonrpc: "2.0",
53+
result: payload,
54+
id,
5155
});
52-
let message = serde_json::to_string(&payload).unwrap();
53-
print!(
54-
"Content-Length: {}\r\nContent-Type: application/vscode-jsonrpc; charset=utf-8\r\n\r\n{}",
55-
message.len(),
56-
message
57-
);
58-
let _ = io::stdout().flush();
5956
}
6057

6158
pub fn send_error(id: Option<&RequestId>, code: i32, message: String) {
62-
let payload = serde_json::json!({
63-
"jsonrpc": "2.0",
64-
"error": { "code": code, "message": message },
65-
"id": id
59+
#[derive(Serialize)]
60+
struct ErrorBody {
61+
code: i32,
62+
message: String,
63+
}
64+
65+
#[derive(Serialize)]
66+
struct ErrorReply<'a> {
67+
jsonrpc: &'static str,
68+
error: ErrorBody,
69+
id: Option<&'a RequestId>,
70+
}
71+
72+
output::send(&ErrorReply {
73+
jsonrpc: "2.0",
74+
error: ErrorBody { code, message },
75+
id,
6676
});
67-
let message = serde_json::to_string(&payload).unwrap();
68-
print!(
69-
"Content-Length: {}\r\nContent-Type: application/vscode-jsonrpc; charset=utf-8\r\n\r\n{}",
70-
message.len(),
71-
message
72-
);
73-
let _ = io::stdout().flush();
7477
}

0 commit comments

Comments
 (0)