Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch panics in the server and transmit them as errors to the client. #1981

Merged
merged 1 commit into from
Nov 23, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 46 additions & 13 deletions src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use futures::future::FutureExt;
use futures::{future, stream, Sink, SinkExt, Stream, StreamExt, TryFutureExt};
use number_prefix::NumberPrefix;
use serde::{Deserialize, Serialize};
use std::cell::Cell;
use std::collections::HashMap;
use std::env;
use std::ffi::{OsStr, OsString};
Expand Down Expand Up @@ -392,12 +393,29 @@ impl DistClientContainer {
}
}

thread_local! {
/// catch_unwind doesn't provide panic location, so we store that
/// information via a panic hook to be used when catch_unwind
/// catches a panic.
static PANIC_LOCATION: Cell<Option<(String, u32, u32)>> = Cell::new(None);
}

/// Start an sccache server, listening on `port`.
///
/// Spins an event loop handling client connections until a client
/// requests a shutdown.
pub fn start_server(config: &Config, port: u16) -> Result<()> {
info!("start_server: port: {}", port);
let panic_hook = std::panic::take_hook();
std::panic::set_hook(Box::new(move |info| {
PANIC_LOCATION.with(|l| {
l.set(
info.location()
.map(|loc| (loc.file().to_string(), loc.line(), loc.column())),
)
});
panic_hook(info)
}));
let client = unsafe { Client::new() };
let runtime = tokio::runtime::Builder::new_multi_thread()
.enable_all()
Expand Down Expand Up @@ -1182,20 +1200,35 @@ where
let task = async move {
let dist_client = me.dist_client.get_client().await;
let result = match dist_client {
Ok(client) => {
hasher
.get_cached_or_compile(
client,
creator,
storage,
arguments,
cwd,
env_vars,
cache_control,
pool,
Ok(client) => std::panic::AssertUnwindSafe(hasher.get_cached_or_compile(
client,
creator,
storage,
arguments,
cwd,
env_vars,
cache_control,
pool,
))
.catch_unwind()
.await
.map_err(|e| {
let panic = e
.downcast_ref::<&str>()
.map(|s| &**s)
.or_else(|| e.downcast_ref::<String>().map(|s| &**s))
.unwrap_or("An unknown panic was caught.");
let thread = std::thread::current();
let thread_name = thread.name().unwrap_or("unnamed");
if let Some((file, line, column)) = PANIC_LOCATION.with(|l| l.take()) {
anyhow!(
"thread '{thread_name}' panicked at {file}:{line}:{column}: {panic}"
)
.await
}
} else {
anyhow!("thread '{thread_name}' panicked: {panic}")
}
})
.and_then(std::convert::identity),
Err(e) => Err(e),
};
let mut cache_write = None;
Expand Down
Loading