Skip to content

Commit

Permalink
Kill the child process if it times out
Browse files Browse the repository at this point in the history
  • Loading branch information
jneem committed Mar 26, 2024
1 parent 076b4ca commit b8870c0
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 9 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ pyo3-build-config = "0.17.3"
regex = "1"
rustyline = "11.0"
rustyline-derive = "0.8.0"
scopeguard = "1.2.0"
serde = "1.0.164"
serde_json = "1.0.96"
serde_repr = "0.1"
Expand Down
1 change: 1 addition & 0 deletions lsp/nls/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ lsp-server.workspace = true
lsp-types.workspace = true
nickel-lang-core.workspace = true
regex.workspace = true
scopeguard.workspace = true
serde = { workspace = true, features = ["derive"] }
serde_json.workspace = true
thiserror.workspace = true
Expand Down
30 changes: 21 additions & 9 deletions lsp/nls/src/background.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,18 +165,31 @@ impl SupervisorState {
ret
}

// Evaluate the nickel file with the given uri, blocking until it completes or times out.
fn eval(&self, uri: &Url) -> anyhow::Result<Diagnostics> {
let path = std::env::current_exe()?;
let mut child = std::process::Command::new(path)
.arg("--background-eval")
.env("RUST_BACKTRACE", "1")
.stdout(std::process::Stdio::piped())
.stdin(std::process::Stdio::piped())
.spawn()?;
let mut tx = child
.stdin
.take()
.ok_or_else(|| anyhow!("failed to get worker stdin"))?;

let tx = child.stdin.take();
let rx = child.stdout.take();

scopeguard::defer! {
// If we successfully deserialized the response, the child should be just about done anyway
// (and killing an already-finished process isn't an error).
// Otherwise, we might have timed out waiting for the child, so kill it to reclaim resources.
if child.kill().is_ok() {
// We should wait on the child process to avoid having zombies, but if the
// kill failed then we skip waiting because we don't actually want to block.
let _ = child.wait();
}
}

let mut tx = tx.ok_or_else(|| anyhow!("failed to get worker stdin"))?;
let rx = rx.ok_or_else(|| anyhow!("failed to get worker stdout"))?;

let dependencies = self.dependencies(uri);
let eval = EvalRef {
Expand All @@ -188,10 +201,9 @@ impl SupervisorState {
};
bincode::serialize_into(&mut tx, &eval)?;

let result = run_with_timeout(move || child.wait_with_output(), EVAL_TIMEOUT)??;
Ok(bincode::deserialize_from(std::io::Cursor::new(
result.stdout,
))?)
let result = run_with_timeout(move || bincode::deserialize_from(rx), EVAL_TIMEOUT);

Ok(result??)
}

fn handle_command(&mut self, cmd: Command) {
Expand Down

0 comments on commit b8870c0

Please sign in to comment.