fix(clipboard): unix, don't let the next file's preload fail a request (#16364)

After serving a request for file N, `serve_file_contents` preloads the
next non-directory file with `load_handle()?`. When that file can no
longer be opened (deleted or renamed after the copy, or unreadable), its
error replaced N's reply, so every size and range request for N failed
although N itself is intact.

Keep the preload best-effort. Its failure is logged at trace, because it
can fire on every request for N, and N+1's own request still reports the
error.

Signed-off-by: chlee <sourcehatchery@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Cheolhee LeeandClaude Opus 5.5 authored and GitHub committed 2026-09-28 11:46:34 +08:00
1 parent 8a76628007
commit 89fcf56c4e
1 file changed
+95 -1
+95 -1
View File
@@ -324,7 +324,10 @@ impl ClipFiles {
// hot reload next file
for next_file in self.file_list.iter_mut().skip(file_idx + 1) {
if !next_file.is_dir {
next_file.load_handle()?;
// Best-effort: if it still cannot be opened, its own request reports that.
if let Err(e) = next_file.load_handle() {
log::trace!("failed to preload the next file: {:?}", e);
}
break;
}
}
@@ -918,4 +921,95 @@ mod sig_test {
clear_files();
}
fn list_files(selection: &[&PathBuf]) -> ClipFiles {
let files: Vec<String> = selection.iter().map(|p| path_str(p)).collect();
let mut clip = ClipFiles::default();
clip.sync_files(&files, fingerprint(&files)).unwrap();
clip
}
fn read_head(clip: &mut ClipFiles, file_idx: usize) -> Result<Vec<u8>, CliprdrError> {
let request = FileContentsRequest::Range {
stream_id: 0,
file_idx,
offset: 0,
length: 4,
};
match clip.serve_file_contents(0, request)? {
ClipboardFile::FileContentsResponse { requested_data, .. } => Ok(requested_data),
other => panic!("unexpected response: {:?}", other),
}
}
// Preloading the file after the requested one is best-effort: a file that can no longer
// be opened fails its own requests, not those of the file before it.
fn assert_only_its_own_requests_fail(clip: &mut ClipFiles, broken: &PathBuf) {
let first_idx = clip.first_file_index;
let broken_idx = clip
.file_list
.iter()
.position(|f| &f.path == broken)
.unwrap();
let size = FileContentsRequest::Size {
stream_id: 0,
file_idx: first_idx,
};
clip.serve_file_contents(0, size).unwrap();
assert_eq!(read_head(clip, first_idx).unwrap(), b"AAAA");
assert!(matches!(
read_head(clip, broken_idx),
Err(CliprdrError::FileError { .. })
));
}
#[test]
fn a_deleted_next_file_fails_only_its_own_requests() {
let tmp = TmpDir::new("next_deleted");
let (first, second) = (tmp.join("first.bin"), tmp.join("second.bin"));
fs::write(&first, b"AAAAAAAA").unwrap();
fs::write(&second, b"BBBBBBBB").unwrap();
let mut clip = list_files(&[&first, &second]);
fs::remove_file(&second).unwrap();
assert_only_its_own_requests_fail(&mut clip, &second);
}
#[test]
fn an_unreadable_next_file_fails_only_its_own_requests() {
use std::os::unix::fs::PermissionsExt;
let tmp = TmpDir::new("next_unreadable");
let (first, second) = (tmp.join("first.bin"), tmp.join("second.bin"));
fs::write(&first, b"AAAAAAAA").unwrap();
fs::write(&second, b"BBBBBBBB").unwrap();
let mut clip = list_files(&[&first, &second]);
fs::set_permissions(&second, fs::Permissions::from_mode(0o000)).unwrap();
// Root opens it regardless of its mode.
if fs::File::open(&second).is_ok() {
return;
}
assert_only_its_own_requests_fail(&mut clip, &second);
}
#[test]
fn the_next_file_is_preloaded_past_a_directory() {
let tmp = TmpDir::new("next_preloaded");
let first = tmp.join("first.bin");
fs::write(&first, b"AAAAAAAA").unwrap();
let dir = tmp.join("dir");
fs::create_dir(&dir).unwrap();
let inner = dir.join("inner.bin");
fs::write(&inner, b"BBBBBBBB").unwrap();
// Listed as first.bin, dir, dir/inner.bin.
let mut clip = list_files(&[&first, &dir]);
let first_idx = clip.first_file_index;
let inner_idx = clip.file_list.len() - 1;
assert_eq!(clip.file_list[inner_idx].path, inner);
assert_eq!(read_head(&mut clip, first_idx).unwrap(), b"AAAA");
assert!(clip.file_list[inner_idx].handle.is_some());
// The last file has no next file to preload.
assert_eq!(read_head(&mut clip, inner_idx).unwrap(), b"BBBB");
}
}