From 1e5bf3461b5f8fc70710a0de9d58e5943a2b2c97 Mon Sep 17 00:00:00 2001 From: naoNao89 <90588855+naoNao89@users.noreply.github.com> Date: Mon, 15 Dec 2025 21:51:11 +0700 Subject: [PATCH 1/2] 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 --- src/uu/mv/src/mv.rs | 23 ++++++++++++------ tests/by-util/test_mv.rs | 50 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 723875f615f..9caa2bc0b62 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -1082,15 +1082,24 @@ fn copy_dir_contents_recursive( display_manager, )?; } else { - // Copy file with or without hardlink support based on platform + // Check if this is a FIFO to avoid blocking on fs::copy (issue #9656) #[cfg(unix)] { - copy_file_with_hardlinks_helper( - &from_path, - &to_path, - hardlink_tracker, - hardlink_scanner, - )?; + let metadata = from_path.symlink_metadata()?; + let file_type = metadata.file_type(); + + if is_fifo(file_type) { + // Handle FIFO specially to avoid blocking on fs::copy + rename_fifo_fallback(&from_path, &to_path)?; + } else { + // Copy file with hardlink support + copy_file_with_hardlinks_helper( + &from_path, + &to_path, + hardlink_tracker, + hardlink_scanner, + )?; + } } #[cfg(not(unix))] { diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index f28fc8c28a6..1b493ee53f2 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -2537,6 +2537,56 @@ fn test_special_file_different_filesystem() { std::fs::remove_dir_all("/dev/shm/tmp").unwrap(); } +/// Test moving a directory containing a FIFO file across different filesystems (issue #9656) +/// Without proper FIFO handling, this test will hang indefinitely when +/// copy_dir_contents_recursive tries to fs::copy() the FIFO +#[cfg(unix)] +#[test] +fn test_mv_dir_containing_fifo_cross_filesystem() { + use std::time::Duration; + + // Skip if /dev/shm not available + if !Path::new("/dev/shm").exists() { + return; + } + + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("a"); + at.mkfifo("a/f"); + + // This will hang without the fix, so use timeout + // Move to /dev/shm which is typically a different filesystem (tmpfs) + ucmd.args(&["a", "/dev/shm/test_mv_fifo_dir"]) + .timeout(Duration::from_secs(2)) + .succeeds(); + + assert!(!at.dir_exists("a")); + assert!(Path::new("/dev/shm/test_mv_fifo_dir").exists()); + assert!(Path::new("/dev/shm/test_mv_fifo_dir/f").exists()); + std::fs::remove_dir_all("/dev/shm/test_mv_fifo_dir").ok(); +} + +/// Test moving a directory containing a FIFO file (same filesystem - issue #9656) +/// This tests FIFO handling doesn't fail even on same filesystem +#[cfg(unix)] +#[test] +fn test_mv_dir_containing_fifo_same_filesystem() { + use std::time::Duration; + + let (at, mut ucmd) = at_and_ucmd!(); + at.mkdir("a"); + at.mkfifo("a/f"); + + // This will hang without the fix, so use timeout + ucmd.args(&["a", "b"]) + .timeout(Duration::from_secs(2)) + .succeeds(); + + assert!(!at.dir_exists("a")); + assert!(at.dir_exists("b")); + assert!(at.is_fifo("b/f")); +} + /// Test cross-device move with permission denied error /// This test mimics the scenario from the GNU part-fail test where /// a cross-device move fails due to permission errors when removing the target file From 68a35de0c7dd78d061cad98c38c1a1b0659a083f Mon Sep 17 00:00:00 2001 From: naoNao89 <90588855+naoNao89@users.noreply.github.com> Date: Sun, 21 Dec 2025 05:59:40 +0700 Subject: [PATCH 2/2] test: add cross-platform mount infrastructure and refactor mv FIFO test - Add macOS mount_temp_fs() implementation using hdiutil ramdisk - Refactor test_mv_dir_containing_fifo to use TestScenario infrastructure - Replace platform-specific /dev/shm dependency with portable mount_temp_fs() - Consolidate separate cross-fs and same-fs tests into single test - Add root privilege requirement check - Implement automatic cleanup via Drop trait on all platforms This completes cross-platform test infrastructure: - Linux/FreeBSD/Android: tmpfs mounting (already existed) - macOS: ramdisk creation + HFS+ format + mount (new) Both implementations provide mount_temp_fs()/umount_temp_fs() for cross-device testing without relying on system-specific paths. --- .../cspell.dictionaries/jargon.wordlist.txt | 3 + tests/by-util/test_mv.rs | 55 ++++++-------- tests/uutests/src/lib/util.rs | 76 +++++++++++++++++++ 3 files changed, 104 insertions(+), 30 deletions(-) diff --git a/.vscode/cspell.dictionaries/jargon.wordlist.txt b/.vscode/cspell.dictionaries/jargon.wordlist.txt index d2febb7724f..a027ce14bdc 100644 --- a/.vscode/cspell.dictionaries/jargon.wordlist.txt +++ b/.vscode/cspell.dictionaries/jargon.wordlist.txt @@ -90,8 +90,10 @@ microbenchmarks microbenchmarking multibyte multicall +newfs nmerge noatime +nomount nocache nocreat noctty @@ -171,6 +173,7 @@ inacc maint proc procs +ramdisk # * constants xffff diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 1b493ee53f2..3dba0df6b5e 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -2545,46 +2545,41 @@ fn test_special_file_different_filesystem() { fn test_mv_dir_containing_fifo_cross_filesystem() { use std::time::Duration; - // Skip if /dev/shm not available - if !Path::new("/dev/shm").exists() { + let mut scene = TestScenario::new(util_name!()); + + // Test must be run as root (or with `sudo -E`) + if scene.cmd("whoami").run().stdout_str() != "root\n" { return; } - let (at, mut ucmd) = at_and_ucmd!(); - at.mkdir("a"); - at.mkfifo("a/f"); - - // This will hang without the fix, so use timeout - // Move to /dev/shm which is typically a different filesystem (tmpfs) - ucmd.args(&["a", "/dev/shm/test_mv_fifo_dir"]) - .timeout(Duration::from_secs(2)) - .succeeds(); - - assert!(!at.dir_exists("a")); - assert!(Path::new("/dev/shm/test_mv_fifo_dir").exists()); - assert!(Path::new("/dev/shm/test_mv_fifo_dir/f").exists()); - std::fs::remove_dir_all("/dev/shm/test_mv_fifo_dir").ok(); -} - -/// Test moving a directory containing a FIFO file (same filesystem - issue #9656) -/// This tests FIFO handling doesn't fail even on same filesystem -#[cfg(unix)] -#[test] -fn test_mv_dir_containing_fifo_same_filesystem() { - use std::time::Duration; + { + let at = &scene.fixtures; + at.mkdir("a"); + at.mkfifo("a/f"); + at.mkdir("mnt"); + } - let (at, mut ucmd) = at_and_ucmd!(); - at.mkdir("a"); - at.mkfifo("a/f"); + // Prepare the mount + let mountpoint_path = scene.fixtures.plus_as_string("mnt"); + scene + .mount_temp_fs(&mountpoint_path) + .expect("mounting tmpfs failed"); // This will hang without the fix, so use timeout - ucmd.args(&["a", "b"]) + // Move to the mounted tmpfs which is a different filesystem + scene + .ucmd() + .args(&["a", "mnt/dest"]) .timeout(Duration::from_secs(2)) .succeeds(); + // Ditch the mount before the asserts + scene.umount_temp_fs(); + + let at = &scene.fixtures; assert!(!at.dir_exists("a")); - assert!(at.dir_exists("b")); - assert!(at.is_fifo("b/f")); + assert!(at.dir_exists("mnt/dest")); + assert!(at.is_fifo("mnt/dest/f")); } /// Test cross-device move with permission denied error diff --git a/tests/uutests/src/lib/util.rs b/tests/uutests/src/lib/util.rs index 108a2b056f4..4216c534876 100644 --- a/tests/uutests/src/lib/util.rs +++ b/tests/uutests/src/lib/util.rs @@ -1339,6 +1339,8 @@ pub struct TestScenario { tmpd: Rc, #[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))] tmp_fs_mountpoint: Option, + #[cfg(target_vendor = "apple")] + tmp_fs_ramdisk: Option, } impl TestScenario { @@ -1355,6 +1357,8 @@ impl TestScenario { tmpd, #[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))] tmp_fs_mountpoint: None, + #[cfg(target_vendor = "apple")] + tmp_fs_ramdisk: None, }; let mut fixture_path_builder = env::current_dir().unwrap(); fixture_path_builder.push(TESTS_DIR); @@ -1422,6 +1426,64 @@ impl TestScenario { Ok(()) } + /// Mounts a temporary filesystem at the specified mount point (macOS). + #[cfg(target_vendor = "apple")] + pub fn mount_temp_fs(&mut self, mount_point: &str) -> core::result::Result<(), String> { + if self.tmp_fs_ramdisk.is_some() { + return Err("already mounted".to_string()); + } + + // Create a 10MB ramdisk using hdiutil (10 * 2048 = 20480 512-byte sectors) + let attach_result = self + .cmd("hdiutil") + .args(&["attach", "-nomount", "ram://20480"]) + .run(); + + if !attach_result.succeeded() { + return Err("Failed to create ramdisk".to_string()); + } + + let ramdisk_device = attach_result.stdout_str().trim().to_string(); + if ramdisk_device.is_empty() { + return Err("hdiutil returned empty device name".to_string()); + } + + // Format the ramdisk with HFS+ filesystem + let format_result = self + .cmd("newfs_hfs") + .arg("-M") + .arg("700") + .arg(&ramdisk_device) + .run(); + + if !format_result.succeeded() { + // Clean up ramdisk on failure + let _ = self.cmd("hdiutil").args(&["detach", &ramdisk_device]).run(); + return Err(format!( + "Failed to format ramdisk: {}", + format_result.stderr_str() + )); + } + + // Mount the ramdisk at the specified mount point + let mount_result = self + .cmd("mount") + .args(&["-t", "hfs", &ramdisk_device, mount_point]) + .run(); + + if !mount_result.succeeded() { + // Clean up ramdisk on failure + let _ = self.cmd("hdiutil").args(&["detach", &ramdisk_device]).run(); + return Err(format!( + "Failed to mount ramdisk: {}", + mount_result.stderr_str() + )); + } + + self.tmp_fs_ramdisk = Some(ramdisk_device); + Ok(()) + } + #[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))] /// Unmounts the temporary filesystem if it is currently mounted. pub fn umount_temp_fs(&mut self) { @@ -1430,12 +1492,26 @@ impl TestScenario { self.tmp_fs_mountpoint = None; } } + + #[cfg(target_vendor = "apple")] + /// Unmounts and detaches the temporary ramdisk (macOS). + pub fn umount_temp_fs(&mut self) { + if let Some(ramdisk_device) = self.tmp_fs_ramdisk.as_ref() { + // hdiutil detach will unmount automatically + self.cmd("hdiutil") + .args(&["detach", ramdisk_device]) + .succeeds(); + self.tmp_fs_ramdisk = None; + } + } } impl Drop for TestScenario { fn drop(&mut self) { #[cfg(any(target_os = "linux", target_os = "android", target_os = "freebsd"))] self.umount_temp_fs(); + #[cfg(target_vendor = "apple")] + self.umount_temp_fs(); } }