diff --git a/Cargo.lock b/Cargo.lock index 8c6f729c51c..c290bacfd85 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2164,6 +2164,7 @@ name = "gix-tempfile" version = "3.0.2" dependencies = [ "dashmap", + "document-features", "libc", "once_cell", "signal-hook", @@ -3486,15 +3487,6 @@ version = "0.6.28" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "456c603be3e8d448b072f410900c09faf164fbce2d480456f50eea6e25f9c848" -[[package]] -name = "remove_dir_all" -version = "0.5.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3acd125665422973a33ac9d3dd2df85edad0f4ae9b00dafb1a05e43a9f5ef8e7" -dependencies = [ - "winapi", -] - [[package]] name = "reqwest" version = "0.11.14" @@ -3930,16 +3922,15 @@ dependencies = [ [[package]] name = "tempfile" -version = "3.3.0" +version = "3.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5cdb1ef4eaeeaddc8fbd371e5017057064af0911902ef36b39801f67cc6d79e4" +checksum = "af18f7ae1acd354b992402e9ec5864359d693cd8a79dcbef59f76891701c1e95" dependencies = [ "cfg-if", "fastrand", - "libc", "redox_syscall", - "remove_dir_all", - "winapi", + "rustix", + "windows-sys 0.42.0", ] [[package]] diff --git a/Makefile b/Makefile index 64abd08db15..6c8123d7783 100644 --- a/Makefile +++ b/Makefile @@ -72,6 +72,8 @@ check: ## Build all code in suitable configurations cd gitoxide-core && if cargo check --all-features 2>/dev/null; then false; else true; fi cd gix-hash && cargo check --all-features \ && cargo check + cd gix-tempfile && cargo check --features signals \ + && cargo check cd gix-object && cargo check --all-features \ && cargo check --features verbose-object-parsing-errors cd gix-index && cargo check --features serde1 @@ -139,6 +141,8 @@ check: ## Build all code in suitable configurations unit-tests: ## run all unit tests cargo test --all + cd gix-tempfile && cargo test --features signals \ + && cargo test cd gix-features && cargo test && cargo test --all-features cd gix-ref/tests && cargo test --all-features cd gix-odb && cargo test && cargo test --all-features diff --git a/gix-tempfile/Cargo.toml b/gix-tempfile/Cargo.toml index 7b9f6c2e8dd..399177c5a03 100644 --- a/gix-tempfile/Cargo.toml +++ b/gix-tempfile/Cargo.toml @@ -9,6 +9,21 @@ edition = "2021" include = ["src/**/*", "LICENSE-*", "README.md", "CHANGELOG.md"] rust-version = "1.64" +[[example]] +name = "delete-tempfiles-on-sigterm" +path = "examples/delete-tempfiles-on-sigterm.rs" +required-features = ["signals"] + +[[example]] +name = "delete-tempfiles-on-sigterm-interactive" +path = "examples/delete-tempfiles-on-sigterm-interactive.rs" +required-features = ["signals"] + +[[example]] +name = "try-deadlock-on-cleanup" +path = "examples/try-deadlock-on-cleanup.rs" +required-features = ["signals"] + [lib] doctest = false test = true @@ -16,9 +31,22 @@ test = true [dependencies] dashmap = "5.1.0" once_cell = { version = "1.8.0", default-features = false, features = ["race", "std"] } -signal-hook = { version = "0.3.9", default-features = false } -signal-hook-registry = "1.4.0" -tempfile = "3.2.0" +tempfile = "3.4.0" + +signal-hook = { version = "0.3.9", default-features = false, optional = true } +signal-hook-registry = { version = "1.4.0", optional = true } + +document-features = { version = "0.2.0", optional = true } + +[features] +default = [] +signals = ["dep:signal-hook", "dep:signal-hook-registry"] [target.'cfg(not(windows))'.dependencies] libc = { version = "0.2.98", default-features = false } + +[package.metadata.docs.rs] +all-features = true +features = ["document-features"] +rustdoc-args = ["--cfg", "docsrs"] + diff --git a/gix-tempfile/examples/delete-tempfiles-on-sigterm-interactive.rs b/gix-tempfile/examples/delete-tempfiles-on-sigterm-interactive.rs index 8eb93fd6031..8843b66c358 100644 --- a/gix-tempfile/examples/delete-tempfiles-on-sigterm-interactive.rs +++ b/gix-tempfile/examples/delete-tempfiles-on-sigterm-interactive.rs @@ -1,9 +1,14 @@ -use std::path::PathBuf; - -use gix_tempfile::{AutoRemove, ContainingDirectory}; +#[cfg(not(feature = "signals"))] +fn main() { + panic!("The `signals` feature needs to be set to compile this example"); +} +#[cfg(feature = "signals")] fn main() -> std::io::Result<()> { - gix_tempfile::setup(Default::default()); + use gix_tempfile::{AutoRemove, ContainingDirectory}; + use std::path::PathBuf; + + gix_tempfile::signal::setup(Default::default()); let filepath = PathBuf::new().join("writable-tempfile.ext"); let markerpath = PathBuf::new().join("marker.ext"); let _tempfile = gix_tempfile::writable_at(&filepath, ContainingDirectory::Exists, AutoRemove::Tempfile)?; diff --git a/gix-tempfile/examples/delete-tempfiles-on-sigterm.rs b/gix-tempfile/examples/delete-tempfiles-on-sigterm.rs index 72919a28882..4906fe72560 100644 --- a/gix-tempfile/examples/delete-tempfiles-on-sigterm.rs +++ b/gix-tempfile/examples/delete-tempfiles-on-sigterm.rs @@ -1,12 +1,17 @@ -use std::{ - io::{stdout, Write}, - path::PathBuf, -}; - -use gix_tempfile::{AutoRemove, ContainingDirectory}; +#[cfg(not(feature = "signals"))] +fn main() { + panic!("The `signals` feature needs to be set to compile this example"); +} +#[cfg(feature = "signals")] fn main() -> std::io::Result<()> { - gix_tempfile::setup(Default::default()); + use gix_tempfile::{AutoRemove, ContainingDirectory}; + use std::{ + io::{stdout, Write}, + path::PathBuf, + }; + + gix_tempfile::signal::setup(Default::default()); let filepath = PathBuf::new().join("tempfile.ext"); let _tempfile = gix_tempfile::mark_at(&filepath, ContainingDirectory::Exists, AutoRemove::Tempfile)?; assert!(filepath.is_file(), "a tempfile was created"); diff --git a/gix-tempfile/examples/try-deadlock-on-cleanup.rs b/gix-tempfile/examples/try-deadlock-on-cleanup.rs index e0625b2649c..81d99258154 100644 --- a/gix-tempfile/examples/try-deadlock-on-cleanup.rs +++ b/gix-tempfile/examples/try-deadlock-on-cleanup.rs @@ -1,15 +1,21 @@ -use std::{ - path::Path, - sync::{ - atomic::{AtomicUsize, Ordering}, - Arc, - }, - time::Duration, -}; - -use gix_tempfile::{handle::Writable, AutoRemove, ContainingDirectory, Handle}; +#[cfg(not(feature = "signals"))] +fn main() { + panic!("The `signals` feature needs to be set to compile this example"); +} +#[cfg(feature = "signals")] fn main() -> Result<(), Box> { + use std::{ + path::Path, + sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, + }, + time::Duration, + }; + + use gix_tempfile::{handle::Writable, AutoRemove, ContainingDirectory, Handle}; + let secs_to_run: usize = std::env::args() .nth(1) .ok_or("the first argument is the amount of seconds to run")? @@ -19,7 +25,7 @@ fn main() -> Result<(), Box> { let tempfiles_created = Arc::new(AtomicUsize::default()); let tempfiles_registry_locked = Arc::new(AtomicUsize::default()); let signal_raised = Arc::new(AtomicUsize::default()); - gix_tempfile::setup(gix_tempfile::SignalHandlerMode::DeleteTempfilesOnTermination); + gix_tempfile::signal::setup(gix_tempfile::signal::handler::Mode::DeleteTempfilesOnTermination); for tid in 0..suspected_dashmap_block_size { std::thread::spawn({ @@ -83,15 +89,15 @@ fn main() -> Result<(), Box> { signal_hook::low_level::raise(signal_hook::consts::SIGINT)?; signal_raised.fetch_add(1, Ordering::SeqCst); } -} -fn tempfile_for_thread_or_panic(tid: i32, tmp: &Path, count: &AtomicUsize) -> Handle { - let res = gix_tempfile::writable_at( - tmp.join(format!("thread-{tid}")), - ContainingDirectory::Exists, - AutoRemove::Tempfile, - ) - .unwrap(); - count.fetch_add(1, Ordering::SeqCst); - res + fn tempfile_for_thread_or_panic(tid: i32, tmp: &Path, count: &AtomicUsize) -> Handle { + let res = gix_tempfile::writable_at( + tmp.join(format!("thread-{tid}")), + ContainingDirectory::Exists, + AutoRemove::Tempfile, + ) + .unwrap(); + count.fetch_add(1, Ordering::SeqCst); + res + } } diff --git a/gix-tempfile/src/forksafe.rs b/gix-tempfile/src/forksafe.rs index 804db1ed634..1da7b2b2c5a 100644 --- a/gix-tempfile/src/forksafe.rs +++ b/gix-tempfile/src/forksafe.rs @@ -1,4 +1,4 @@ -use std::{io::Write, path::Path}; +use std::path::Path; use tempfile::{NamedTempFile, TempPath}; @@ -88,6 +88,7 @@ impl ForksafeTempfile { } pub fn drop_without_deallocation(self) { + use std::io::Write; let temppath = match self.inner { TempfileOrTemppath::Tempfile(file) => { let (mut file, temppath) = file.into_parts(); diff --git a/gix-tempfile/src/handle.rs b/gix-tempfile/src/handle.rs index df857107439..f20e237ee1a 100644 --- a/gix-tempfile/src/handle.rs +++ b/gix-tempfile/src/handle.rs @@ -3,7 +3,7 @@ use std::{io, path::Path}; use tempfile::{NamedTempFile, TempPath}; -use crate::{AutoRemove, ContainingDirectory, ForksafeTempfile, Handle, NEXT_MAP_INDEX, REGISTER}; +use crate::{AutoRemove, ContainingDirectory, ForksafeTempfile, Handle, NEXT_MAP_INDEX, REGISTRY}; /// Marker to signal the Registration is an open file able to be written to. #[derive(Debug)] @@ -45,7 +45,7 @@ impl Handle<()> { ForksafeTempfile::new(builder.rand_bytes(0).tempfile_in(parent_dir)?, cleanup, mode) }; let id = NEXT_MAP_INDEX.fetch_add(1, std::sync::atomic::Ordering::SeqCst); - expect_none(REGISTER.insert(id, Some(tempfile))); + expect_none(REGISTRY.insert(id, Some(tempfile))); Ok(id) } @@ -57,7 +57,7 @@ impl Handle<()> { ) -> io::Result { let containing_directory = directory.resolve(containing_directory.as_ref())?; let id = NEXT_MAP_INDEX.fetch_add(1, std::sync::atomic::Ordering::SeqCst); - expect_none(REGISTER.insert( + expect_none(REGISTRY.insert( id, Some(ForksafeTempfile::new( NamedTempFile::new_in(containing_directory)?, @@ -86,7 +86,7 @@ impl Handle { /// /// It's a theoretical possibility that the file isn't present anymore if signals interfere, hence the `Option` pub fn take(self) -> Option { - let res = REGISTER.remove(&self.id); + let res = REGISTRY.remove(&self.id); std::mem::forget(self); res.and_then(|(_k, v)| v.map(|v| v.into_temppath())) } @@ -123,7 +123,7 @@ impl Handle { /// /// It's a theoretical possibility that the file isn't present anymore if signals interfere, hence the `Option` pub fn take(self) -> Option { - let res = REGISTER.remove(&self.id); + let res = REGISTRY.remove(&self.id); std::mem::forget(self); res.and_then(|(_k, v)| v.map(|v| v.into_tempfile().expect("correct runtime typing"))) } @@ -134,17 +134,17 @@ impl Handle { /// it right after to perform more updates of this kind in other tempfiles. When all succeed, they can be renamed one after /// another. pub fn close(self) -> std::io::Result> { - match REGISTER.remove(&self.id) { + match REGISTRY.remove(&self.id) { Some((id, Some(t))) => { std::mem::forget(self); - expect_none(REGISTER.insert(id, Some(t.close()))); + expect_none(REGISTRY.insert(id, Some(t.close()))); Ok(Handle:: { id, _marker: Default::default(), }) } None | Some((_, None)) => Err(std::io::Error::new( - std::io::ErrorKind::Interrupted, + std::io::ErrorKind::NotFound, format!("The tempfile with id {} wasn't available anymore", self.id), )), } @@ -162,14 +162,14 @@ impl Handle { /// The caller must assure that the signal handler for cleanup will be followed by an abort call so that /// this code won't run again on a removed instance. An error will occur otherwise. pub fn with_mut(&mut self, once: impl FnOnce(&mut NamedTempFile) -> T) -> std::io::Result { - match REGISTER.remove(&self.id) { + match REGISTRY.remove(&self.id) { Some((id, Some(mut t))) => { let res = once(t.as_mut_tempfile().expect("correct runtime typing")); - expect_none(REGISTER.insert(id, Some(t))); + expect_none(REGISTRY.insert(id, Some(t))); Ok(res) } None | Some((_, None)) => Err(std::io::Error::new( - std::io::ErrorKind::Interrupted, + std::io::ErrorKind::NotFound, format!("The tempfile with id {} wasn't available anymore", self.id), )), } @@ -210,7 +210,7 @@ pub mod persist { use crate::{ handle::{expect_none, Closed, Writable}, - Handle, REGISTER, + Handle, REGISTRY, }; mod error { @@ -247,7 +247,7 @@ pub mod persist { /// Note that it might not exist anymore if an interrupt handler managed to steal it and allowed the program to return to /// its normal flow. pub fn persist(self, path: impl AsRef) -> Result, Error> { - let res = REGISTER.remove(&self.id); + let res = REGISTRY.remove(&self.id); match res.and_then(|(_k, v)| v.map(|v| v.persist(path))) { Some(Ok(Some(file))) => { @@ -259,7 +259,7 @@ pub mod persist { Ok(None) } Some(Err((err, tempfile))) => { - expect_none(REGISTER.insert(self.id, Some(tempfile))); + expect_none(REGISTRY.insert(self.id, Some(tempfile))); Err(Error:: { error: err, handle: self, @@ -274,7 +274,7 @@ pub mod persist { /// Persist this tempfile to replace the file at the given `path` if necessary, in a way that recovers the original instance /// on error. pub fn persist(self, path: impl AsRef) -> Result<(), Error> { - let res = REGISTER.remove(&self.id); + let res = REGISTRY.remove(&self.id); match res.and_then(|(_k, v)| v.map(|v| v.persist(path))) { None | Some(Ok(None)) => { @@ -282,7 +282,7 @@ pub mod persist { Ok(()) } Some(Err((err, tempfile))) => { - expect_none(REGISTER.insert(self.id, Some(tempfile))); + expect_none(REGISTRY.insert(self.id, Some(tempfile))); Err(Error:: { error: err, handle: self, @@ -312,7 +312,7 @@ fn expect_none(v: Option) { impl Drop for Handle { fn drop(&mut self) { - if let Some((_id, Some(tempfile))) = REGISTER.remove(&self.id) { + if let Some((_id, Some(tempfile))) = REGISTRY.remove(&self.id) { tempfile.drop_impl(); } } diff --git a/gix-tempfile/src/handler.rs b/gix-tempfile/src/handler.rs deleted file mode 100644 index 7b17b144ad2..00000000000 --- a/gix-tempfile/src/handler.rs +++ /dev/null @@ -1,83 +0,0 @@ -//! -use std::sync::atomic::Ordering; - -use crate::{SignalHandlerMode, NEXT_MAP_INDEX, REGISTER, SIGNAL_HANDLER_MODE}; - -/// Remove all tempfiles still registered on our global registry. -/// -/// # Safety -/// Note that Mutexes of any kind are not allowed, and so aren't allocation or deallocation of memory. -/// We are using lock-free datastructures and sprinkle in `std::mem::forget` to avoid deallocating. -/// Most importantly, we use `try_lock()` which uses an atomic int only without waiting, making our register safe to use, -/// at the expense of possibly missing a lock file if another thread wants to obtain it or put it back -/// (i.e. mutates the register shard). -pub fn cleanup_tempfiles() { - let current_pid = std::process::id(); - let one_past_last_index = NEXT_MAP_INDEX.load(Ordering::SeqCst); - for idx in 0..one_past_last_index { - if let Some(entry) = REGISTER.try_entry(idx) { - entry.and_modify(|tempfile| { - if tempfile - .as_ref() - .map_or(false, |tf| tf.owning_process_id == current_pid) - { - if let Some(tempfile) = tempfile.take() { - tempfile.drop_without_deallocation(); - } - } - }); - } - } -} - -/// On linux we can handle the actual signal as we know it. -#[cfg(not(windows))] -pub(crate) fn cleanup_tempfiles_nix(sig: &libc::siginfo_t) { - cleanup_tempfiles(); - let restore_original_behaviour = SignalHandlerMode::DeleteTempfilesOnTerminationAndRestoreDefaultBehaviour as usize; - if SIGNAL_HANDLER_MODE.load(std::sync::atomic::Ordering::SeqCst) == restore_original_behaviour { - signal_hook::low_level::emulate_default_handler(sig.si_signo).ok(); - } -} - -/// On windows, assume sig-term and emulate sig-term unconditionally. -#[cfg(windows)] -pub(crate) fn cleanup_tempfiles_windows() { - cleanup_tempfiles(); - let restore_original_behaviour = SignalHandlerMode::DeleteTempfilesOnTerminationAndRestoreDefaultBehaviour as usize; - if SIGNAL_HANDLER_MODE.load(std::sync::atomic::Ordering::SeqCst) == restore_original_behaviour { - signal_hook::low_level::emulate_default_handler(signal_hook::consts::SIGTERM).ok(); - } -} - -#[cfg(test)] -mod tests { - use std::path::Path; - - use crate::{AutoRemove, ContainingDirectory}; - - fn filecount_in(path: impl AsRef) -> usize { - std::fs::read_dir(path).expect("valid dir").count() - } - - #[test] - fn various_termination_signals_remove_tempfiles_unconditionally() -> Result<(), Box> { - crate::setup(Default::default()); - let dir = tempfile::tempdir()?; - for sig in signal_hook::consts::TERM_SIGNALS { - let _tempfile = crate::new(dir.path(), ContainingDirectory::Exists, AutoRemove::Tempfile)?; - assert_eq!( - filecount_in(dir.path()), - 1, - "only one tempfile exists no matter the iteration" - ); - signal_hook::low_level::raise(*sig)?; - assert_eq!( - filecount_in(dir.path()), - 0, - "the signal triggers removal but won't terminate the process (anymore)" - ); - } - Ok(()) - } -} diff --git a/gix-tempfile/src/lib.rs b/gix-tempfile/src/lib.rs index 7a5977fe499..47de6d32eb1 100644 --- a/gix-tempfile/src/lib.rs +++ b/gix-tempfile/src/lib.rs @@ -1,7 +1,7 @@ //! git-style registered tempfiles that are removed upon typical termination signals. //! //! To register signal handlers in a typical application that doesn't have its own, call -//! [`gix_tempfile::setup(Default::default())`][setup()] before creating the first tempfile. +//! [`gix_tempfile::signal::setup(Default::default())`][signal::setup()] before creating the first tempfile. //! //! Signal handlers are powered by [`signal-hook`] to get notified when the application is told to shut down //! to assure tempfiles are deleted. The deletion is filtered by process id to allow forks to have their own @@ -9,11 +9,11 @@ //! //! ### Initial Setup //! -//! As no handlers for `TERMination` are installed, it is required to call [`setup()`] before creating the first tempfile. +//! As no handlers for `TERMination` are installed, it is required to call [`signal::setup()`] before creating the first tempfile. //! This also allows to control how `git-tempfiles` integrates with other handlers under application control. //! //! As a general rule of thumb, use `Default::default()` as argument to emulate the default behaviour and -//! abort the process after cleaning temporary files. Read more about options in [SignalHandlerMode]. +//! abort the process after cleaning temporary files. Read more about options in [signal::handler::Mode]. //! //! # Limitations //! @@ -24,6 +24,13 @@ //! but not others. Any other operation dealing with the tempfile suffers from the same issue. //! //! [signal-hook]: https://docs.rs/signal-hook +//! +//! ## Feature Flags +#![cfg_attr( + feature = "document-features", + cfg_attr(doc, doc = ::document_features::document_features!()) +)] +#![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))] #![deny(missing_docs, rust_2018_idioms, unsafe_code)] use std::{ @@ -39,7 +46,9 @@ use once_cell::sync::Lazy; mod fs; pub use fs::{create_dir, remove_dir}; -pub mod handler; +#[cfg(feature = "signals")] +/// signal setup and reusable handlers. +pub mod signal; mod forksafe; use forksafe::ForksafeTempfile; @@ -47,22 +56,24 @@ use forksafe::ForksafeTempfile; pub mod handle; use crate::handle::{Closed, Writable}; -static SIGNAL_HANDLER_MODE: AtomicUsize = AtomicUsize::new(SignalHandlerMode::None as usize); +/// +pub mod registry; + static NEXT_MAP_INDEX: AtomicUsize = AtomicUsize::new(0); -static REGISTER: Lazy>> = Lazy::new(|| { - let mode = SIGNAL_HANDLER_MODE.load(std::sync::atomic::Ordering::SeqCst); - if mode != SignalHandlerMode::None as usize { +static REGISTRY: Lazy>> = Lazy::new(|| { + #[cfg(feature = "signals")] + if signal::handler::MODE.load(std::sync::atomic::Ordering::SeqCst) != signal::handler::Mode::None as usize { for sig in signal_hook::consts::TERM_SIGNALS { // SAFETY: handlers are considered unsafe because a lot can go wrong. See `cleanup_tempfiles()` for details on safety. #[allow(unsafe_code)] unsafe { #[cfg(not(windows))] { - signal_hook_registry::register_sigaction(*sig, handler::cleanup_tempfiles_nix) + signal_hook_registry::register_sigaction(*sig, signal::handler::cleanup_tempfiles_nix) } #[cfg(windows)] { - signal_hook::low_level::register(*sig, handler::cleanup_tempfiles_windows) + signal_hook::low_level::register(*sig, signal::handler::cleanup_tempfiles_windows) } } .expect("signals can always be installed"); @@ -71,32 +82,6 @@ static REGISTER: Lazy>> = Lazy::new(|| { DashMap::new() }); -/// Define how our signal handlers act -#[derive(Debug, Clone, Copy, Ord, PartialOrd, Eq, PartialEq)] -pub enum SignalHandlerMode { - /// Do not install a signal handler at all, but have somebody else call our handler directly. - None = 0, - /// Delete all remaining registered tempfiles on termination. - DeleteTempfilesOnTermination = 1, - /// Delete all remaining registered tempfiles on termination and emulate the default handler behaviour. - /// - /// This typically leads to the process being aborted. - DeleteTempfilesOnTerminationAndRestoreDefaultBehaviour = 2, -} - -impl Default for SignalHandlerMode { - /// By default we will emulate the default behaviour and abort the process. - /// - /// While testing, we will not abort the process. - fn default() -> Self { - if cfg!(test) { - SignalHandlerMode::DeleteTempfilesOnTermination - } else { - SignalHandlerMode::DeleteTempfilesOnTerminationAndRestoreDefaultBehaviour - } - } -} - /// A type expressing the ways we can deal with directories containing a tempfile. #[derive(Debug, Clone, Copy, Ord, PartialOrd, Eq, PartialEq)] pub enum ContainingDirectory { @@ -179,27 +164,3 @@ pub fn mark_at( ) -> io::Result> { Handle::::at(path, directory, cleanup) } - -/// Initialize signal handlers and other state to keep track of tempfiles, and **must be called before the first tempfile is created**, -/// allowing to set the `mode` in which signal handlers are installed. -/// -/// Only has an effect the first time it is called. -/// -/// Note that it is possible to not call this function and instead call [handler::cleanup_tempfiles()][crate::handler::cleanup_tempfiles()] -/// from a handler under the applications control. -pub fn setup(mode: SignalHandlerMode) { - SIGNAL_HANDLER_MODE.store(mode as usize, std::sync::atomic::Ordering::SeqCst); - Lazy::force(®ISTER); -} - -/// DO NOT USE - use [`setup()`] instead. -/// -/// Indeed this is merely the old name of `setup()`, which is now a required part of configuring gix-tempfile. -#[deprecated( - since = "2.0.0", - note = "call setup(…) instead, this function will be removed in the next major release" -)] -#[doc(hidden)] -pub fn force_setup(mode: SignalHandlerMode) { - setup(mode) -} diff --git a/gix-tempfile/src/registry.rs b/gix-tempfile/src/registry.rs new file mode 100644 index 00000000000..a6e629c8fdc --- /dev/null +++ b/gix-tempfile/src/registry.rs @@ -0,0 +1,45 @@ +use crate::{NEXT_MAP_INDEX, REGISTRY}; +use std::sync::atomic::Ordering; + +/// Remove all tempfiles still registered on our global registry, and leak their data to be signal-safe. +/// This happens on a best-effort basis with all errors being ignored. +/// +/// # Safety +/// Note that Mutexes of any kind are not allowed, and so aren't allocation or deallocation of memory. +/// We are using lock-free datastructures and sprinkle in `std::mem::forget` to avoid deallocating. +/// Most importantly, we use `try_lock()` which uses an atomic int only without blocking, making our register method safe to use, +/// at the expense of possibly missing a lock file if another thread wants to obtain it or put it back +/// (i.e. mutates the registry shard). +pub fn cleanup_tempfiles_signal_safe() { + let current_pid = std::process::id(); + let one_past_last_index = NEXT_MAP_INDEX.load(Ordering::SeqCst); + for idx in 0..one_past_last_index { + if let Some(entry) = REGISTRY.try_entry(idx) { + entry.and_modify(|tempfile| { + if tempfile + .as_ref() + .map_or(false, |tf| tf.owning_process_id == current_pid) + { + if let Some(tempfile) = tempfile.take() { + tempfile.drop_without_deallocation(); + } + } + }); + } + } +} + +/// Remove all tempfiles still registered on our global registry. +/// This happens on a best-effort basis with all errors being ignored. +/// +/// # Note +/// +/// Must not be called from within signal hooks. For that, use [`cleanup_tempfiles_signal_safe()`]. +pub fn cleanup_tempfiles() { + let current_pid = std::process::id(); + REGISTRY.iter_mut().for_each(|mut tf| { + if tf.as_ref().map_or(false, |tf| tf.owning_process_id == current_pid) { + tf.take(); + } + }); +} diff --git a/gix-tempfile/src/signal.rs b/gix-tempfile/src/signal.rs new file mode 100644 index 00000000000..3e0e7729c17 --- /dev/null +++ b/gix-tempfile/src/signal.rs @@ -0,0 +1,100 @@ +use crate::REGISTRY; +use once_cell::sync::Lazy; + +/// Initialize signal handlers and other state to keep track of tempfiles, and **must be called before the first tempfile is created**, +/// allowing to set the `mode` in which signal handlers are installed. +/// +/// Only has an effect the first time it is called. +/// +/// Note that it is possible to not call this function and instead call +/// [registry::cleanup_tempfiles_signal_safe()][crate::registry::cleanup_tempfiles_signal_safe()] +/// from a signal handler under the application's control. +pub fn setup(mode: handler::Mode) { + handler::MODE.store(mode as usize, std::sync::atomic::Ordering::SeqCst); + Lazy::force(®ISTRY); +} + +/// +pub mod handler { + use std::sync::atomic::AtomicUsize; + + pub(crate) static MODE: AtomicUsize = AtomicUsize::new(Mode::None as usize); + + /// Define how our signal handlers act + #[derive(Debug, Clone, Copy, Ord, PartialOrd, Eq, PartialEq)] + pub enum Mode { + /// Do not install a signal handler at all, but have somebody else call our handler directly. + None = 0, + /// Delete all remaining registered tempfiles on termination. + DeleteTempfilesOnTermination = 1, + /// Delete all remaining registered tempfiles on termination and emulate the default handler behaviour. + /// + /// This typically leads to the process being aborted. + DeleteTempfilesOnTerminationAndRestoreDefaultBehaviour = 2, + } + + impl Default for Mode { + /// By default we will emulate the default behaviour and abort the process. + /// + /// While testing, we will not abort the process. + fn default() -> Self { + if cfg!(test) { + Mode::DeleteTempfilesOnTermination + } else { + Mode::DeleteTempfilesOnTerminationAndRestoreDefaultBehaviour + } + } + } + + /// On linux we can handle the actual signal as we know it. + #[cfg(not(windows))] + pub(crate) fn cleanup_tempfiles_nix(sig: &libc::siginfo_t) { + crate::registry::cleanup_tempfiles_signal_safe(); + let restore_original_behaviour = Mode::DeleteTempfilesOnTerminationAndRestoreDefaultBehaviour as usize; + if MODE.load(std::sync::atomic::Ordering::SeqCst) == restore_original_behaviour { + signal_hook::low_level::emulate_default_handler(sig.si_signo).ok(); + } + } + + /// On windows, assume sig-term and emulate sig-term unconditionally. + #[cfg(windows)] + pub(crate) fn cleanup_tempfiles_windows() { + crate::registry::cleanup_tempfiles_signal_safe(); + let restore_original_behaviour = Mode::DeleteTempfilesOnTerminationAndRestoreDefaultBehaviour as usize; + if MODE.load(std::sync::atomic::Ordering::SeqCst) == restore_original_behaviour { + signal_hook::low_level::emulate_default_handler(signal_hook::consts::SIGTERM).ok(); + } + } + + #[cfg(test)] + mod tests { + use std::path::Path; + + use crate::{AutoRemove, ContainingDirectory}; + + fn filecount_in(path: impl AsRef) -> usize { + std::fs::read_dir(path).expect("valid dir").count() + } + + #[test] + fn various_termination_signals_remove_tempfiles_unconditionally() -> Result<(), Box> { + crate::signal::setup(Default::default()); + let dir = tempfile::tempdir()?; + for sig in signal_hook::consts::TERM_SIGNALS { + let _tempfile = crate::new(dir.path(), ContainingDirectory::Exists, AutoRemove::Tempfile)?; + assert_eq!( + filecount_in(dir.path()), + 1, + "only one tempfile exists no matter the iteration" + ); + signal_hook::low_level::raise(*sig)?; + assert_eq!( + filecount_in(dir.path()), + 0, + "the signal triggers removal but won't terminate the process (anymore)" + ); + } + Ok(()) + } + } +} diff --git a/gix-tempfile/tests/tempfile/mod.rs b/gix-tempfile/tests/tempfile/mod.rs index 4a1c983f56c..b22fc867bbf 100644 --- a/gix-tempfile/tests/tempfile/mod.rs +++ b/gix-tempfile/tests/tempfile/mod.rs @@ -1,11 +1,15 @@ mod fs; mod handle; +mod registry; +#[cfg(feature = "signals")] mod setup { #[test] fn can_be_called_multiple_times() { // we could probably be smart and figure out that this does the right thing, but… it's good enough it won't fail ;). - gix_tempfile::setup(gix_tempfile::SignalHandlerMode::DeleteTempfilesOnTermination); - gix_tempfile::setup(gix_tempfile::SignalHandlerMode::DeleteTempfilesOnTerminationAndRestoreDefaultBehaviour); + gix_tempfile::signal::setup(gix_tempfile::signal::handler::Mode::DeleteTempfilesOnTermination); + gix_tempfile::signal::setup( + gix_tempfile::signal::handler::Mode::DeleteTempfilesOnTerminationAndRestoreDefaultBehaviour, + ); } } diff --git a/gix-tempfile/tests/tempfile/registry.rs b/gix-tempfile/tests/tempfile/registry.rs new file mode 100644 index 00000000000..219290ad938 --- /dev/null +++ b/gix-tempfile/tests/tempfile/registry.rs @@ -0,0 +1,30 @@ +use std::io::Write; +use std::path::Path; + +use gix_tempfile::{AutoRemove, ContainingDirectory}; + +fn filecount_in(path: impl AsRef) -> usize { + std::fs::read_dir(path).expect("valid dir").count() +} + +#[test] +fn cleanup_tempfiles() -> Result<(), Box> { + let dir = tempfile::tempdir()?; + let mut tempfile = gix_tempfile::new(dir.path(), ContainingDirectory::Exists, AutoRemove::Tempfile)?; + assert_eq!( + filecount_in(dir.path()), + 1, + "only one tempfile exists no matter the iteration" + ); + gix_tempfile::registry::cleanup_tempfiles(); + assert_eq!( + filecount_in(dir.path()), + 0, + "the signal triggers removal but won't terminate the process (anymore)" + ); + assert!( + tempfile.write_all(b"bogus").is_err(), + "cannot write into a tempfile that doesn't exist in registry" + ); + Ok(()) +} diff --git a/gix/Cargo.toml b/gix/Cargo.toml index bdbb57d27be..8b544c3fd1b 100644 --- a/gix/Cargo.toml +++ b/gix/Cargo.toml @@ -92,7 +92,7 @@ cache-efficiency-debug = ["gix-features/cache-efficiency-debug"] [dependencies] gix-ref = { version = "^0.24.1", path = "../gix-ref" } gix-discover = { version = "^0.13.1", path = "../gix-discover" } -gix-tempfile = { version = "^3.0.0", path = "../gix-tempfile" } +gix-tempfile = { version = "^3.0.0", path = "../gix-tempfile", features = ["signals"] } gix-lock = { version = "^3.0.0", path = "../gix-lock" } gix-validate = { version = "^0.7.3", path = "../gix-validate" } gix-sec = { version = "^0.6.2", path = "../gix-sec" } diff --git a/gix/src/interrupt.rs b/gix/src/interrupt.rs index 7e49ab68c5a..c94cbdbfa89 100644 --- a/gix/src/interrupt.rs +++ b/gix/src/interrupt.rs @@ -89,7 +89,7 @@ mod init { } let msg_idx = INTERRUPT_COUNT.fetch_add(1, Ordering::SeqCst); if msg_idx == 1 { - gix_tempfile::handler::cleanup_tempfiles(); + gix_tempfile::registry::cleanup_tempfiles_signal_safe(); signal_hook::low_level::emulate_default_handler(*sig).ok(); } interrupt(); @@ -100,7 +100,7 @@ mod init { } // This means that they won't setup a handler allowing us to call them right before we actually abort. - gix_tempfile::setup(gix_tempfile::SignalHandlerMode::None); + gix_tempfile::signal::setup(gix_tempfile::signal::handler::Mode::None); Ok(Deregister(hooks)) } diff --git a/tests/journey/gix.sh b/tests/journey/gix.sh index d2d924ed280..196ac442b2a 100644 --- a/tests/journey/gix.sh +++ b/tests/journey/gix.sh @@ -11,9 +11,9 @@ title "gix-tempfile crate" (when "running the example program to raise a signal with a tempfile present" it "fails as the process aborts" && { - expect_run $ABORTED cargo run --example delete-tempfiles-on-sigterm + expect_run $ABORTED cargo run --features signals --example delete-tempfiles-on-sigterm } - TEMPFILE="$(cargo run --example delete-tempfiles-on-sigterm 2>/dev/null || true)" + TEMPFILE="$(cargo run --features signals --example delete-tempfiles-on-sigterm 2>/dev/null || true)" it "outputs a tempfile with an expected name" && { expect_run $SUCCESSFULLY test "$TEMPFILE" = "tempfile.ext" } @@ -25,12 +25,12 @@ title "gix-tempfile crate" (when "running the example program to help assure there cannot be deadlocks" ABORTED=134 it "succeeds as it won't deadlock" && { - expect_run $ABORTED cargo run --release --example try-deadlock-on-cleanup -- 5 + expect_run $ABORTED cargo run --release --features signals --example try-deadlock-on-cleanup -- 1 } ) ) -title "gix-tempfile crate" +title "gix crate" (when "testing 'gix'" snapshot="$snapshot/gix" cd gix