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
1 change: 1 addition & 0 deletions docs/notes/2.34.x.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Fixed an issue where `pants --changed-since` would unnecessarily invalidate all

The `indicatif-spinner` `dynamic_ui_renderer` (default), now renders on a dedicated thread. This should reduce jitter and display a smoother spinner under heavy load.

Fixed a bug where `pantsd` would crash on macOS (during Docker builds) because it retained cached paths to temporary directories that had been deleted by the OS.

### Goals

Expand Down
18 changes: 17 additions & 1 deletion src/rust/fs/store/src/immutable_inputs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,23 @@ impl ImmutableInputs {
directory_digest: DirectoryDigest,
) -> Result<PathBuf, StoreError> {
let digest = directory_digest.as_digest();
let cell = self.0.contents.lock().entry(digest).or_default().clone();
// we must verify the memoized path still exists on disk.
// If it was deleted, we invalidate the cache and force re-materialization.
let cell = {
let mut contents = self.0.contents.lock();
let cell = contents.entry(digest).or_default().clone();
if let Some(path) = cell.get() {
if path.exists() {
cell
} else {
let new_cell = Arc::new(OnceCell::new());
contents.insert(digest, new_cell.clone());
new_cell
}
} else {
cell
}
};

// We (might) need to initialize the value.
//
Expand Down
38 changes: 37 additions & 1 deletion src/rust/fs/store/src/local_tests.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
// Copyright 2022 Pants project contributors (see CONTRIBUTORS.md).
// Licensed under the Apache License, Version 2.0 (see LICENSE).
use crate::local::ByteStore;
use crate::{EntryType, LocalOptions, ShrinkBehavior};
use crate::{EntryType, ImmutableInputs, LocalOptions, ShrinkBehavior, Store};

use std::collections::HashSet;
use std::io::Write;
use std::path::Path;
use std::time::Duration;

use bytes::{BufMut, Bytes, BytesMut};
use fs::DirectoryDigest;
use hashing::{Digest, Fingerprint};
use tempfile::{NamedTempFile, TempDir};
use testutil::data::{TestData, TestDirectory};
Expand Down Expand Up @@ -807,3 +808,38 @@ fn get_directory_size(path: &Path) -> usize {
}
len
}

#[tokio::test]
async fn recovers_from_deleted_directory_issue_23411() {
let executor = task_executor::Executor::new();
let store_dir = TempDir::new().unwrap();
let store = Store::local_only(executor, store_dir.path()).unwrap();

let base_dir = TempDir::new().unwrap();
let immutable_inputs = ImmutableInputs::new(store.clone(), base_dir.path()).unwrap();

let digest = DirectoryDigest::from_persisted_digest(TestDirectory::empty().digest());

let path1 = immutable_inputs.path_for_dir(digest.clone()).await.unwrap();
assert!(
path1.exists(),
"The directory should have been created the first time"
);

std::fs::remove_dir_all(&path1).unwrap();
assert!(
!path1.exists(),
"The directory was physically deleted by the 'reaper'"
);

let path2 = immutable_inputs.path_for_dir(digest).await.unwrap();

assert!(
path2.exists(),
"The code must have detected the absence and recreated the directory on the disk"
);
assert_ne!(
path1, path2,
"The returned path will be different because a new random chroot has been generated"
);
}
Loading