bug#22859: 25.0.91; Errors in auto-revert-notify-handler

bug#22859: 25.0.91; Errors in auto-revert-notify-handler

[John Wiegley]

While doing general editing and saving of files with emacs-25, I'm getting the
following backtrace somewhat often:

--8<---------------cut here---------------start------------->8---
Debugger entered: ((cl-assertion-failed (stringp file1) nil))
  cl--assertion-failed((stringp file1))
  auto-revert-notify-handler((29 renamed "/Users/johnw/bae/rings/vignettes/simple_parser_fiat/Main.hs"))
  file-notify-callback((29 (rename) "/Users/johnw/bae/rings/vignettes/simple_parser_fiat/Main.hs"))
  file-notify-handle-event((file-notify (29 (rename) "/Users/johnw/bae/rings/vignettes/simple_parser_fiat/Main.hs") file-notify-callback))
  funcall-interactively(file-notify-handle-event (file-notify (29 (rename) "/Users/johnw/bae/rings/vignettes/simple_parser_fiat/Main.hs") file-notify-callback))
  call-interactively(file-notify-handle-event nil [(file-notify (29 (rename) "/Users/johnw/bae/rings/vignettes/simple_parser_fiat/Main.hs") file-notify-callback)])
  command-execute(file-notify-handle-event nil [(file-notify (29 (rename) "/Users/johnw/bae/rings/vignettes/simple_parser_fiat/Main.hs") file-notify-callback)] t)
--8<---------------cut here---------------end--------------->8---

The responsible code, in autorevert.el:

--8<---------------cut here---------------start------------->8---
(defun auto-revert-notify-handler (event)
  "Handle an EVENT returned from file notification."
  (with-demoted-errors
    (let* ((descriptor (car event))
	   (action (nth 1 event))
	   (file (nth 2 event))
	   (file1 (nth 3 event)) ;; Target of `renamed'.
	   (buffers (gethash descriptor
			     auto-revert-notify-watch-descriptor-hash-list)))
      ;; Check, that event is meant for us.
      (cl-assert descriptor)
      ;; Since we watch a directory, a file name must be returned.
      (cl-assert (stringp file))
      (when (eq action 'renamed) (cl-assert (stringp file1)))
--8<---------------cut here---------------end--------------->8---

However, I'm not performing any file renames. Perhaps this is due to the
renaming of files used when creating backups? What code might be raising a
rename enough without a rename target? From a cursory inspection, it looks
like it might be the file notification logic?

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#22859: 25.0.91; Errors in auto-revert-notify-handler

[Michael Albinus]

John Wiegley <jwiegley@gmail.com> writes:

Hi John,

> However, I'm not performing any file renames. Perhaps this is due to the
> renaming of files used when creating backups?

Yes.

> What code might be raising a rename enough without a rename target?
> From a cursory inspection, it looks like it might be the file
> notification logic?

Yes, it is file notification. The kqueue backend is inferior compared to
the other backends. In case of a file rename, it reports only the source
file name. The other backends report also the target file name.

I've fixed this by propagating a `deleted' event upwards in such
cases. autorevert falls back to the polling mode, as it should. If you
want to avoid this effect, you might set `backup-by-copying' to t.

Could you, pls, check whether the patch works for you? I've pushed it to
the emacs-25 branch.

Best regards, Michael.

bug#22859: 25.0.91; Errors in auto-revert-notify-handler

[John Wiegley]

>>>>> Michael Albinus <michael.albinus@gmx.de> writes:

> I've fixed this by propagating a `deleted' event upwards in such cases.
> autorevert falls back to the polling mode, as it should. If you want to
> avoid this effect, you might set `backup-by-copying' to t.

But no matter what backup-by-copying is set to, we won't see the failing
rename event, right?

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#22859: 25.0.91; Errors in auto-revert-notify-handler

[Michael Albinus]

John Wiegley <jwiegley@gmail.com> writes:

>> I've fixed this by propagating a `deleted' event upwards in such cases.
>> autorevert falls back to the polling mode, as it should. If you want to
>> avoid this effect, you might set `backup-by-copying' to t.
>
> But no matter what backup-by-copying is set to, we won't see the failing
> rename event, right?

Yes.

Best regards, Michael.

bug#22859: 25.0.91; Errors in auto-revert-notify-handler

[Michael Albinus]

Michael Albinus <michael.albinus@gmx.de> writes:

Hi John,

> I've fixed this by propagating a `deleted' event upwards in such
> cases. autorevert falls back to the polling mode, as it should. If you
> want to avoid this effect, you might set `backup-by-copying' to t.
>
> Could you, pls, check whether the patch works for you? I've pushed it to
> the emacs-25 branch.

Do you still want to crosscheck the fix, or could I close the bug?

Best regards, Michael.

bug#22859: 25.0.91; Errors in auto-revert-notify-handler

[John Wiegley]

>>>>> Michael Albinus <michael.albinus@gmx.de> writes:

> Do you still want to crosscheck the fix, or could I close the bug?

I haven't noticed the problem since you pushed that fix, so it appears
resolved to me.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#22859: 25.0.91; Errors in auto-revert-notify-handler

[Michael Albinus]

Version: 25.1

John Wiegley <jwiegley@gmail.com> writes:

>> Do you still want to crosscheck the fix, or could I close the bug?
>
> I haven't noticed the problem since you pushed that fix, so it appears
> resolved to me.

Thanks for confirmation, I'm closing the bug.

Best regards, Michael.