From 5bca9fa1719a85eb797bc240103bc063b2b24cc2 Mon Sep 17 00:00:00 2001 From: Vim Wickramasinghe Date: Tue, 27 Jan 2026 15:48:26 +0100 Subject: [PATCH 1/5] [TOW-1394][WIP] Attempt cache cleanup --- crates/tower-runtime/src/subprocess.rs | 45 ++- crates/tower-runtime/tests/subprocess_test.rs | 301 ++++++++++++++++++ crates/tower-uv/src/lib.rs | 35 +- 3 files changed, 367 insertions(+), 14 deletions(-) create mode 100644 crates/tower-runtime/tests/subprocess_test.rs diff --git a/crates/tower-runtime/src/subprocess.rs b/crates/tower-runtime/src/subprocess.rs index 713722f2..7bafaedb 100644 --- a/crates/tower-runtime/src/subprocess.rs +++ b/crates/tower-runtime/src/subprocess.rs @@ -78,6 +78,18 @@ impl ExecutionBackend for SubprocessBackend { _ => self.cache_dir.clone(), }; + // Create a unique temp directory for uv if no cache directory is configured + // This prevents uv from leaving lock files scattered in /tmp + let uv_temp_dir = if cache_dir.is_none() { + let temp_path = std::env::temp_dir().join(format!("tower-uv-{}", spec.id)); + tokio::fs::create_dir_all(&temp_path) + .await + .map_err(|_| Error::PackageCreateFailed)?; + Some(temp_path) + } else { + None + }; + // Receive package stream and unpack it let package = self.receive_and_unpack_package(spec.package_stream).await?; @@ -86,6 +98,14 @@ impl ExecutionBackend for SubprocessBackend { .clone() .ok_or(Error::PackageUnpackFailed)?; + // Set custom TMPDIR for uv to use + let mut env_vars = spec.env_vars; + if let Some(ref temp_dir) = uv_temp_dir { + env_vars.insert("TMPDIR".to_string(), temp_dir.to_string_lossy().to_string()); + env_vars.insert("TEMP".to_string(), temp_dir.to_string_lossy().to_string()); + env_vars.insert("TMP".to_string(), temp_dir.to_string_lossy().to_string()); + } + let opts = StartOptions { ctx: spec.telemetry_ctx, package: Package::from_unpacked_path(unpacked_path).await?, @@ -93,7 +113,7 @@ impl ExecutionBackend for SubprocessBackend { environment: spec.environment, secrets: spec.secrets, parameters: spec.parameters, - env_vars: spec.env_vars, + env_vars, output_sender: output_sender.clone(), cache_dir, }; @@ -106,6 +126,7 @@ impl ExecutionBackend for SubprocessBackend { app: Arc::new(Mutex::new(app)), output_receiver: Arc::new(Mutex::new(output_receiver)), _package: package, // Keep package alive so temp dir doesn't get cleaned up + uv_temp_dir, }) } @@ -133,7 +154,18 @@ pub struct SubprocessHandle { id: String, app: Arc>, output_receiver: Arc>, - _package: Package, // Keep package alive to prevent temp dir cleanup + _package: Package, // Keep package alive to prevent temp dir cleanup + uv_temp_dir: Option, // Track uv's temp directory for cleanup +} + +impl Drop for SubprocessHandle { + fn drop(&mut self) { + // Best-effort cleanup of temp directory when handle is dropped + if let Some(temp_dir) = self.uv_temp_dir.take() { + // Spawn blocking task to remove directory + let _ = std::fs::remove_dir_all(&temp_dir); + } + } } #[async_trait] @@ -195,6 +227,15 @@ impl ExecutionHandle for SubprocessHandle { async fn cleanup(&mut self) -> Result<(), Error> { // Ensure the app is terminated self.terminate().await?; + + // Clean up uv's temp directory if it was created + if let Some(ref temp_dir) = self.uv_temp_dir { + if let Err(e) = tokio::fs::remove_dir_all(temp_dir).await { + // Log but don't fail - cleanup is best-effort + tower_telemetry::debug!("Failed to clean up uv temp directory: {:?}", e); + } + } + Ok(()) } } diff --git a/crates/tower-runtime/tests/subprocess_test.rs b/crates/tower-runtime/tests/subprocess_test.rs new file mode 100644 index 00000000..9bb6854e --- /dev/null +++ b/crates/tower-runtime/tests/subprocess_test.rs @@ -0,0 +1,301 @@ +use std::collections::HashMap; +use std::path::PathBuf; + +use tower_runtime::execution::{ + CacheBackend, CacheConfig, CacheIsolation, ExecutionBackend, ExecutionHandle, ExecutionSpec, + NetworkingSpec, ResourceLimits, RuntimeConfig, +}; +use tower_runtime::subprocess::SubprocessBackend; +use tower_runtime::Status; + +use config::Towerfile; +use tower_package::{Package, PackageSpec}; +use tower_telemetry::{self, debug}; + +fn get_example_app_dir(name: &str) -> PathBuf { + let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + path.push("tests"); + path.push("example-apps"); + path.push(name); + + if !path.exists() { + panic!("Example app directory does not exist: {}", path.display()); + } + + path +} + +async fn build_package_from_dir(dir: &PathBuf) -> Package { + let towerfile = Towerfile::from_path(dir.join("Towerfile")).expect("Failed to load Towerfile"); + let spec = PackageSpec::from_towerfile(&towerfile); + let mut package = Package::build(spec) + .await + .expect("Failed to build package from directory"); + package.unpack().await.expect("Failed to unpack package"); + package +} + +/// Count uv lock files in /tmp directory +fn count_uv_lock_files() -> usize { + let tmp_dir = std::env::temp_dir(); + let mut count = 0; + + if let Ok(entries) = std::fs::read_dir(&tmp_dir) { + for entry in entries.flatten() { + if let Ok(file_name) = entry.file_name().into_string() { + if file_name.starts_with("uv-") && file_name.ends_with(".lock") { + count += 1; + } + } + } + } + + count +} + +/// Create a package stream from a built package +async fn create_package_stream(package: Package) -> Box { + // Get the package tar.gz path + let tar_gz_path = package + .package_file_path + .expect("Package should have tar.gz file"); + + let file = tokio::fs::File::open(&tar_gz_path) + .await + .expect("Failed to open package file"); + + Box::new(file) +} + +#[tokio::test] +async fn test_temp_file_accumulation_happy_path() { + tower_telemetry::enable_logging( + tower_telemetry::LogLevel::Debug, + tower_telemetry::LogFormat::Plain, + tower_telemetry::LogDestination::Stdout, + ); + + debug!("Testing temp file accumulation - happy path"); + + // Count uv lock files before execution + let before_count = count_uv_lock_files(); + debug!("UV lock files before execution: {}", before_count); + + // Build a simple app + let hello_world_dir = get_example_app_dir("01-hello-world"); + let package = build_package_from_dir(&hello_world_dir).await; + + // Create subprocess backend WITHOUT cache directory (this triggers the problem) + let backend = SubprocessBackend::new(None); + + // Create execution spec + let spec = ExecutionSpec { + id: "test-exec-happy".to_string(), + telemetry_ctx: tower_telemetry::Context::new(), + package_stream: create_package_stream(package).await, + environment: "test".to_string(), + secrets: HashMap::new(), + parameters: HashMap::new(), + env_vars: HashMap::new(), + runtime: RuntimeConfig { + image: "python:3.11".to_string(), + version: None, + cache: CacheConfig { + enable_bundle_cache: false, + enable_runtime_cache: false, + enable_dependency_cache: false, + backend: CacheBackend::None, + isolation: CacheIsolation::None, + }, + entrypoint: None, + command: None, + }, + resources: ResourceLimits { + cpu_millicores: None, + memory_mb: None, + storage_mb: None, + max_pids: None, + gpu_count: 0, + timeout_seconds: 300, + }, + networking: None, + }; + + // Create and run execution + let mut handle = backend + .create(spec) + .await + .expect("Failed to create execution"); + + // Wait for completion + let status = handle + .wait_for_completion() + .await + .expect("Failed to wait for completion"); + debug!("Execution completed with status: {:?}", status); + + assert_eq!(status, Status::Exited, "App should exit successfully"); + + // Count uv lock files after execution (in system /tmp) + let after_count = count_uv_lock_files(); + debug!("UV lock files in /tmp after execution: {}", after_count); + + // With the fix: temp files should NOT accumulate in /tmp because they go in isolated directory + assert_eq!( + after_count, before_count, + "UV temp files should NOT accumulate in /tmp with the fix. Before: {}, After: {}", + before_count, after_count + ); + + // Verify the isolated temp directory was created + let temp_dir_path = std::env::temp_dir().join("tower-uv-test-exec-happy"); + debug!("Checking for isolated temp directory: {:?}", temp_dir_path); + assert!( + temp_dir_path.exists(), + "Isolated temp directory should exist during execution" + ); + + // Clean up the execution + handle.cleanup().await.expect("Failed to cleanup"); + + // Count again after cleanup + let after_cleanup_count = count_uv_lock_files(); + debug!( + "UV lock files in /tmp after cleanup: {}", + after_cleanup_count + ); + + // With the fix: cleanup should not increase files in /tmp + assert_eq!( + after_cleanup_count, before_count, + "UV temp files in /tmp should remain unchanged after cleanup. Before: {}, After cleanup: {}", + before_count, after_cleanup_count + ); + + // Verify the isolated temp directory was cleaned up + assert!( + !temp_dir_path.exists(), + "Isolated temp directory should be cleaned up after execution" + ); +} + +#[tokio::test] +async fn test_temp_file_accumulation_termination() { + tower_telemetry::enable_logging( + tower_telemetry::LogLevel::Debug, + tower_telemetry::LogFormat::Plain, + tower_telemetry::LogDestination::Stdout, + ); + + debug!("Testing temp file accumulation - termination scenario"); + + // Count uv lock files before execution + let before_count = count_uv_lock_files(); + debug!("UV lock files before execution: {}", before_count); + + // Build an app with dependencies that takes longer to start + let use_faker_dir = get_example_app_dir("02-use-faker"); + let package = build_package_from_dir(&use_faker_dir).await; + + // Create subprocess backend WITHOUT cache directory + let backend = SubprocessBackend::new(None); + + // Create execution spec + let spec = ExecutionSpec { + id: "test-exec-terminate".to_string(), + telemetry_ctx: tower_telemetry::Context::new(), + package_stream: create_package_stream(package).await, + environment: "test".to_string(), + secrets: HashMap::new(), + parameters: HashMap::new(), + env_vars: HashMap::new(), + runtime: RuntimeConfig { + image: "python:3.11".to_string(), + version: None, + cache: CacheConfig { + enable_bundle_cache: false, + enable_runtime_cache: false, + enable_dependency_cache: false, + backend: CacheBackend::None, + isolation: CacheIsolation::None, + }, + entrypoint: None, + command: None, + }, + resources: ResourceLimits { + cpu_millicores: None, + memory_mb: None, + storage_mb: None, + max_pids: None, + gpu_count: 0, + timeout_seconds: 300, + }, + networking: None, + }; + + // Create execution + let mut handle = backend + .create(spec) + .await + .expect("Failed to create execution"); + + // Let it start (uv venv + sync will create temp files) + tokio::time::sleep(tokio::time::Duration::from_secs(2)).await; + + // Check that temp files were NOT created in /tmp (with the fix) + let during_count = count_uv_lock_files(); + debug!("UV lock files in /tmp during execution: {}", during_count); + + // With the fix: no files should accumulate in system /tmp + assert_eq!( + during_count, before_count, + "UV temp files should NOT accumulate in /tmp during execution. Before: {}, During: {}", + before_count, during_count + ); + + // Verify the isolated temp directory exists + let temp_dir_path = std::env::temp_dir().join("tower-uv-test-exec-terminate"); + debug!("Checking for isolated temp directory: {:?}", temp_dir_path); + assert!( + temp_dir_path.exists(), + "Isolated temp directory should exist during execution" + ); + + // Terminate the execution + handle.terminate().await.expect("Failed to terminate"); + debug!("Execution terminated"); + + // Count uv lock files after termination + let after_count = count_uv_lock_files(); + debug!("UV lock files in /tmp after termination: {}", after_count); + + // With the fix: no files should have been added to /tmp + assert_eq!( + after_count, before_count, + "UV temp files in /tmp should remain unchanged after termination. Before: {}, After: {}", + before_count, after_count + ); + + // Clean up + handle.cleanup().await.expect("Failed to cleanup"); + + // Count again after cleanup + let after_cleanup_count = count_uv_lock_files(); + debug!( + "UV lock files in /tmp after cleanup: {}", + after_cleanup_count + ); + + // With the fix: still no files in /tmp + assert_eq!( + after_cleanup_count, before_count, + "UV temp files in /tmp should remain unchanged after cleanup. Before: {}, After cleanup: {}", + before_count, after_cleanup_count + ); + + // Verify the isolated temp directory was cleaned up + assert!( + !temp_dir_path.exists(), + "Isolated temp directory should be cleaned up after explicit cleanup" + ); +} diff --git a/crates/tower-uv/src/lib.rs b/crates/tower-uv/src/lib.rs index 33daf16b..91077319 100644 --- a/crates/tower-uv/src/lib.rs +++ b/crates/tower-uv/src/lib.rs @@ -61,14 +61,18 @@ fn normalize_env_vars(env_vars: &HashMap) -> HashMap) -> HashMap Date: Wed, 28 Jan 2026 15:35:36 +0100 Subject: [PATCH 2/5] In case of no cache_dir provided create temporary directories for uv and clean them --- crates/tower-runtime/src/subprocess.rs | 11 +- crates/tower-runtime/tests/subprocess_test.rs | 263 ++++++++++++++++++ 2 files changed, 272 insertions(+), 2 deletions(-) diff --git a/crates/tower-runtime/src/subprocess.rs b/crates/tower-runtime/src/subprocess.rs index 7bafaedb..be4b651e 100644 --- a/crates/tower-runtime/src/subprocess.rs +++ b/crates/tower-runtime/src/subprocess.rs @@ -79,7 +79,7 @@ impl ExecutionBackend for SubprocessBackend { }; // Create a unique temp directory for uv if no cache directory is configured - // This prevents uv from leaving lock files scattered in /tmp + // This gives UV an isolated cache that we can clean up completely after execution let uv_temp_dir = if cache_dir.is_none() { let temp_path = std::env::temp_dir().join(format!("tower-uv-{}", spec.id)); tokio::fs::create_dir_all(&temp_path) @@ -98,9 +98,16 @@ impl ExecutionBackend for SubprocessBackend { .clone() .ok_or(Error::PackageUnpackFailed)?; - // Set custom TMPDIR for uv to use + // Set UV_CACHE_DIR to control where UV stores its cache and lock files + // This ensures all UV artifacts go into our managed temp directory let mut env_vars = spec.env_vars; if let Some(ref temp_dir) = uv_temp_dir { + // UV_CACHE_DIR is the primary control for UV's cache location + env_vars.insert( + "UV_CACHE_DIR".to_string(), + temp_dir.to_string_lossy().to_string(), + ); + // Also set TMPDIR for any other temporary files env_vars.insert("TMPDIR".to_string(), temp_dir.to_string_lossy().to_string()); env_vars.insert("TEMP".to_string(), temp_dir.to_string_lossy().to_string()); env_vars.insert("TMP".to_string(), temp_dir.to_string_lossy().to_string()); diff --git a/crates/tower-runtime/tests/subprocess_test.rs b/crates/tower-runtime/tests/subprocess_test.rs index 9bb6854e..0ed0da6d 100644 --- a/crates/tower-runtime/tests/subprocess_test.rs +++ b/crates/tower-runtime/tests/subprocess_test.rs @@ -53,6 +53,60 @@ fn count_uv_lock_files() -> usize { count } +/// Find UV cache directories in /tmp and default cache location +/// UV creates cache directories when TMPDIR is set and no --cache-dir is provided +fn find_uv_cache_dirs() -> Vec { + let mut cache_dirs = Vec::new(); + + // Check /tmp for temporary cache directories + let tmp_dir = std::env::temp_dir(); + if let Ok(entries) = std::fs::read_dir(&tmp_dir) { + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + // Check if it looks like a UV cache by inspecting contents + if path.join("wheels").exists() + || path.join("built-wheels").exists() + || path.join("simple-v12").exists() + || path.join("archive-v0").exists() + { + cache_dirs.push(path); + } + } + } + } + + // Also check default UV cache location + if let Ok(home) = std::env::var("HOME") { + let default_uv_cache = PathBuf::from(home).join(".cache").join("uv"); + if default_uv_cache.exists() { + cache_dirs.push(default_uv_cache); + } + } + + cache_dirs +} + +/// Calculate total size of directories in bytes +fn calculate_dir_size(paths: &[PathBuf]) -> u64 { + let mut total = 0u64; + for path in paths { + if let Ok(metadata) = std::fs::metadata(path) { + if metadata.is_file() { + total += metadata.len(); + } else if metadata.is_dir() { + // Recursively calculate directory size + if let Ok(entries) = std::fs::read_dir(path) { + for entry in entries.flatten() { + total += calculate_dir_size(&[entry.path()]); + } + } + } + } + } + total +} + /// Create a package stream from a built package async fn create_package_stream(package: Package) -> Box { // Get the package tar.gz path @@ -299,3 +353,212 @@ async fn test_temp_file_accumulation_termination() { "Isolated temp directory should be cleaned up after explicit cleanup" ); } + +#[tokio::test] +async fn test_uv_cache_directory_accumulation() { + tower_telemetry::enable_logging( + tower_telemetry::LogLevel::Debug, + tower_telemetry::LogFormat::Plain, + tower_telemetry::LogDestination::Stdout, + ); + + debug!("Testing UV cache directory accumulation"); + + // Find existing UV cache directories and calculate size + let before_dirs = find_uv_cache_dirs(); + let before_size = calculate_dir_size(&before_dirs); + debug!( + "Before execution: {} UV cache dirs, total size: {} bytes ({} MB)", + before_dirs.len(), + before_size, + before_size / 1024 / 1024 + ); + for dir in &before_dirs { + debug!(" - {:?}", dir); + } + + // Run multiple executions with dependencies to trigger cache creation + for i in 0..3 { + debug!("Execution {} starting", i); + + let use_faker_dir = get_example_app_dir("02-use-faker"); + let package = build_package_from_dir(&use_faker_dir).await; + + let backend = SubprocessBackend::new(None); + + let spec = ExecutionSpec { + id: format!("test-cache-{}", i), + telemetry_ctx: tower_telemetry::Context::new(), + package_stream: create_package_stream(package).await, + environment: "test".to_string(), + secrets: HashMap::new(), + parameters: HashMap::new(), + env_vars: HashMap::new(), + runtime: RuntimeConfig { + image: "python:3.11".to_string(), + version: None, + cache: CacheConfig { + enable_bundle_cache: false, + enable_runtime_cache: false, + enable_dependency_cache: false, + backend: CacheBackend::None, + isolation: CacheIsolation::None, + }, + entrypoint: None, + command: None, + }, + resources: ResourceLimits { + cpu_millicores: None, + memory_mb: None, + storage_mb: None, + max_pids: None, + gpu_count: 0, + timeout_seconds: 300, + }, + networking: None, + }; + + let mut handle = backend + .create(spec) + .await + .expect("Failed to create execution"); + + // Give UV time to start and create cache + tokio::time::sleep(tokio::time::Duration::from_millis(500)).await; + + // Check isolated cache directory DURING execution + let isolated_cache_path = std::env::temp_dir().join(format!("tower-uv-test-cache-{}", i)); + if isolated_cache_path.exists() { + debug!( + "Execution {} - Isolated cache directory exists during execution", + i + ); + + // List what's inside + if let Ok(entries) = std::fs::read_dir(&isolated_cache_path) { + for entry in entries.flatten().take(10) { + debug!(" {:?}", entry.file_name()); + } + } + + // Check for UV cache structure + let has_cache_artifacts = isolated_cache_path.join("wheels").exists() + || isolated_cache_path.join("built-wheels").exists() + || isolated_cache_path.join("simple-v12").exists() + || isolated_cache_path.join("archive-v0").exists(); + + if has_cache_artifacts { + debug!(" ✓ Contains UV cache artifacts"); + } else { + debug!(" ⚠ No typical UV cache structure found"); + } + } else { + debug!( + " ⚠ Isolated cache directory does not exist during execution {}", + i + ); + } + + // Wait for completion + handle + .wait_for_completion() + .await + .expect("Failed to wait for completion"); + + // Verify cache size before cleanup + if isolated_cache_path.exists() { + let cache_size = calculate_dir_size(&[isolated_cache_path.clone()]); + debug!( + "Execution {} - Cache size before cleanup: {} MB", + i, + cache_size / 1024 / 1024 + ); + } + + // Clean up + handle.cleanup().await.expect("Failed to cleanup"); + + // Verify cleanup removed the isolated cache + if isolated_cache_path.exists() { + debug!(" ❌ WARNING: Isolated cache still exists after cleanup!"); + } else { + debug!(" ✓ Isolated cache cleaned up successfully"); + } + + debug!("Execution {} completed and cleaned up", i); + } + + // Check for cache directories after executions + let after_dirs = find_uv_cache_dirs(); + let after_size = calculate_dir_size(&after_dirs); + debug!( + "After {} executions: {} UV cache dirs, total size: {} bytes ({} MB)", + 3, + after_dirs.len(), + after_size, + after_size / 1024 / 1024 + ); + for dir in &after_dirs { + debug!(" - {:?}", dir); + } + + // The real problem: cache directories accumulating + let size_increase = after_size.saturating_sub(before_size); + debug!( + "Size increase: {} bytes ({} MB)", + size_increase, + size_increase / 1024 / 1024 + ); + + // With UV_CACHE_DIR set to isolated temp directories: + // 1. Cache should NOT accumulate in ~/.cache/uv + // 2. Cache should NOT accumulate in /tmp + // 3. Each execution's cache is cleaned up after completion + + // Check if default UV cache grew (it shouldn't with our fix) + if let Ok(home) = std::env::var("HOME") { + let default_uv_cache = PathBuf::from(home).join(".cache").join("uv"); + let before_default = before_dirs.iter().find(|p| **p == default_uv_cache); + let after_default = after_dirs.iter().find(|p| **p == default_uv_cache); + + match (before_default, after_default) { + (Some(_), Some(_)) => { + // Cache existed before and after - verify it didn't grow + debug!( + "Default UV cache (~/.cache/uv) exists - should not have grown significantly" + ); + assert!( + size_increase < 10_000_000, // Less than 10MB growth + "Default UV cache should not grow when UV_CACHE_DIR is set. Growth: {} MB", + size_increase / 1024 / 1024 + ); + } + (None, Some(_)) => { + // Cache was created - this is wrong, UV should use our isolated dir + panic!("UV created cache in ~/.cache/uv despite UV_CACHE_DIR being set!"); + } + _ => { + debug!("Default UV cache doesn't exist - good, UV is using isolated directories"); + } + } + } + + // Count cache directories in /tmp (there should be none after cleanup) + let tmp_cache_dirs: Vec<_> = after_dirs + .iter() + .filter(|p| p.starts_with(&std::env::temp_dir())) + .collect(); + + debug!( + "Cache directories remaining in /tmp: {} (should be 0)", + tmp_cache_dirs.len() + ); + + // With our fix, all isolated temp dirs should be cleaned up + assert_eq!( + tmp_cache_dirs.len(), + 0, + "No UV cache directories should remain in /tmp after cleanup. Found: {:?}", + tmp_cache_dirs + ); +} From a01f013c76532d28684cc568e2fbf16f3cd0a1db Mon Sep 17 00:00:00 2001 From: Vim Wickramasinghe Date: Wed, 28 Jan 2026 15:58:01 +0100 Subject: [PATCH 3/5] Correctly set cache and clean --- crates/tower-runtime/src/subprocess.rs | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/crates/tower-runtime/src/subprocess.rs b/crates/tower-runtime/src/subprocess.rs index be4b651e..7bf77876 100644 --- a/crates/tower-runtime/src/subprocess.rs +++ b/crates/tower-runtime/src/subprocess.rs @@ -79,15 +79,16 @@ impl ExecutionBackend for SubprocessBackend { }; // Create a unique temp directory for uv if no cache directory is configured - // This gives UV an isolated cache that we can clean up completely after execution - let uv_temp_dir = if cache_dir.is_none() { + let (final_cache_dir, uv_temp_dir) = if cache_dir.is_none() { let temp_path = std::env::temp_dir().join(format!("tower-uv-{}", spec.id)); tokio::fs::create_dir_all(&temp_path) .await .map_err(|_| Error::PackageCreateFailed)?; - Some(temp_path) + // Use the temp directory as cache_dir and track it for cleanup + (Some(temp_path.clone()), Some(temp_path)) } else { - None + // Use provided cache_dir, no temp dir to clean up + (cache_dir, None) }; // Receive package stream and unpack it @@ -98,16 +99,9 @@ impl ExecutionBackend for SubprocessBackend { .clone() .ok_or(Error::PackageUnpackFailed)?; - // Set UV_CACHE_DIR to control where UV stores its cache and lock files - // This ensures all UV artifacts go into our managed temp directory + // Set TMPDIR to the same isolated directory to ensure lock files also go there let mut env_vars = spec.env_vars; if let Some(ref temp_dir) = uv_temp_dir { - // UV_CACHE_DIR is the primary control for UV's cache location - env_vars.insert( - "UV_CACHE_DIR".to_string(), - temp_dir.to_string_lossy().to_string(), - ); - // Also set TMPDIR for any other temporary files env_vars.insert("TMPDIR".to_string(), temp_dir.to_string_lossy().to_string()); env_vars.insert("TEMP".to_string(), temp_dir.to_string_lossy().to_string()); env_vars.insert("TMP".to_string(), temp_dir.to_string_lossy().to_string()); @@ -122,7 +116,7 @@ impl ExecutionBackend for SubprocessBackend { parameters: spec.parameters, env_vars, output_sender: output_sender.clone(), - cache_dir, + cache_dir: final_cache_dir, // UV will use this via --cache-dir flag }; // Start the LocalApp From 6248f10f619fff52f47cbdd53fd36127d13cc790 Mon Sep 17 00:00:00 2001 From: Vim Wickramasinghe Date: Thu, 29 Jan 2026 11:59:14 +0100 Subject: [PATCH 4/5] Track & explicitly remove tmp pacakge dir --- crates/tower-runtime/src/subprocess.rs | 30 ++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/crates/tower-runtime/src/subprocess.rs b/crates/tower-runtime/src/subprocess.rs index 7bf77876..5dc6e347 100644 --- a/crates/tower-runtime/src/subprocess.rs +++ b/crates/tower-runtime/src/subprocess.rs @@ -11,6 +11,7 @@ use crate::{App, OutputReceiver, StartOptions, Status}; use async_trait::async_trait; use std::path::PathBuf; use std::sync::Arc; +use tmpdir::TmpDir; use tokio::fs::File; use tokio::io::AsyncWriteExt; use tokio::sync::Mutex; @@ -92,13 +93,17 @@ impl ExecutionBackend for SubprocessBackend { }; // Receive package stream and unpack it - let package = self.receive_and_unpack_package(spec.package_stream).await?; + let mut package = self.receive_and_unpack_package(spec.package_stream).await?; let unpacked_path = package .unpacked_path .clone() .ok_or(Error::PackageUnpackFailed)?; + // Extract tmp_dir from package for cleanup tracking + // We need to keep this alive until execution completes + let package_tmp_dir = package.tmp_dir.take(); + // Set TMPDIR to the same isolated directory to ensure lock files also go there let mut env_vars = spec.env_vars; if let Some(ref temp_dir) = uv_temp_dir { @@ -126,7 +131,7 @@ impl ExecutionBackend for SubprocessBackend { id: spec.id, app: Arc::new(Mutex::new(app)), output_receiver: Arc::new(Mutex::new(output_receiver)), - _package: package, // Keep package alive so temp dir doesn't get cleaned up + package_tmp_dir, uv_temp_dir, }) } @@ -155,17 +160,21 @@ pub struct SubprocessHandle { id: String, app: Arc>, output_receiver: Arc>, - _package: Package, // Keep package alive to prevent temp dir cleanup - uv_temp_dir: Option, // Track uv's temp directory for cleanup + package_tmp_dir: Option, // Track package temp directory for cleanup + uv_temp_dir: Option, // Track UV's temp directory for cleanup } impl Drop for SubprocessHandle { fn drop(&mut self) { - // Best-effort cleanup of temp directory when handle is dropped + // Best-effort cleanup of UV temp directory when handle is dropped if let Some(temp_dir) = self.uv_temp_dir.take() { - // Spawn blocking task to remove directory let _ = std::fs::remove_dir_all(&temp_dir); } + + // Best-effort cleanup of package temp directory when handle is dropped + if let Some(tmp_dir) = self.package_tmp_dir.take() { + let _ = std::fs::remove_dir_all(tmp_dir.to_path_buf()); + } } } @@ -237,6 +246,15 @@ impl ExecutionHandle for SubprocessHandle { } } + // Clean up package temp directory + if let Some(tmp_dir) = self.package_tmp_dir.take() { + let path = tmp_dir.to_path_buf(); + if let Err(e) = tokio::fs::remove_dir_all(&path).await { + // Log but don't fail - cleanup is best-effort + tower_telemetry::debug!("Failed to clean up package temp directory: {:?}", e); + } + } + Ok(()) } } From 8a70950006f131b8eca1608aa6d56f0c9566d6bd Mon Sep 17 00:00:00 2001 From: Vim Wickramasinghe Date: Thu, 29 Jan 2026 12:19:55 +0100 Subject: [PATCH 5/5] Test cleanup --- crates/tower-runtime/src/subprocess.rs | 2 +- crates/tower-runtime/tests/subprocess_test.rs | 505 ++---------------- 2 files changed, 58 insertions(+), 449 deletions(-) diff --git a/crates/tower-runtime/src/subprocess.rs b/crates/tower-runtime/src/subprocess.rs index 5dc6e347..de3147ce 100644 --- a/crates/tower-runtime/src/subprocess.rs +++ b/crates/tower-runtime/src/subprocess.rs @@ -38,7 +38,7 @@ impl SubprocessBackend { mut package_stream: Box, ) -> Result { // Create temp directory for this package - let temp_dir = tmpdir::TmpDir::new("tower-package") + let temp_dir = TmpDir::new("tower-package") .await .map_err(|_| Error::PackageCreateFailed)?; diff --git a/crates/tower-runtime/tests/subprocess_test.rs b/crates/tower-runtime/tests/subprocess_test.rs index 0ed0da6d..40a99929 100644 --- a/crates/tower-runtime/tests/subprocess_test.rs +++ b/crates/tower-runtime/tests/subprocess_test.rs @@ -3,25 +3,24 @@ use std::path::PathBuf; use tower_runtime::execution::{ CacheBackend, CacheConfig, CacheIsolation, ExecutionBackend, ExecutionHandle, ExecutionSpec, - NetworkingSpec, ResourceLimits, RuntimeConfig, + ResourceLimits, RuntimeConfig, }; use tower_runtime::subprocess::SubprocessBackend; use tower_runtime::Status; use config::Towerfile; use tower_package::{Package, PackageSpec}; -use tower_telemetry::{self, debug}; fn get_example_app_dir(name: &str) -> PathBuf { let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); path.push("tests"); path.push("example-apps"); path.push(name); - - if !path.exists() { - panic!("Example app directory does not exist: {}", path.display()); - } - + assert!( + path.exists(), + "Example app directory does not exist: {}", + path.display() + ); path } @@ -35,81 +34,7 @@ async fn build_package_from_dir(dir: &PathBuf) -> Package { package } -/// Count uv lock files in /tmp directory -fn count_uv_lock_files() -> usize { - let tmp_dir = std::env::temp_dir(); - let mut count = 0; - - if let Ok(entries) = std::fs::read_dir(&tmp_dir) { - for entry in entries.flatten() { - if let Ok(file_name) = entry.file_name().into_string() { - if file_name.starts_with("uv-") && file_name.ends_with(".lock") { - count += 1; - } - } - } - } - - count -} - -/// Find UV cache directories in /tmp and default cache location -/// UV creates cache directories when TMPDIR is set and no --cache-dir is provided -fn find_uv_cache_dirs() -> Vec { - let mut cache_dirs = Vec::new(); - - // Check /tmp for temporary cache directories - let tmp_dir = std::env::temp_dir(); - if let Ok(entries) = std::fs::read_dir(&tmp_dir) { - for entry in entries.flatten() { - let path = entry.path(); - if path.is_dir() { - // Check if it looks like a UV cache by inspecting contents - if path.join("wheels").exists() - || path.join("built-wheels").exists() - || path.join("simple-v12").exists() - || path.join("archive-v0").exists() - { - cache_dirs.push(path); - } - } - } - } - - // Also check default UV cache location - if let Ok(home) = std::env::var("HOME") { - let default_uv_cache = PathBuf::from(home).join(".cache").join("uv"); - if default_uv_cache.exists() { - cache_dirs.push(default_uv_cache); - } - } - - cache_dirs -} - -/// Calculate total size of directories in bytes -fn calculate_dir_size(paths: &[PathBuf]) -> u64 { - let mut total = 0u64; - for path in paths { - if let Ok(metadata) = std::fs::metadata(path) { - if metadata.is_file() { - total += metadata.len(); - } else if metadata.is_dir() { - // Recursively calculate directory size - if let Ok(entries) = std::fs::read_dir(path) { - for entry in entries.flatten() { - total += calculate_dir_size(&[entry.path()]); - } - } - } - } - } - total -} - -/// Create a package stream from a built package -async fn create_package_stream(package: Package) -> Box { - // Get the package tar.gz path +async fn create_execution_spec(id: String, package: Package) -> ExecutionSpec { let tar_gz_path = package .package_file_path .expect("Package should have tar.gz file"); @@ -118,35 +43,10 @@ async fn create_package_stream(package: Package) -> Box bool { + let tmp_dir = std::env::temp_dir(); + let uv_cache_dir = tmp_dir.join(format!("tower-uv-{}", execution_id)); + uv_cache_dir.exists() +} + +#[tokio::test] +async fn test_no_temp_file_accumulation_happy_path() { + let execution_id = "test-happy-cleanup"; + + // Execute app with dependencies + let app_dir = get_example_app_dir("02-use-faker"); + let package = build_package_from_dir(&app_dir).await; + let backend = SubprocessBackend::new(None); + let spec = create_execution_spec(execution_id.to_string(), package).await; - // Create and run execution let mut handle = backend .create(spec) .await .expect("Failed to create execution"); - - // Wait for completion let status = handle .wait_for_completion() .await .expect("Failed to wait for completion"); - debug!("Execution completed with status: {:?}", status); assert_eq!(status, Status::Exited, "App should exit successfully"); - // Count uv lock files after execution (in system /tmp) - let after_count = count_uv_lock_files(); - debug!("UV lock files in /tmp after execution: {}", after_count); - - // With the fix: temp files should NOT accumulate in /tmp because they go in isolated directory - assert_eq!( - after_count, before_count, - "UV temp files should NOT accumulate in /tmp with the fix. Before: {}, After: {}", - before_count, after_count - ); - - // Verify the isolated temp directory was created - let temp_dir_path = std::env::temp_dir().join("tower-uv-test-exec-happy"); - debug!("Checking for isolated temp directory: {:?}", temp_dir_path); - assert!( - temp_dir_path.exists(), - "Isolated temp directory should exist during execution" - ); - - // Clean up the execution + // Cleanup handle.cleanup().await.expect("Failed to cleanup"); - // Count again after cleanup - let after_cleanup_count = count_uv_lock_files(); - debug!( - "UV lock files in /tmp after cleanup: {}", - after_cleanup_count - ); - - // With the fix: cleanup should not increase files in /tmp - assert_eq!( - after_cleanup_count, before_count, - "UV temp files in /tmp should remain unchanged after cleanup. Before: {}, After cleanup: {}", - before_count, after_cleanup_count - ); - - // Verify the isolated temp directory was cleaned up + // Verify this execution's temp directory was cleaned up assert!( - !temp_dir_path.exists(), - "Isolated temp directory should be cleaned up after execution" + !uv_temp_dir_exists(execution_id), + "UV temp directory should be cleaned up after execution" ); } #[tokio::test] -async fn test_temp_file_accumulation_termination() { - tower_telemetry::enable_logging( - tower_telemetry::LogLevel::Debug, - tower_telemetry::LogFormat::Plain, - tower_telemetry::LogDestination::Stdout, - ); - - debug!("Testing temp file accumulation - termination scenario"); +async fn test_no_temp_file_accumulation_on_termination() { + let execution_id = "test-terminate-cleanup"; - // Count uv lock files before execution - let before_count = count_uv_lock_files(); - debug!("UV lock files before execution: {}", before_count); - - // Build an app with dependencies that takes longer to start - let use_faker_dir = get_example_app_dir("02-use-faker"); - let package = build_package_from_dir(&use_faker_dir).await; - - // Create subprocess backend WITHOUT cache directory + // Execute app with dependencies + let app_dir = get_example_app_dir("02-use-faker"); + let package = build_package_from_dir(&app_dir).await; let backend = SubprocessBackend::new(None); + let spec = create_execution_spec(execution_id.to_string(), package).await; - // Create execution spec - let spec = ExecutionSpec { - id: "test-exec-terminate".to_string(), - telemetry_ctx: tower_telemetry::Context::new(), - package_stream: create_package_stream(package).await, - environment: "test".to_string(), - secrets: HashMap::new(), - parameters: HashMap::new(), - env_vars: HashMap::new(), - runtime: RuntimeConfig { - image: "python:3.11".to_string(), - version: None, - cache: CacheConfig { - enable_bundle_cache: false, - enable_runtime_cache: false, - enable_dependency_cache: false, - backend: CacheBackend::None, - isolation: CacheIsolation::None, - }, - entrypoint: None, - command: None, - }, - resources: ResourceLimits { - cpu_millicores: None, - memory_mb: None, - storage_mb: None, - max_pids: None, - gpu_count: 0, - timeout_seconds: 300, - }, - networking: None, - }; - - // Create execution let mut handle = backend .create(spec) .await .expect("Failed to create execution"); - // Let it start (uv venv + sync will create temp files) - tokio::time::sleep(tokio::time::Duration::from_secs(2)).await; - - // Check that temp files were NOT created in /tmp (with the fix) - let during_count = count_uv_lock_files(); - debug!("UV lock files in /tmp during execution: {}", during_count); - - // With the fix: no files should accumulate in system /tmp - assert_eq!( - during_count, before_count, - "UV temp files should NOT accumulate in /tmp during execution. Before: {}, During: {}", - before_count, during_count - ); - - // Verify the isolated temp directory exists - let temp_dir_path = std::env::temp_dir().join("tower-uv-test-exec-terminate"); - debug!("Checking for isolated temp directory: {:?}", temp_dir_path); - assert!( - temp_dir_path.exists(), - "Isolated temp directory should exist during execution" - ); - - // Terminate the execution + // Let it start, then terminate + tokio::time::sleep(tokio::time::Duration::from_secs(1)).await; handle.terminate().await.expect("Failed to terminate"); - debug!("Execution terminated"); - // Count uv lock files after termination - let after_count = count_uv_lock_files(); - debug!("UV lock files in /tmp after termination: {}", after_count); - - // With the fix: no files should have been added to /tmp - assert_eq!( - after_count, before_count, - "UV temp files in /tmp should remain unchanged after termination. Before: {}, After: {}", - before_count, after_count - ); - - // Clean up + // Cleanup handle.cleanup().await.expect("Failed to cleanup"); - // Count again after cleanup - let after_cleanup_count = count_uv_lock_files(); - debug!( - "UV lock files in /tmp after cleanup: {}", - after_cleanup_count - ); - - // With the fix: still no files in /tmp - assert_eq!( - after_cleanup_count, before_count, - "UV temp files in /tmp should remain unchanged after cleanup. Before: {}, After cleanup: {}", - before_count, after_cleanup_count - ); - - // Verify the isolated temp directory was cleaned up + // Verify this execution's temp directory was cleaned up assert!( - !temp_dir_path.exists(), - "Isolated temp directory should be cleaned up after explicit cleanup" + !uv_temp_dir_exists(execution_id), + "UV temp directory should be cleaned up after termination" ); } #[tokio::test] -async fn test_uv_cache_directory_accumulation() { - tower_telemetry::enable_logging( - tower_telemetry::LogLevel::Debug, - tower_telemetry::LogFormat::Plain, - tower_telemetry::LogDestination::Stdout, - ); - - debug!("Testing UV cache directory accumulation"); - - // Find existing UV cache directories and calculate size - let before_dirs = find_uv_cache_dirs(); - let before_size = calculate_dir_size(&before_dirs); - debug!( - "Before execution: {} UV cache dirs, total size: {} bytes ({} MB)", - before_dirs.len(), - before_size, - before_size / 1024 / 1024 - ); - for dir in &before_dirs { - debug!(" - {:?}", dir); - } - - // Run multiple executions with dependencies to trigger cache creation +async fn test_multiple_executions_no_accumulation() { + // Run multiple executions for i in 0..3 { - debug!("Execution {} starting", i); - - let use_faker_dir = get_example_app_dir("02-use-faker"); - let package = build_package_from_dir(&use_faker_dir).await; - + let execution_id = format!("test-multi-cleanup-{}", i); + let app_dir = get_example_app_dir("01-hello-world"); + let package = build_package_from_dir(&app_dir).await; let backend = SubprocessBackend::new(None); - - let spec = ExecutionSpec { - id: format!("test-cache-{}", i), - telemetry_ctx: tower_telemetry::Context::new(), - package_stream: create_package_stream(package).await, - environment: "test".to_string(), - secrets: HashMap::new(), - parameters: HashMap::new(), - env_vars: HashMap::new(), - runtime: RuntimeConfig { - image: "python:3.11".to_string(), - version: None, - cache: CacheConfig { - enable_bundle_cache: false, - enable_runtime_cache: false, - enable_dependency_cache: false, - backend: CacheBackend::None, - isolation: CacheIsolation::None, - }, - entrypoint: None, - command: None, - }, - resources: ResourceLimits { - cpu_millicores: None, - memory_mb: None, - storage_mb: None, - max_pids: None, - gpu_count: 0, - timeout_seconds: 300, - }, - networking: None, - }; + let spec = create_execution_spec(execution_id.clone(), package).await; let mut handle = backend .create(spec) .await .expect("Failed to create execution"); - - // Give UV time to start and create cache - tokio::time::sleep(tokio::time::Duration::from_millis(500)).await; - - // Check isolated cache directory DURING execution - let isolated_cache_path = std::env::temp_dir().join(format!("tower-uv-test-cache-{}", i)); - if isolated_cache_path.exists() { - debug!( - "Execution {} - Isolated cache directory exists during execution", - i - ); - - // List what's inside - if let Ok(entries) = std::fs::read_dir(&isolated_cache_path) { - for entry in entries.flatten().take(10) { - debug!(" {:?}", entry.file_name()); - } - } - - // Check for UV cache structure - let has_cache_artifacts = isolated_cache_path.join("wheels").exists() - || isolated_cache_path.join("built-wheels").exists() - || isolated_cache_path.join("simple-v12").exists() - || isolated_cache_path.join("archive-v0").exists(); - - if has_cache_artifacts { - debug!(" ✓ Contains UV cache artifacts"); - } else { - debug!(" ⚠ No typical UV cache structure found"); - } - } else { - debug!( - " ⚠ Isolated cache directory does not exist during execution {}", - i - ); - } - - // Wait for completion handle .wait_for_completion() .await .expect("Failed to wait for completion"); - - // Verify cache size before cleanup - if isolated_cache_path.exists() { - let cache_size = calculate_dir_size(&[isolated_cache_path.clone()]); - debug!( - "Execution {} - Cache size before cleanup: {} MB", - i, - cache_size / 1024 / 1024 - ); - } - - // Clean up handle.cleanup().await.expect("Failed to cleanup"); - // Verify cleanup removed the isolated cache - if isolated_cache_path.exists() { - debug!(" ❌ WARNING: Isolated cache still exists after cleanup!"); - } else { - debug!(" ✓ Isolated cache cleaned up successfully"); - } - - debug!("Execution {} completed and cleaned up", i); - } - - // Check for cache directories after executions - let after_dirs = find_uv_cache_dirs(); - let after_size = calculate_dir_size(&after_dirs); - debug!( - "After {} executions: {} UV cache dirs, total size: {} bytes ({} MB)", - 3, - after_dirs.len(), - after_size, - after_size / 1024 / 1024 - ); - for dir in &after_dirs { - debug!(" - {:?}", dir); - } - - // The real problem: cache directories accumulating - let size_increase = after_size.saturating_sub(before_size); - debug!( - "Size increase: {} bytes ({} MB)", - size_increase, - size_increase / 1024 / 1024 - ); - - // With UV_CACHE_DIR set to isolated temp directories: - // 1. Cache should NOT accumulate in ~/.cache/uv - // 2. Cache should NOT accumulate in /tmp - // 3. Each execution's cache is cleaned up after completion - - // Check if default UV cache grew (it shouldn't with our fix) - if let Ok(home) = std::env::var("HOME") { - let default_uv_cache = PathBuf::from(home).join(".cache").join("uv"); - let before_default = before_dirs.iter().find(|p| **p == default_uv_cache); - let after_default = after_dirs.iter().find(|p| **p == default_uv_cache); - - match (before_default, after_default) { - (Some(_), Some(_)) => { - // Cache existed before and after - verify it didn't grow - debug!( - "Default UV cache (~/.cache/uv) exists - should not have grown significantly" - ); - assert!( - size_increase < 10_000_000, // Less than 10MB growth - "Default UV cache should not grow when UV_CACHE_DIR is set. Growth: {} MB", - size_increase / 1024 / 1024 - ); - } - (None, Some(_)) => { - // Cache was created - this is wrong, UV should use our isolated dir - panic!("UV created cache in ~/.cache/uv despite UV_CACHE_DIR being set!"); - } - _ => { - debug!("Default UV cache doesn't exist - good, UV is using isolated directories"); - } - } + // Verify each execution's temp directory is cleaned up + assert!( + !uv_temp_dir_exists(&execution_id), + "UV temp directory {} should be cleaned up", + execution_id + ); } - - // Count cache directories in /tmp (there should be none after cleanup) - let tmp_cache_dirs: Vec<_> = after_dirs - .iter() - .filter(|p| p.starts_with(&std::env::temp_dir())) - .collect(); - - debug!( - "Cache directories remaining in /tmp: {} (should be 0)", - tmp_cache_dirs.len() - ); - - // With our fix, all isolated temp dirs should be cleaned up - assert_eq!( - tmp_cache_dirs.len(), - 0, - "No UV cache directories should remain in /tmp after cleanup. Found: {:?}", - tmp_cache_dirs - ); }