diff --git a/Cargo.lock b/Cargo.lock index 4ed835b..601b5e3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1819,7 +1819,7 @@ dependencies = [ [[package]] name = "pict-rs" -version = "0.5.9" +version = "0.5.10" dependencies = [ "actix-form-data", "actix-web", diff --git a/Cargo.toml b/Cargo.toml index 4f7152c..61869ce 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "pict-rs" description = "A simple image hosting service" -version = "0.5.9" +version = "0.5.10" authors = ["asonix "] license = "AGPL-3.0" readme = "README.md" diff --git a/defaults.toml b/defaults.toml index 4b6802b..60abfa3 100644 --- a/defaults.toml +++ b/defaults.toml @@ -4,6 +4,7 @@ read_only = false danger_dummy_mode = false max_file_count = 1 temporary_directory = "/tmp" +cleanup_temporary_directory = true [client] timeout = 30 @@ -46,7 +47,7 @@ proxy = "7d" [media.magick] max_width = 10000 max_height = 10000 -max_area = 40000000 +max_area = 20000 memory = 256 map = 512 disk = 1024 diff --git a/pict-rs.nix b/pict-rs.nix index 4c4ebbc..4fb1bfc 100644 --- a/pict-rs.nix +++ b/pict-rs.nix @@ -11,7 +11,7 @@ rustPlatform.buildRustPackage { pname = "pict-rs"; - version = "0.5.9"; + version = "0.5.10"; src = ./.; cargoLock = { diff --git a/pict-rs.toml b/pict-rs.toml index e27990f..586c022 100644 --- a/pict-rs.toml +++ b/pict-rs.toml @@ -37,6 +37,11 @@ max_file_count = 1 # default: The system's advertised temporary directory ("/tmp" on most linuxes) temporary_directory = "/tmp" +## Optional: whether to delete the contents of $temporary_directory/pict-rs on launch +# environment variable: PICTRS__SERVER__CLEANUP_TEMPORARY_DIRECTORY +# default: true +cleanup_temporary_directory = true + ## Optional: path to server certificate to enable TLS # environment variable: PICTRS__SERVER__CERTIFICATE # default: empty diff --git a/releases/0.5.10.md b/releases/0.5.10.md new file mode 100644 index 0000000..413a797 --- /dev/null +++ b/releases/0.5.10.md @@ -0,0 +1,31 @@ +# pict-rs 0.5.10 + +## Overview + +pict-rs 0.5.10 is a small release with changes to how pict-rs handles temporary files. + +### Changes + +- [Temporary File Cleanup](#temporary-file-cleanup) + + +## Upgrade Notes + +There are no significant changes from 0.5.9. Upgrading should be as simple as pulling the new +version. + + +## Descriptions + +### Temporary File Cleanup + +pict-rs now nests its temporary files inside a `pict-rs` toplevel temporary folder. This is useful +because pict-rs 0.5.10 introduces a new behavior: it will completely delete that folder and its +contents on launch. If you are running multiple copies of pict-rs on the same host and they share +your temporary folder, this might cause problems. In that scenario, this behavior can be disabled by +setting `PICTRS__SERVER__CLEANUP_TEMPORARY_DIRECTORY=false` or passing +`--no-cleanup-temporary-directory` on the commandline. + +This new behavior has been introduced in order to better clean up after crashes. If pict-rs is +killed while processing media, maybe due to an OOM, it will leave files behind in the temporary +directory. This can cause the temporary directory to grow, leading to memory or disk problems. diff --git a/src/config/commandline.rs b/src/config/commandline.rs index 035ecbd..d029709 100644 --- a/src/config/commandline.rs +++ b/src/config/commandline.rs @@ -55,6 +55,7 @@ impl Args { address, api_key, temporary_directory, + no_cleanup_temporary_directory, certificate, private_key, client_timeout, @@ -122,6 +123,7 @@ impl Args { danger_dummy_mode, max_file_count, temporary_directory, + cleanup_temporary_directory: !no_cleanup_temporary_directory, certificate, private_key, }; @@ -541,6 +543,7 @@ struct Server { max_file_count: Option, #[serde(skip_serializing_if = "Option::is_none")] temporary_directory: Option, + cleanup_temporary_directory: bool, #[serde(skip_serializing_if = "Option::is_none")] certificate: Option, #[serde(skip_serializing_if = "Option::is_none")] @@ -973,6 +976,10 @@ struct Run { #[arg(long)] temporary_directory: Option, + /// Whether to attempt to clean files left behind from a previous run of pict-rs + #[arg(long)] + no_cleanup_temporary_directory: bool, + /// The path to the TLS certificate. Both the certificate and the private_key must be specified /// to enable TLS #[arg(long)] diff --git a/src/config/defaults.rs b/src/config/defaults.rs index aec1d98..c23a77d 100644 --- a/src/config/defaults.rs +++ b/src/config/defaults.rs @@ -24,6 +24,7 @@ struct ServerDefaults { danger_dummy_mode: bool, max_file_count: u32, temporary_directory: PathBuf, + cleanup_temporary_directory: bool, } #[derive(Clone, Debug, serde::Serialize)] @@ -211,6 +212,7 @@ impl Default for ServerDefaults { danger_dummy_mode: false, max_file_count: 1, temporary_directory: std::env::temp_dir(), + cleanup_temporary_directory: true, } } } diff --git a/src/config/file.rs b/src/config/file.rs index 43b53bd..508878a 100644 --- a/src/config/file.rs +++ b/src/config/file.rs @@ -119,6 +119,8 @@ pub(crate) struct Server { pub(crate) temporary_directory: PathBuf, + pub(crate) cleanup_temporary_directory: bool, + #[serde(skip_serializing_if = "Option::is_none")] pub(crate) certificate: Option, diff --git a/src/lib.rs b/src/lib.rs index 8120625..89da947 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2017,7 +2017,11 @@ impl PictRsConfiguration { // describe all the metrics pict-rs produces init_metrics::init_metrics(); - let tmp_dir = TmpDir::init(&config.server.temporary_directory).await?; + let tmp_dir = TmpDir::init( + &config.server.temporary_directory, + config.server.cleanup_temporary_directory, + ) + .await?; let policy_dir = magick::write_magick_policy(&config.media, &tmp_dir).await?; let client = build_client()?; diff --git a/src/tmp_file.rs b/src/tmp_file.rs index 46e9c14..aacca9a 100644 --- a/src/tmp_file.rs +++ b/src/tmp_file.rs @@ -16,9 +16,17 @@ pub(crate) struct TmpDir { } impl TmpDir { - pub(crate) async fn init>(path: P) -> std::io::Result> { - let path = path.as_ref().join(Uuid::now_v7().to_string()); - tokio::fs::create_dir(&path).await?; + pub(crate) async fn init>(path: P, cleanup: bool) -> std::io::Result> { + let base_path = path.as_ref().join("pict-rs"); + + if cleanup && tokio::fs::metadata(&base_path).await.is_ok() { + tokio::fs::remove_dir_all(&base_path).await?; + } + + let path = base_path.join(Uuid::now_v7().to_string()); + + tokio::fs::create_dir_all(&path).await?; + Ok(Arc::new(TmpDir { path: Some(path) })) } @@ -47,8 +55,13 @@ impl TmpDir { } pub(crate) async fn cleanup(self: Arc) -> std::io::Result<()> { - if let Some(path) = Arc::into_inner(self).and_then(|mut this| this.path.take()) { - tokio::fs::remove_dir_all(path).await?; + if let Some(mut path) = Arc::into_inner(self).and_then(|mut this| this.path.take()) { + tokio::fs::remove_dir_all(&path).await?; + + if path.pop() { + // attempt to remove parent directory if it is empty + let _ = tokio::fs::remove_dir(path).await; + } } Ok(()) @@ -57,9 +70,13 @@ impl TmpDir { impl Drop for TmpDir { fn drop(&mut self) { - if let Some(path) = self.path.take() { + if let Some(mut path) = self.path.take() { tracing::warn!("TmpDir - Blocking remove of {path:?}"); - std::fs::remove_dir_all(path).expect("Removed directory"); + std::fs::remove_dir_all(&path).expect("Removed directory"); + if path.pop() { + // attempt to remove parent directory if it is empty + let _ = std::fs::remove_dir(path); + } } } }