Skip to content
Open
Show file tree
Hide file tree
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
25 changes: 20 additions & 5 deletions src/cache/cache_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,25 @@ use crate::errors::*;
use fs_err as fs;
use std::fmt;
use std::io::{Cursor, Read, Seek, Write};
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use tempfile::NamedTempFile;
use zip::write::FileOptions;
use zip::{CompressionMethod, ZipArchive, ZipWriter};

/// Atomically persist a NamedTempFile: fsync, close the writable fd, rename.
///
/// Caller MUST hold FORK_LOCK.read() for the entire NamedTempFile lifetime
/// (from creation through this call). See FORK_LOCK in lib.rs.
pub(crate) fn persist_temp_file(tmp: NamedTempFile, dest: &Path) -> Result<()> {
tmp.as_file()
.sync_all()
.with_context(|| format!("failed to fsync temp file before persisting to {:?}", dest))?;
let tmp_path = tmp.into_temp_path();
tmp_path
.persist(dest)
.map_err(|e| anyhow::anyhow!("Failed to persist {:?} to {:?}: {}", e.path, dest, e.error))
}

/// Cache object sourced by a file.
#[derive(Clone)]
pub struct FileObjectSource {
Expand Down Expand Up @@ -147,13 +161,14 @@ impl CacheRead {
Some(d) => d,
None => bail!("Output file without a parent directory!"),
};
// Write the cache entry to a tempfile and then atomically
// move it to its final location so that other rustc invocations
// happening in parallel don't see a partially-written file.
// Hold FORK_LOCK while we have a writable fd to prevent
// fork() from inheriting it (causes ETXTBSY, see lib.rs).
let _fork_guard = crate::FORK_LOCK.read().unwrap();
let mut tmp = NamedTempFile::new_in(dir)?;
match (self.get_object(&key, &mut tmp), optional) {
(Ok(mode), _) => {
tmp.persist(&path)?;
persist_temp_file(tmp, &path)?;
drop(_fork_guard);
if let Some(mode) = mode {
set_file_mode(&path, mode)?;
}
Expand Down
11 changes: 9 additions & 2 deletions src/cache/disk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,16 @@ impl Storage for DiskCache {
self.pool
.spawn_blocking(move || {
let start = Instant::now();
// Hold FORK_LOCK while writable fd exists (see lib.rs).
let _fork_guard = crate::FORK_LOCK.read().unwrap();
let mut f = lru
.lock()
.unwrap()
.get_or_init()?
.prepare_add(key, data.len() as u64)?;
f.as_file_mut().write_all(&data)?;
lru.lock().unwrap().get().unwrap().commit(f)?;
drop(_fork_guard);
Ok(start.elapsed())
})
.await?
Expand Down Expand Up @@ -208,20 +211,24 @@ impl Storage for DiskCache {
}

let key = normalize_key(key);
// Hold FORK_LOCK while writable fd exists (see lib.rs).
let _fork_guard = crate::FORK_LOCK.read().unwrap();
let mut f = self
.preprocessor_cache
.lock()
.unwrap()
.get_or_init()?
.prepare_add(key, 0)?;
preprocessor_cache_entry.serialize_to(BufWriter::new(f.as_file_mut()))?;
Ok(self
let result = self
.preprocessor_cache
.lock()
.unwrap()
.get()
.unwrap()
.commit(f)?)
.commit(f);
drop(_fork_guard);
Ok(result?)
}
}

Expand Down
119 changes: 113 additions & 6 deletions src/compiler/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,110 @@ where
fn language(&self) -> Language;
}

/// Scan a command's arguments for `--out-dir` and extract the output directory path.
/// Returns `Some((index_of_value, path))` if found, `None` otherwise.
/// Handles both `--out-dir <path>` (separated) and `--out-dir=<path>` (joined).
fn find_out_dir(args: &[OsString]) -> Option<(usize, PathBuf)> {
let mut i = 0;
while i < args.len() {
let arg = args[i].to_string_lossy();
if arg == "--out-dir" {
if i + 1 < args.len() {
return Some((i + 1, PathBuf::from(&args[i + 1])));
}
} else if let Some(path) = arg.strip_prefix("--out-dir=") {
return Some((i, PathBuf::from(path)));
}
i += 1;
}
None
}

/// Execute a compile command, redirecting `--out-dir` to a temporary directory
/// and atomically renaming outputs to the real output directory afterward.
///
/// This prevents orphaned compiler processes (from killed cargo invocations)
/// from corrupting output files — they write to a temp dir that becomes
/// irrelevant when the next build creates its own temp dir.
async fn execute_with_atomic_out_dir<T>(
compile_cmd: &dyn CompileCommand<T>,
service: &server::SccacheService<T>,
creator: &T,
out_pretty: &str,
) -> Result<process::Output>
where
T: CommandCreatorSync,
{
let args = compile_cmd.get_arguments();

// Only redirect if --out-dir is present (rustc-specific)
let Some((out_dir_idx, real_out_dir)) = find_out_dir(&args) else {
return compile_cmd.execute(service, creator).await;
};

// Create temp dir inside the real out-dir (same filesystem for atomic rename)
let temp_dir = tempfile::Builder::new()
.prefix(".sccache-tmp-")
.tempdir_in(&real_out_dir)
.with_context(|| {
format!(
"failed to create temp dir in {:?} for atomic output",
real_out_dir
)
})?;
let temp_path = temp_dir.path().to_path_buf();

trace!(
"[{}]: Redirecting --out-dir from {:?} to {:?}",
out_pretty, real_out_dir, temp_path
);

// Build new argument list with --out-dir replaced
let mut new_args: Vec<OsString> = args.clone();
let orig_arg = new_args[out_dir_idx].to_string_lossy();
if orig_arg.starts_with("--out-dir=") {
new_args[out_dir_idx] = OsString::from(format!("--out-dir={}", temp_path.display()));
} else {
new_args[out_dir_idx] = OsString::from(&temp_path);
}

// Execute with a new command using the rewritten arguments
let mut cmd = creator
.clone()
.new_command_sync(compile_cmd.get_executable());
cmd.args(&new_args)
.env_clear()
.envs(compile_cmd.get_env_vars())
.current_dir(compile_cmd.get_cwd());
let output = run_input_output(cmd, None).await?;

// After compilation, atomically rename each output file to the real dir
if output.status.success() {
match std::fs::read_dir(&temp_path) {
Ok(entries) => {
for entry in entries {
let entry = entry?;
let file_name = entry.file_name();
let src = entry.path();
let dst = real_out_dir.join(&file_name);
std::fs::rename(&src, &dst)
.with_context(|| format!("failed to rename {:?} to {:?}", src, dst))?;
trace!("[{}]: Renamed {:?} -> {:?}", out_pretty, file_name, dst);
}
}
Err(e) => {
warn!(
"[{}]: Failed to read temp dir {:?}: {}",
out_pretty, temp_path, e
);
}
}
}
// temp_dir dropped here — removes the now-empty directory

Ok(output)
}

#[cfg(not(feature = "dist-client"))]
async fn dist_or_local_compile<T>(
service: &server::SccacheService<T>,
Expand All @@ -839,8 +943,7 @@ where
.context("Failed to generate compile commands")?;

debug!("[{}]: Compiling locally", out_pretty);
compile_cmd
.execute(&service, &creator)
execute_with_atomic_out_dir(compile_cmd.as_ref(), &service, &creator, &out_pretty)
.await
.map(move |o| (cacheable, DistType::NoDist, o))
}
Expand Down Expand Up @@ -873,10 +976,14 @@ where
Some(dc) => dc,
None => {
debug!("[{}]: Compiling locally", out_pretty);
return compile_cmd
.execute(service, &creator)
.await
.map(move |o| (cacheable, DistType::NoDist, o));
return execute_with_atomic_out_dir(
compile_cmd.as_ref(),
service,
&creator,
&out_pretty,
)
.await
.map(move |o| (cacheable, DistType::NoDist, o));
}
};

Expand Down
43 changes: 29 additions & 14 deletions src/compiler/rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ pub struct RustCompilation {
pub struct CrateTypes {
rlib: bool,
staticlib: bool,
others: Vec<String>,
}

/// Emit types that we will cache.
Expand Down Expand Up @@ -1075,6 +1076,7 @@ fn parse_arguments(arguments: &[OsString], cwd: &Path) -> CompilerArguments<Pars
let mut crate_types = CrateTypes {
rlib: false,
staticlib: false,
others: vec![],
};
let mut extra_filename = None;
let mut externs = vec![];
Expand Down Expand Up @@ -1125,10 +1127,16 @@ fn parse_arguments(arguments: &[OsString], cwd: &Path) -> CompilerArguments<Pars
})) => {
// We can't cache non-rlib/staticlib crates, because rustc invokes the
// system linker to link them, and we don't know about all the linker inputs.
// However, if SCCACHE_RUST_CRATE_TYPE_ALLOW_HASH is set, we allow caching
// all crate types. The env var value is hashed into the cache key so that
// machines with different linker setups get separate cache entries.
if !others.is_empty() {
let others: Vec<&str> = others.iter().map(String::as_str).collect();
let others_string = others.join(",");
cannot_cache!("crate-type", others_string)
if std::env::var("SCCACHE_RUST_CRATE_TYPE_ALLOW_HASH").is_err() {
let others: Vec<&str> = others.iter().map(String::as_str).collect();
let others_string = others.join(",");
cannot_cache!("crate-type", others_string)
}
crate_types.others.extend(others.iter().cloned());
}
crate_types.rlib |= rlib;
crate_types.staticlib |= staticlib;
Expand Down Expand Up @@ -1235,10 +1243,12 @@ fn parse_arguments(arguments: &[OsString], cwd: &Path) -> CompilerArguments<Pars
// If it's not an rlib and not a staticlib then crate-type wasn't passed,
// so it will usually be inferred as a binary, though the `#![crate_type`
// annotation may dictate otherwise - either way, we don't know what to do.
if let CrateTypes {
rlib: false,
staticlib: false,
} = crate_types
// Unless SCCACHE_RUST_CRATE_TYPE_ALLOW_HASH is set, in which case we
// allow caching regardless.
if !crate_types.rlib
&& !crate_types.staticlib
&& crate_types.others.is_empty()
&& std::env::var("SCCACHE_RUST_CRATE_TYPE_ALLOW_HASH").is_err()
{
cannot_cache!("crate-type", "No crate-type passed".to_owned())
}
Expand Down Expand Up @@ -1540,6 +1550,15 @@ where
cwd.hash(&mut HashToDigest { digest: &mut m });
// 10. The version of the compiler.
self.version.hash(&mut HashToDigest { digest: &mut m });
// 11. SCCACHE_RUST_CRATE_TYPE_ALLOW_HASH, if set and we have unsupported
// crate types: differentiates cache entries for machines with different
// linker setups.
if !self.parsed_args.crate_types.others.is_empty() {
if let Ok(val) = std::env::var("SCCACHE_RUST_CRATE_TYPE_ALLOW_HASH") {
m.update(b"SCCACHE_RUST_CRATE_TYPE_ALLOW_HASH=");
m.update(val.as_bytes());
}
}

// Turn arguments into a simple Vec<OsString> to calculate outputs.
let flat_os_string_arguments: Vec<OsString> = os_string_arguments
Expand Down Expand Up @@ -2161,13 +2180,8 @@ impl pkg::InputsPackager for RustInputsPackager {

// If we're just creating an rlib then the only thing inspected inside dependency rlibs is the
// metadata, in which case we can create a trimmed rlib (which is actually a .a) with the metadata
let can_trim_rlibs = matches!(
crate_types,
CrateTypes {
rlib: true,
staticlib: false,
}
);
let can_trim_rlibs =
crate_types.rlib && !crate_types.staticlib && crate_types.others.is_empty();

let mut builder = tar::Builder::new(wtr);

Expand Down Expand Up @@ -3477,6 +3491,7 @@ proc_macro false
crate_types: CrateTypes {
rlib: true,
staticlib: false,
others: vec![],
},
dep_info: None,
emit,
Expand Down
18 changes: 18 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,24 @@ pub mod server;
pub mod util;

use std::env;
use std::sync::RwLock;

/// Lock to prevent fork() from inheriting writable file descriptors.
///
/// The sccache daemon handles concurrent compile requests. The jobserver
/// crate's `configure()` uses `pre_exec`, which forces Rust's Command::spawn
/// to use fork+exec instead of posix_spawn. At fork time, the child inherits
/// ALL of the daemon's open file descriptors. If any fd has write access to
/// a file that cargo later hard-links and tries to execve(), the kernel
/// returns ETXTBSY because the forked child still holds the writable fd
/// between fork() and execve() (O_CLOEXEC only fires at execve, not fork).
///
/// To prevent this:
/// - Code that holds writable fds to output files takes a **read lock**
/// (multiple writers can proceed concurrently).
/// - Code that calls fork/spawn takes a **write lock**, ensuring no writable
/// fds exist at fork time.
pub static FORK_LOCK: RwLock<()> = RwLock::new(());

/// VERSION is the pkg version of sccache.
///
Expand Down
13 changes: 6 additions & 7 deletions src/lru_disk_cache/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,12 +343,8 @@ impl LruDiskCache {

/// Commit an entry coming from `LruDiskCache::prepare_add`.
pub fn commit(&mut self, entry: LruDiskCacheAddEntry) -> Result<()> {
let LruDiskCacheAddEntry {
mut file,
key,
size,
} = entry;
file.flush()?;
let LruDiskCacheAddEntry { file, key, size } = entry;
file.as_file().sync_all()?;
let real_size = file.as_file().metadata()?.len();
// If the file is larger than the size that had been advertized, ensure
// we have enough space for it.
Expand All @@ -361,7 +357,10 @@ impl LruDiskCache {
self.pending_size -= size;
let path = self.rel_to_abs_path(&key);
fs::create_dir_all(path.parent().unwrap())?;
file.persist(path).map_err(|e| e.error)?;
// Close the writable fd before rename to prevent fork() from
// inheriting it (see persist_temp_file in cache_io.rs).
let tmp_path = file.into_temp_path();
tmp_path.persist(&path).map_err(|e| e.error)?;
self.lru.insert(key, real_size);
Ok(())
}
Expand Down
5 changes: 5 additions & 0 deletions src/mock_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,14 @@ impl RunCommand for AsyncCommand {

let token = self.jobserver.acquire().await?;
let mut inner = tokio::process::Command::from(inner);
// Take FORK_LOCK write lock to ensure no writable fds exist at
// fork time. The jobserver uses pre_exec which forces fork+exec
// (not posix_spawn), and fork inherits all fds. See lib.rs.
let _fork_guard = crate::FORK_LOCK.write().unwrap();
let child = inner
.spawn()
.with_context(|| format!("failed to spawn {:?}", inner))?;
drop(_fork_guard);

Ok(Child {
inner: child,
Expand Down
Loading