bug#34069: dired-do-rename on fifos hangs if cross device

bug#34069: dired-do-rename on fifos hangs if cross device

[積丹尼 Dan Jacobson]

$ mkfifo x
then do dired-do-rename x /tmp #not on the same filesystem
This just hangs forever.
(Don't ask me why I am doing this.)
emacs-version "26.1"
$ mv x /tmp #works just fine though.

bug#34069: dired-do-rename on fifos hangs if cross device

[Stefan Kangas]

tags 34069 + confirmed
retitle 34069 rename-file on fifos hangs if cross device
thanks

積丹尼 Dan Jacobson <jidanni@jidanni.org> writes:

> $ mkfifo x
> then do dired-do-rename x /tmp #not on the same filesystem
> This just hangs forever.
> (Don't ask me why I am doing this.)
> emacs-version "26.1"
> $ mv x /tmp #works just fine though.

It actually hangs on the rename-file call in copy-file.

Here is a minimal reproducer:

(progn (shell-command "mkfifo foo")
       (rename-file "foo" "/tmp"))

bug#34069: rename-file on fifos hangs if cross device

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> Here is a minimal reproducer:
>
> (progn (shell-command "mkfifo foo")
>        (rename-file "foo" "/tmp"))

Thanks; I've now fixed this in Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#34069: rename-file on fifos hangs if cross device

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 217 bytes --]

In reviewing that patch I discovered that there's a similar bug in 
copy-file, and that there's some opportunity to simplify and robustify 
the rename-file code. I installed the attached patches on master to do that.

[-- Attachment #2: 0001-Don-t-hang-when-copying-FIFOs.patch --]
[-- Type: text/x-patch, Size: 1111 bytes --]

From ab3cfa4a17663cf479f286149a2289974dd67240 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 18 Dec 2022 11:45:06 -0800
Subject: [PATCH 1/2] =?UTF-8?q?Don=E2=80=99t=20hang=20when=20copying=20FIF?=
 =?UTF-8?q?Os?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/fileio.c (Fcopy_file): Open the input file with O_NONBLOCK.
This prevents a hang if the input file is a FIFO.
If it’s a regular file O_NONBLOCK has no effect;
otherwise the file is soon rejected anyway.
---
 src/fileio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/fileio.c b/src/fileio.c
index 92335b639c..a50f8d67c1 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -2219,7 +2219,7 @@ DEFUN ("copy-file", Fcopy_file, Scopy_file, 2, 6,
       report_file_error ("Copying permissions to", newname);
     }
 #else /* not WINDOWSNT */
-  ifd = emacs_open (SSDATA (encoded_file), O_RDONLY, 0);
+  ifd = emacs_open (SSDATA (encoded_file), O_RDONLY | O_NONBLOCK, 0);
 
   if (ifd < 0)
     report_file_error ("Opening input file", file);
-- 
2.38.1


[-- Attachment #3: 0002-Improve-rename-file-fix-bug-34069.patch --]
[-- Type: text/x-patch, Size: 2698 bytes --]

From 79e1bff2694444a27036b08e8fa2a6619b40dc2a Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 18 Dec 2022 12:57:57 -0800
Subject: [PATCH 2/2] Improve rename-file fix (bug#34069)

* src/fileio.c (Frename_file): No need for a special case to
rename a fifo, since we already tried and failed to rename it.
Also improve symlink handling, in that if readlink fails report an
error rather than trying to treat the link as a regular file.
---
 src/fileio.c | 35 ++++++++++++-----------------------
 1 file changed, 12 insertions(+), 23 deletions(-)

diff --git a/src/fileio.c b/src/fileio.c
index a50f8d67c1..789f3d509e 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -134,6 +134,7 @@ #define DRIVE_LETTER(x) c_tolower (x)
    is added here.  */
 static Lisp_Object Vwrite_region_annotation_buffers;
 
+static Lisp_Object emacs_readlinkat (int, char const *);
 static Lisp_Object file_name_directory (Lisp_Object);
 static bool a_write (int, Lisp_Object, ptrdiff_t, ptrdiff_t,
 		     Lisp_Object *, struct coding_system *);
@@ -2710,31 +2711,19 @@ DEFUN ("rename-file", Frename_file, Srename_file, 2, 3,
     }
   if (dirp)
     call4 (Qcopy_directory, file, newname, Qt, Qnil);
-  else
-    {
-      Lisp_Object symlink_target
-	= (S_ISLNK (file_st.st_mode)
-	   ? check_emacs_readlinkat (AT_FDCWD, file, SSDATA (encoded_file))
-	   : Qnil);
-      if (!NILP (symlink_target))
-	Fmake_symbolic_link (symlink_target, newname, ok_if_already_exists);
-      else if (S_ISFIFO (file_st.st_mode))
-	{
-	  /* If it's a FIFO, calling `copy-file' will hang if it's a
-	     inter-file system move, so do it here.  (It will signal
-	     an error in that case, but it won't hang in any case.)  */
-	  if (!NILP (ok_if_already_exists))
-	    barf_or_query_if_file_exists (newname, false,
-					  "rename to it",
-					  FIXNUMP (ok_if_already_exists),
-					  false);
-	  if (rename (SSDATA (encoded_file), SSDATA (encoded_newname)) != 0)
-	    report_file_errno ("Renaming", list2 (file, newname), errno);
-	  return Qnil;
-	}
+  else if (S_ISREG (file_st.st_mode))
+    Fcopy_file (file, newname, ok_if_already_exists, Qt, Qt, Qt);
+  else if (S_ISLNK (file_st.st_mode))
+    {
+      Lisp_Object target = emacs_readlinkat (AT_FDCWD,
+					     SSDATA (encoded_file));
+      if (!NILP (target))
+	Fmake_symbolic_link (target, newname, ok_if_already_exists);
       else
-	Fcopy_file (file, newname, ok_if_already_exists, Qt, Qt, Qt);
+	report_file_error ("Renaming", list2 (file, newname));
     }
+  else
+    report_file_errno ("Renaming", list2 (file, newname), rename_errno);
 
   specpdl_ref count = SPECPDL_INDEX ();
   specbind (Qdelete_by_moving_to_trash, Qnil);
-- 
2.38.1