Warn when a local process execution sandbox cannot be deleted (#23696)

## Problem

`AsyncDropSandbox::drop` deletes a local sandbox by dropping its
`TempDir`, and `tempfile`'s `Drop` ignores any removal error. A sandbox
that contains a read-only directory is therefore left behind in the
temporary directory with no message. #23636 is one case: an extracted Go
module cache leaked about 700 sandboxes and 2.1 GB per cold run, and
nothing reported it. @tdyas asked there for at least a warning.

## Fix

Close the `TempDir` explicitly on the blocking cleanup task and log a
warning with the sandbox path when that fails. Behavior is otherwise
unchanged: cleanup still runs in the background and a failure does not
fail the run.

## Verification

Two unit tests in `local_tests.rs`: a normal sandbox is deleted, and a
sandbox with a read-only subdirectory returns an error that names its
path. End to end, a cold `check` of a Go module on main (without #23636)
now logs the warning for every leaked download sandbox.

Notice: Claude used for code drafting, tests and verification.

Author: rdeknijf

docs/notes/2.34.x.md

@@ -45,6 +45,8 @@ Fixed `system_binary` / PATH lookup crashing with `IntrinsicError: Operation not
 
 Fixed pantsd logging remote Authentication bearer tokens in plaintext when `remote_store_headers` / `remote_execution_headers` / `remote_oauth_bearer_token` (or the matching `DynamicRemoteOptions` header fields) changed and the scheduler reinitialized. Those sensitive values are now shown as `<redacted>` in the reinitialization diff ([#23685](https://github.com/pantsbuild/pants/issues/23685)).
 
+Pants now logs a warning when it cannot delete a local process execution sandbox. Cleanup failures were previously ignored, so sandboxes containing read-only directories leaked in the temporary directory without any message.
+
 ### Goals
 
 ### Backends

src/rust/process_execution/src/local.rs

@@ -22,7 +22,7 @@ use fs::{
 use futures::stream::{BoxStream, StreamExt, TryStreamExt};
 use futures::{FutureExt, TryFutureExt, try_join};
 use hashing::Digest;
-use log::{debug, info};
+use log::{debug, info, warn};
 use nails::execution::ExitCode;
 use sandboxer::Sandboxer;
 use serde::Serialize;
@@ -826,11 +826,27 @@ impl AsyncDropSandbox {
 impl Drop for AsyncDropSandbox {
     fn drop(&mut self) {
         if let Some(sandbox) = self.2.take() {
-            let _background_cleanup = self.0.spawn_blocking(|| std::mem::drop(sandbox));
+            let _background_cleanup = self.0.spawn_blocking(move || {
+                if let Err(e) = remove_sandbox(sandbox) {
+                    warn!("{e}");
+                }
+            });
         }
     }
 }
 
+/// Delete a sandbox directory, reporting failure instead of silently leaking it (which is what
+/// dropping the `TempDir` does).
+pub(crate) fn remove_sandbox(sandbox: TempDir) -> Result<(), String> {
+    let path = sandbox.path().to_owned();
+    sandbox.close().map_err(|e| {
+        format!(
+            "Failed to delete local process execution dir {}: {e}",
+            path.display()
+        )
+    })
+}
+
 /// Create a file called __run.sh with the env, cwd and argv used by Pants to facilitate debugging.
 pub fn setup_run_sh_script(
     sandbox_path: &Path,

src/rust/process_execution/src/local_tests.rs

@@ -804,3 +804,34 @@ async fn run_command_locally_in_dir(
 fn one_second() -> Option<Duration> {
     Some(Duration::from_millis(1000))
 }
+
+#[test]
+fn remove_sandbox_deletes_dir() {
+    let sandbox = TempDir::new().unwrap();
+    let path = sandbox.path().to_owned();
+    std::fs::write(path.join("file"), "content").unwrap();
+
+    local::remove_sandbox(sandbox).unwrap();
+    assert!(!path.exists());
+}
+
+#[test]
+#[cfg(unix)]
+fn remove_sandbox_reports_undeletable_dir() {
+    use std::os::unix::fs::PermissionsExt;
+
+    // A read-only directory, like the extracted Go module cache, cannot have its entries removed.
+    let sandbox = TempDir::new().unwrap();
+    let path = sandbox.path().to_owned();
+    let read_only = path.join("read_only");
+    std::fs::create_dir(&read_only).unwrap();
+    std::fs::write(read_only.join("file"), "content").unwrap();
+    std::fs::set_permissions(&read_only, std::fs::Permissions::from_mode(0o555)).unwrap();
+
+    let err = local::remove_sandbox(sandbox).unwrap_err();
+    assert!(err.contains(&path.display().to_string()), "{err}");
+    assert!(path.exists());
+
+    std::fs::set_permissions(&read_only, std::fs::Permissions::from_mode(0o755)).unwrap();
+    std::fs::remove_dir_all(&path).unwrap();
+}