bug#46397: 27.1; Cannot delete buffer pointing to a file in a path that includes a file

[Paul Eggert <eggert@cs.ucla.edu>]

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

On 2/10/21 7:05 AM, Eli Zaretskii wrote:

> I think instead of ignoring some errors, we should allow the user to
> get out of these situations, after showing the error.  That is,
> instead of silently ignoring the error on some low level, we should
> propagate it to userlock.el and allow the user to decide whether
> he/she wants to ignore the error or do something about that.  These
> errors could mean something innocent, or they could mean something
> more serious, and we shouldn't second-guess which one is it.

I agree with this. However, I think there are two bugs here.

The first bug is that the low-level locking code is busted, in that it 
thinks there's a lock file when ENOTDIR says there isn't. I installed 
the first attached patch into master to fix this bug. This patch isn't 
what Matt suggested - it's a bit earlier in the low-level C code where 
the bug really occurred. (The area that Matt was looking at was later 
on, when we have the lock and are removing it, and the ENOENT check 
there is for the rare case where NFS is being used and its unlink 
implementation gets confused and fails with ENOENT even though it was 
actually successful and where any error other than ENOENT is serious.)

The second bug is that once you're in a tricky situation where the file 
can't be unlocked for whatever reason and you attempt to exit Emacs, 
Emacs tries to auto-save the buffer, fails because of the lock problem, 
and then gets into a weird state where you cannot do anything. This 
problem can happen in other scenarios. For example:

* Run Emacs and visit the file /tmp/a/b where /tmp/a does not exist. 
Emacs will warn "Use M-x make-directory RET RET to create the directory 
and its parents"; ignore the warning.

* Type some characters so that /tmp/a/b's buffer is nonempty.

* Create an inaccessible directory /tmp/a by running "mkdir -m 0 /tmp/a" 
outside Emacs.

* Type C-x C-c to exit Emacs. It will say "Save file /tmp/a/b?" Type 
"n". It will then say "Modified buffers exist; exit anyway? (yes or 
no)". Type "yes". Emacs will then hang, in a weird state where it is 
trying to auto-save but hanging in the middle of that.

I did not fix this latter problem, so it needs further investigation. 
While looking into it, though, I did see some longstanding code in 
files.el that could use a bit of sprucing up given the more-modern 
primitives available now, and so installed the second attached patch 
into master.

[-- Attachment #2: 0001-Fix-file-lock-issue-Bug-46397.patch --]
[-- Type: text/x-patch, Size: 958 bytes --]

From 4459dcc07865f6ae1f21f624fcb09cf8fdaecdb5 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 10 Feb 2021 10:50:44 -0800
Subject: [PATCH 1/2] Fix file lock issue (Bug#46397)

* src/filelock.c (current_lock_owner):
Also treat ENOTDIR as meaning the lock file does not exist.
---
 src/filelock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/filelock.c b/src/filelock.c
index 35baa0c666..373fc00a42 100644
--- a/src/filelock.c
+++ b/src/filelock.c
@@ -532,7 +532,7 @@ current_lock_owner (lock_info_type *owner, char *lfname)
   /* If nonexistent lock file, all is well; otherwise, got strange error. */
   lfinfolen = read_lock_data (lfname, owner->user);
   if (lfinfolen < 0)
-    return errno == ENOENT ? 0 : errno;
+    return errno == ENOENT || errno == ENOTDIR ? 0 : errno;
   if (MAX_LFINFO < lfinfolen)
     return ENAMETOOLONG;
   owner->user[lfinfolen] = 0;
-- 
2.27.0


[-- Attachment #3: 0002-Simplify-and-speed-up-after-find-file.patch --]
[-- Type: text/x-patch, Size: 2349 bytes --]

From 4467073c50d2c7fbbb30530d1a0a25f8272ff56f Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 10 Feb 2021 10:55:42 -0800
Subject: [PATCH 2/2] Simplify and speed up after-find-file

Use newer primitives like file-accessible-directory-p to simplify
and speed up longstanding code in after-find-file.
* lisp/files.el (after-find-file):
Prefer file-exists-p + file-symlink-p to file-attributes +
file-symlink-p + file-chase-links + file-exists-p.
Prefer file-accessible-directory-p to directory-file-name +
file-attributes.
Prefer file-directory-p to file-name-directory + file-exists-p.
---
 lisp/files.el | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index dada69c145..9ff8f31e37 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -2530,13 +2530,11 @@ after-find-file
 	   (msg
 	    (cond
 	     ((not warn) nil)
-	     ((and error (file-attributes buffer-file-name))
+	     ((and error (file-exists-p buffer-file-name))
 	      (setq buffer-read-only t)
-	      (if (and (file-symlink-p buffer-file-name)
-		       (not (file-exists-p
-			     (file-chase-links buffer-file-name))))
-		  "Symbolic link that points to nonexistent file"
-		"File exists, but cannot be read"))
+	      "File exists, but cannot be read")
+	     ((and error (file-symlink-p buffer-file-name))
+	      "Symbolic link that points to nonexistent file")
 	     ((not buffer-read-only)
 	      (if (and warn
 		       ;; No need to warn if buffer is auto-saved
@@ -2553,13 +2551,12 @@ after-find-file
 	     ((not error)
 	      (setq not-serious t)
 	      "Note: file is write protected")
-	     ((file-attributes (directory-file-name default-directory))
+	     ((file-accessible-directory-p default-directory)
 	      "File not found and directory write-protected")
-	     ((file-exists-p (file-name-directory buffer-file-name))
-	      (setq buffer-read-only nil))
 	     (t
 	      (setq buffer-read-only nil)
-	      "Use M-x make-directory RET RET to create the directory and its parents"))))
+	      (unless (file-directory-p default-directory)
+		"Use M-x make-directory RET RET to create the directory and its parents")))))
       (when msg
 	(message "%s" msg)
 	(or not-serious (sit-for 1 t))))
-- 
2.27.0