Skip to content

Commit f7ef980

Browse files
committed
fix(mv): prevent hang when moving directories containing FIFO files
Fixes #9656 When moving a directory containing FIFO files, copy_dir_contents_recursive() would call fs::copy() on the FIFO, causing an indefinite hang. This fix adds FIFO detection before the copy operation and uses make_fifo() instead, matching the behavior of rename_fifo_fallback(). - Add FIFO detection in copy_dir_contents_recursive() - Create new FIFO at destination using make_fifo() - Add test with timeout protection to prevent future regressions
1 parent 06d843f commit f7ef980

File tree

2 files changed

+66
-7
lines changed

2 files changed

+66
-7
lines changed

src/uu/mv/src/mv.rs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,15 +1082,28 @@ fn copy_dir_contents_recursive(
10821082
display_manager,
10831083
)?;
10841084
} else {
1085-
// Copy file with or without hardlink support based on platform
1085+
// Check if this is a FIFO to avoid blocking on fs::copy (issue #9656)
10861086
#[cfg(unix)]
10871087
{
1088-
copy_file_with_hardlinks_helper(
1089-
&from_path,
1090-
&to_path,
1091-
hardlink_tracker,
1092-
hardlink_scanner,
1093-
)?;
1088+
let metadata = from_path.symlink_metadata()?;
1089+
let file_type = metadata.file_type();
1090+
1091+
if is_fifo(file_type) {
1092+
// Handle FIFO specially to avoid blocking on fs::copy
1093+
if to_path.try_exists()? {
1094+
fs::remove_file(&to_path)?;
1095+
}
1096+
make_fifo(&to_path)?;
1097+
fs::remove_file(&from_path)?;
1098+
} else {
1099+
// Copy file with hardlink support
1100+
copy_file_with_hardlinks_helper(
1101+
&from_path,
1102+
&to_path,
1103+
hardlink_tracker,
1104+
hardlink_scanner,
1105+
)?;
1106+
}
10941107
}
10951108
#[cfg(not(unix))]
10961109
{

tests/by-util/test_mv.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2537,6 +2537,52 @@ fn test_special_file_different_filesystem() {
25372537
std::fs::remove_dir_all("/dev/shm/tmp").unwrap();
25382538
}
25392539

2540+
/// Test moving a directory containing a FIFO file across different filesystems (issue #9656)
2541+
/// Without proper FIFO handling, this test will hang indefinitely when
2542+
/// copy_dir_contents_recursive tries to fs::copy() the FIFO
2543+
#[cfg(unix)]
2544+
#[ignore = "requires access to a different filesystem"]
2545+
#[test]
2546+
fn test_mv_dir_containing_fifo_cross_filesystem() {
2547+
use std::time::Duration;
2548+
2549+
let (at, mut ucmd) = at_and_ucmd!();
2550+
at.mkdir("a");
2551+
at.mkfifo("a/f");
2552+
2553+
// This will hang without the fix, so use timeout
2554+
// Move to /dev/shm which is typically a different filesystem (tmpfs)
2555+
ucmd.args(&["a", "/dev/shm/test_mv_fifo_dir"])
2556+
.timeout(Duration::from_secs(2))
2557+
.succeeds();
2558+
2559+
assert!(!at.dir_exists("a"));
2560+
assert!(Path::new("/dev/shm/test_mv_fifo_dir").exists());
2561+
assert!(Path::new("/dev/shm/test_mv_fifo_dir/f").exists());
2562+
std::fs::remove_dir_all("/dev/shm/test_mv_fifo_dir").ok();
2563+
}
2564+
2565+
/// Test moving a directory containing a FIFO file (same filesystem - issue #9656)
2566+
/// This tests FIFO handling doesn't fail even on same filesystem
2567+
#[cfg(unix)]
2568+
#[test]
2569+
fn test_mv_dir_containing_fifo_same_filesystem() {
2570+
use std::time::Duration;
2571+
2572+
let (at, mut ucmd) = at_and_ucmd!();
2573+
at.mkdir("a");
2574+
at.mkfifo("a/f");
2575+
2576+
// This will hang without the fix, so use timeout
2577+
ucmd.args(&["a", "b"])
2578+
.timeout(Duration::from_secs(2))
2579+
.succeeds();
2580+
2581+
assert!(!at.dir_exists("a"));
2582+
assert!(at.dir_exists("b"));
2583+
assert!(at.is_fifo("b/f"));
2584+
}
2585+
25402586
/// Test cross-device move with permission denied error
25412587
/// This test mimics the scenario from the GNU part-fail test where
25422588
/// a cross-device move fails due to permission errors when removing the target file

0 commit comments

Comments
 (0)