Skip to content

Commit

Permalink
Catch panics in the server and transmit them as errors to the client.
Browse files Browse the repository at this point in the history
Currently, when the server panics, the client never gets a response, and
the compilation gets stuck. The only indication of something going wrong
is in the logs if they are enabled.

With this change, the error is transmitted to the client, which then
fails and prints it. For instance, one of the out-of-bound accesses
fixed in mozilla#1980 now prints this on the client side before returning a
non-zero exit code:

sccache: encountered fatal error
sccache: error: thread 'tokio-runtime-worker' panicked at src/compiler/c.rs:612:25: range end index 7 out of range for slice of length 1
sccache: caused by: thread 'tokio-runtime-worker' panicked at src/compiler/c.rs:612:25: range end index 7 out of range for slice of length 1

Fixes mozilla#756
  • Loading branch information
glandium committed Nov 22, 2023
1 parent 68d0409 commit d004af4
Showing 1 changed file with 44 additions and 13 deletions.
57 changes: 44 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,27 @@ 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.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 +1198,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_else(|| "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.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

0 comments on commit d004af4

Please sign in to comment.