unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
@ 2023-10-06 17:13 Daniel Jacobowitz
  2023-10-07  6:17 ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel Jacobowitz @ 2023-10-06 17:13 UTC (permalink / raw)
  To: 66381

I use a network filesystem which sometimes has to restart, e.g.  for
updates or when credentials expire. Normally, file notifications work
fine on this filesystem, but after a restart the old notifications will
never fire. Documentation says "By default, Auto Revert mode will poll
files for changes periodically even when file notifications are used." -
experimentally the file is never polled.

Ideally the notification would be recreated, but falling back to
polling would be
an improvement.

Reproduce from emacs -Q:

(global-auto-revert-mode t)
Open a file on the filesystem where notifications break
Append to the file externally -> emacs notices immediately
Restart the daemon, breaking notifications
Append to the file externally -> emacs never notices

This has been broken for a while. From auto-revert-handler:

         (revert
          (if buffer-file-name
              (and (or auto-revert-remote-files
                       (not (file-remote-p buffer-file-name)))
                   (or (not auto-revert-notify-watch-descriptor)
                       auto-revert-notify-modified-p)
                 ... eventually poll

Prior to bug#20943, there was a call to buffer-modified-p at the top
level that checked unconditionally whether the file had been modified.

Build:

In GNU Emacs 29.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.37,
 cairo version 1.16.0) of 2023-09-03, modified by Debian built on
 kokoro-ubuntu
System Description: Debian GNU/Linux rodete

There's a bunch of local bits in our Emacs build, but I'm ~mostly sure
they are unrelated
to this bug.

-- 
Thanks,
Daniel





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-06 17:13 bug#66381: 29.1; Auto-revert not polling files when notifications are enabled Daniel Jacobowitz
@ 2023-10-07  6:17 ` Eli Zaretskii
  2023-10-07 15:15   ` Michael Albinus
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2023-10-07  6:17 UTC (permalink / raw)
  To: Daniel Jacobowitz, Michael Albinus; +Cc: 66381

> From: Daniel Jacobowitz <daniel.jacobowitz@gmail.com>
> Date: Fri, 6 Oct 2023 13:13:40 -0400
> 
> I use a network filesystem which sometimes has to restart, e.g.  for
> updates or when credentials expire. Normally, file notifications work
> fine on this filesystem, but after a restart the old notifications will
> never fire. Documentation says "By default, Auto Revert mode will poll
> files for changes periodically even when file notifications are used." -
> experimentally the file is never polled.
> 
> Ideally the notification would be recreated, but falling back to
> polling would be
> an improvement.

Michael, can we reliably know when the watch handles are stale?  If
not, then users of such "restarting" network filesystems will need to
disable auto-revert-use-notify.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-07  6:17 ` Eli Zaretskii
@ 2023-10-07 15:15   ` Michael Albinus
  2023-10-07 16:14     ` Daniel Jacobowitz
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Albinus @ 2023-10-07 15:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 66381, Daniel Jacobowitz

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

>> I use a network filesystem which sometimes has to restart, e.g.  for
>> updates or when credentials expire. Normally, file notifications work
>> fine on this filesystem, but after a restart the old notifications will
>> never fire. Documentation says "By default, Auto Revert mode will poll
>> files for changes periodically even when file notifications are used." -
>> experimentally the file is never polled.
>> 
>> Ideally the notification would be recreated, but falling back to
>> polling would be
>> an improvement.
>
> Michael, can we reliably know when the watch handles are stale?  If
> not, then users of such "restarting" network filesystems will need to
> disable auto-revert-use-notify.

We have `file-notify-valid-p'. It calls the backend specific funtion,
for example `inotify-valid-p' in the case of GNU/Linux using
inotify. The OP didn't tell us which backend he's using, so let's assume
inotify ATM.

This backend specific check does not ask the operating system, it checks
only some internal data structures. This isn't a robust check for a
stale watch handle.

However, inotify sends also an internal event IN_IGNORED, which is
raised according to inotify(7)

--8<---------------cut here---------------start------------->8---
           IN_IGNORED
                  Watch  was removed explicitly (inotify_rm_watch(2)) or auto‐
                  matically (file was deleted, or filesystem  was  unmounted).
                  See also BUGS.
--8<---------------cut here---------------end--------------->8---

filenotify.el translates this to the Emacs file-notify event `stopped',
and if this event arrives, the respective data structures are
updated. In case of a network filesystem unmount, everything shall work.

A recreation of file notification after a remount is not supported. This
would be a new feature.

However, the situation is more complex I fear. If I understand the OP
message correctly, file notification is used for auto-revert-mode. There
is the user option `auto-revert-notify-exclude-dir-regexp'. If a
directory located on a file name matching this regexp, file notification
is discarded. File names like  "/mnt", "/net/", or "/tmp_mnt/" match. So
in order to use file notification with auto-revert-mode on such a
location, this user option must be adapted.

I haven't tested the combination of watching a mounted file system, and
unmounting them, in autorevert.el. According to
`auto-revert-notify-handler', the file-notify event `stopped' is
handled, and auto-revert-mode continues with polling. I would test this
once I see a clear recipe (starting with

--8<---------------cut here---------------start------------->8---
emacs -Q --eval '(setq auto-revert-debug t file-notify-debug t)'
--8<---------------cut here---------------end--------------->8---

and telling all steps how to enable auto-revert-mode and how to unmount
the filesystem and what happens afterwards. The two debug options show
us the story in the *Messages* buffer.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-07 15:15   ` Michael Albinus
@ 2023-10-07 16:14     ` Daniel Jacobowitz
  2023-10-07 17:02       ` Michael Albinus
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel Jacobowitz @ 2023-10-07 16:14 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 66381, Eli Zaretskii

emacs -Q --eval '(setq auto-revert-debug t file-notify-debug t)'
--eval '(global-auto-revert-mode t)' file.txt

Modify the file outside of Emacs:

file-notify-handle-event (file-notify ((1 . 0) (modify) "file.txt" 0)
file-notify--callback-inotify)
file-notify-callback (1 . 0) changed "/path/to/file.txt" nil
#s(file-notify--watch "/path/to" "file.txt"
auto-revert-notify-handler) "/path/to/file.txt" "/path/to/google3"
auto-revert-notify-handler ((1 . 0) changed "/path/to/file.txt")
Reverting buffer ‘file.txt’

Restart the network filesystem (sudo systemctl restart netfs). Modify
the file outside of Emacs again.

Nothing new in *Messages* for either the restart or the modification.

Hit a key in the file buffer:

file.txt changed on disk; really edit the buffer? (y, n, r or C-h)

Same story with inotify; there's no event when the filesystem
restarts. I'm not explicitly unmounting it, systemd kills and restarts
the job providing the fuse filesystem.

The path does not match auto-revert-notify-exclude-dir-regexp.

If supporting this is of general interest, I'd do it by (A)
re-enabling polling even when using notification; (B) removing and
re-adding the watch when polling reports changes that we weren't
notified about. If it's not of general interest, the best alternative
would be removing the bit of documentation that says "By default, Auto
Revert mode will poll files for changes periodically even when file
notifications are used."

For anyone with a similar problem, since the filesystem restarts are
typically overnight as part of system updates, I went with a simple
workaround:

(run-with-timer 1800 1800 (lambda () (progn
                                       (global-auto-revert-mode 'toggle)
                                       (global-auto-revert-mode 'toggle))))

On Sat, Oct 7, 2023 at 11:15 AM Michael Albinus <michael.albinus@gmx.de> wrote:
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> Hi Eli,
>
> >> I use a network filesystem which sometimes has to restart, e.g.  for
> >> updates or when credentials expire. Normally, file notifications work
> >> fine on this filesystem, but after a restart the old notifications will
> >> never fire. Documentation says "By default, Auto Revert mode will poll
> >> files for changes periodically even when file notifications are used." -
> >> experimentally the file is never polled.
> >>
> >> Ideally the notification would be recreated, but falling back to
> >> polling would be
> >> an improvement.
> >
> > Michael, can we reliably know when the watch handles are stale?  If
> > not, then users of such "restarting" network filesystems will need to
> > disable auto-revert-use-notify.
>
> We have `file-notify-valid-p'. It calls the backend specific funtion,
> for example `inotify-valid-p' in the case of GNU/Linux using
> inotify. The OP didn't tell us which backend he's using, so let's assume
> inotify ATM.
>
> This backend specific check does not ask the operating system, it checks
> only some internal data structures. This isn't a robust check for a
> stale watch handle.
>
> However, inotify sends also an internal event IN_IGNORED, which is
> raised according to inotify(7)
>
> --8<---------------cut here---------------start------------->8---
>            IN_IGNORED
>                   Watch  was removed explicitly (inotify_rm_watch(2)) or auto‐
>                   matically (file was deleted, or filesystem  was  unmounted).
>                   See also BUGS.
> --8<---------------cut here---------------end--------------->8---
>
> filenotify.el translates this to the Emacs file-notify event `stopped',
> and if this event arrives, the respective data structures are
> updated. In case of a network filesystem unmount, everything shall work.
>
> A recreation of file notification after a remount is not supported. This
> would be a new feature.
>
> However, the situation is more complex I fear. If I understand the OP
> message correctly, file notification is used for auto-revert-mode. There
> is the user option `auto-revert-notify-exclude-dir-regexp'. If a
> directory located on a file name matching this regexp, file notification
> is discarded. File names like  "/mnt", "/net/", or "/tmp_mnt/" match. So
> in order to use file notification with auto-revert-mode on such a
> location, this user option must be adapted.
>
> I haven't tested the combination of watching a mounted file system, and
> unmounting them, in autorevert.el. According to
> `auto-revert-notify-handler', the file-notify event `stopped' is
> handled, and auto-revert-mode continues with polling. I would test this
> once I see a clear recipe (starting with
>
> --8<---------------cut here---------------start------------->8---
> emacs -Q --eval '(setq auto-revert-debug t file-notify-debug t)'
> --8<---------------cut here---------------end--------------->8---
>
> and telling all steps how to enable auto-revert-mode and how to unmount
> the filesystem and what happens afterwards. The two debug options show
> us the story in the *Messages* buffer.
>
> Best regards, Michael.



-- 
Thanks,
Daniel





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-07 16:14     ` Daniel Jacobowitz
@ 2023-10-07 17:02       ` Michael Albinus
       [not found]         ` <CAN9gPaEi7yPgC4cA9fR-TtSmsVg6CoTOZ=bbZFa1gjQA_B1vGA@mail.gmail.com>
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Albinus @ 2023-10-07 17:02 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: 66381, Eli Zaretskii

Daniel Jacobowitz <daniel.jacobowitz@gmail.com> writes:

Hi Daniel,

> emacs -Q --eval '(setq auto-revert-debug t file-notify-debug t)'
> --eval '(global-auto-revert-mode t)' file.txt
>
> Modify the file outside of Emacs:
>
> file-notify-handle-event (file-notify ((1 . 0) (modify) "file.txt" 0)
> file-notify--callback-inotify)
> file-notify-callback (1 . 0) changed "/path/to/file.txt" nil
> #s(file-notify--watch "/path/to" "file.txt"
> auto-revert-notify-handler) "/path/to/file.txt" "/path/to/google3"
> auto-revert-notify-handler ((1 . 0) changed "/path/to/file.txt")
> Reverting buffer ‘file.txt’
>
> Restart the network filesystem (sudo systemctl restart netfs). Modify
> the file outside of Emacs again.
>
> Nothing new in *Messages* for either the restart or the modification.

I miss messages like

--8<---------------cut here---------------start------------->8---
file-notify-handle-event (file-notify ((2 . 0) stopped "/net/garfunkel/Multimedia/foo") auto-revert-notify-handler)
auto-revert-notify-handler ((2 . 0) stopped "/net/garfunkel/Multimedia/foo")
--8<---------------cut here---------------end--------------->8---

which I see in my test. "/net/garfunkel/Multimedia" is the mount point.

What is "/path/to/file.txt" exactly, and what is the related mount
point? You might send this information privately, if it is disclosed information.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
       [not found]         ` <CAN9gPaEi7yPgC4cA9fR-TtSmsVg6CoTOZ=bbZFa1gjQA_B1vGA@mail.gmail.com>
@ 2023-10-07 17:59           ` Michael Albinus
  2023-10-07 18:14             ` Daniel Jacobowitz
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Albinus @ 2023-10-07 17:59 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: 66381, Eli Zaretskii

Daniel Jacobowitz <daniel.jacobowitz@gmail.com> writes:

Hi Daniel,

> It's /google/src/<censored>/file.txt, with the mount
> point on /google/src. Mount details:
>
> srcfsd /google/src fuse.srcfsd
> rw,nosuid,nodev,relatime,user_id=125,group_id=0,default_permissions,allow_other
> 0 0
>
> There's no stop notification generated, whether I restart or kill the
> daemon or just umount -l. inotifywait(1) agrees. I've got no idea what
> parts of this may be local kernel behavior versus FUSE in general, but
> I would guess that FUSE in general is more likely.

Yes, likely this is due to the srcfsd FUSE type. I don't use it, so I
cannot test.

Since inotify doesn't deliver IN_IGNORED (and the file-notify `stopped'
event doesn't appear therefore) we cannot do too much. We have no API
which sends heartbeat checks on watched file systems; such a check could
solve the problem. But there are no plans. Your workaround by restarting
global-auto-revert-mode every night might be the best you could use.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-07 17:59           ` Michael Albinus
@ 2023-10-07 18:14             ` Daniel Jacobowitz
  2023-10-07 18:28               ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel Jacobowitz @ 2023-10-07 18:14 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 66381, Eli Zaretskii

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

Coming back to my original suggestion: the documentation says that polling
is used even if notifications are enabled. The implementation does not poll
if there's a notification registered. Which behavior is intended?

On Sat, Oct 7, 2023 at 1:59 PM Michael Albinus <michael.albinus@gmx.de>
wrote:

> Daniel Jacobowitz <daniel.jacobowitz@gmail.com> writes:
>
> Hi Daniel,
>
> > It's /google/src/<censored>/file.txt, with the mount
> > point on /google/src. Mount details:
> >
> > srcfsd /google/src fuse.srcfsd
> >
> rw,nosuid,nodev,relatime,user_id=125,group_id=0,default_permissions,allow_other
> > 0 0
> >
> > There's no stop notification generated, whether I restart or kill the
> > daemon or just umount -l. inotifywait(1) agrees. I've got no idea what
> > parts of this may be local kernel behavior versus FUSE in general, but
> > I would guess that FUSE in general is more likely.
>
> Yes, likely this is due to the srcfsd FUSE type. I don't use it, so I
> cannot test.
>
> Since inotify doesn't deliver IN_IGNORED (and the file-notify `stopped'
> event doesn't appear therefore) we cannot do too much. We have no API
> which sends heartbeat checks on watched file systems; such a check could
> solve the problem. But there are no plans. Your workaround by restarting
> global-auto-revert-mode every night might be the best you could use.
>
> Best regards, Michael.
>


-- 
Thanks,
Daniel

[-- Attachment #2: Type: text/html, Size: 2030 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-07 18:14             ` Daniel Jacobowitz
@ 2023-10-07 18:28               ` Eli Zaretskii
  2023-10-07 18:41                 ` Daniel Jacobowitz
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2023-10-07 18:28 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: 66381, michael.albinus

> From: Daniel Jacobowitz <daniel.jacobowitz@gmail.com>
> Date: Sat, 7 Oct 2023 14:14:45 -0400
> Cc: 66381@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
> 
> Coming back to my original suggestion: the documentation says that polling is used even if
> notifications are enabled. The implementation does not poll if there's a notification registered.

That last sentence is not true.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-07 18:28               ` Eli Zaretskii
@ 2023-10-07 18:41                 ` Daniel Jacobowitz
  2023-10-07 18:48                   ` Daniel Jacobowitz
  2023-10-07 19:00                   ` Eli Zaretskii
  0 siblings, 2 replies; 19+ messages in thread
From: Daniel Jacobowitz @ 2023-10-07 18:41 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 66381, michael.albinus

Isn't it?

In auto-revert-handler:
https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/autorevert.el#n779

          (if buffer-file-name
              (and (or auto-revert-remote-files
                       (not (file-remote-p buffer-file-name)))
                   (or (not auto-revert-notify-watch-descriptor)
                       auto-revert-notify-modified-p)
                   (if auto-revert-tail-mode
                       (and (file-readable-p buffer-file-name)
                            (/= auto-revert-tail-pos
                                (setq size
                                      (file-attribute-size
                                       (file-attributes buffer-file-name)))))
                     (funcall (or buffer-stale-function
                                  #'buffer-stale--default-function)
                              t)))

When buffer-file-name, revert is true iff:

1. auto-revert-remote-files or the file is not remote
AND 2. there is no watch descriptor or a notification was received
AND 3. some details about auto-revert-tail-mode OR t

If auto-revert-notify-watch-descriptor and not
auto-revert-notify-modified-p, then the file won't be reverted.
auto-revert-handler does get called by the polling timer, but it
doesn't revert.


On Sat, Oct 7, 2023 at 2:28 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Daniel Jacobowitz <daniel.jacobowitz@gmail.com>
> > Date: Sat, 7 Oct 2023 14:14:45 -0400
> > Cc: 66381@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
> >
> > Coming back to my original suggestion: the documentation says that polling is used even if
> > notifications are enabled. The implementation does not poll if there's a notification registered.
>
> That last sentence is not true.



-- 
Thanks,
Daniel





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-07 18:41                 ` Daniel Jacobowitz
@ 2023-10-07 18:48                   ` Daniel Jacobowitz
  2023-10-07 19:00                   ` Eli Zaretskii
  1 sibling, 0 replies; 19+ messages in thread
From: Daniel Jacobowitz @ 2023-10-07 18:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 66381, michael.albinus

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

I definitely got that last bit about buffer-stale-function, but I think my
point still stands - the file isn't checked.

On Sat, Oct 7, 2023, 2:41 PM Daniel Jacobowitz <daniel.jacobowitz@gmail.com>
wrote:

> Isn't it?
>
> In auto-revert-handler:
> https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/autorevert.el#n779
>
>           (if buffer-file-name
>               (and (or auto-revert-remote-files
>                        (not (file-remote-p buffer-file-name)))
>                    (or (not auto-revert-notify-watch-descriptor)
>                        auto-revert-notify-modified-p)
>                    (if auto-revert-tail-mode
>                        (and (file-readable-p buffer-file-name)
>                             (/= auto-revert-tail-pos
>                                 (setq size
>                                       (file-attribute-size
>                                        (file-attributes
> buffer-file-name)))))
>                      (funcall (or buffer-stale-function
>                                   #'buffer-stale--default-function)
>                               t)))
>
> When buffer-file-name, revert is true iff:
>
> 1. auto-revert-remote-files or the file is not remote
> AND 2. there is no watch descriptor or a notification was received
> AND 3. some details about auto-revert-tail-mode OR t
>
> If auto-revert-notify-watch-descriptor and not
> auto-revert-notify-modified-p, then the file won't be reverted.
> auto-revert-handler does get called by the polling timer, but it
> doesn't revert.
>
>
> On Sat, Oct 7, 2023 at 2:28 PM Eli Zaretskii <eliz@gnu.org> wrote:
> >
> > > From: Daniel Jacobowitz <daniel.jacobowitz@gmail.com>
> > > Date: Sat, 7 Oct 2023 14:14:45 -0400
> > > Cc: 66381@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
> > >
> > > Coming back to my original suggestion: the documentation says that
> polling is used even if
> > > notifications are enabled. The implementation does not poll if there's
> a notification registered.
> >
> > That last sentence is not true.
>
>
>
> --
> Thanks,
> Daniel
>

[-- Attachment #2: Type: text/html, Size: 3178 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-07 18:41                 ` Daniel Jacobowitz
  2023-10-07 18:48                   ` Daniel Jacobowitz
@ 2023-10-07 19:00                   ` Eli Zaretskii
  2023-10-08  8:24                     ` Michael Albinus
  1 sibling, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2023-10-07 19:00 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: 66381, michael.albinus

> From: Daniel Jacobowitz <daniel.jacobowitz@gmail.com>
> Date: Sat, 7 Oct 2023 14:41:35 -0400
> Cc: michael.albinus@gmx.de, 66381@debbugs.gnu.org
> 
> In auto-revert-handler:
> https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/autorevert.el#n779
> 
>           (if buffer-file-name
>               (and (or auto-revert-remote-files
>                        (not (file-remote-p buffer-file-name)))
>                    (or (not auto-revert-notify-watch-descriptor)
>                        auto-revert-notify-modified-p)
>                    (if auto-revert-tail-mode
>                        (and (file-readable-p buffer-file-name)
>                             (/= auto-revert-tail-pos
>                                 (setq size
>                                       (file-attribute-size
>                                        (file-attributes buffer-file-name)))))
>                      (funcall (or buffer-stale-function
>                                   #'buffer-stale--default-function)
>                               t)))
> 
> When buffer-file-name, revert is true iff:
> 
> 1. auto-revert-remote-files or the file is not remote
> AND 2. there is no watch descriptor or a notification was received
> AND 3. some details about auto-revert-tail-mode OR t
> 
> If auto-revert-notify-watch-descriptor and not
> auto-revert-notify-modified-p, then the file won't be reverted.
> auto-revert-handler does get called by the polling timer, but it
> doesn't revert.

That's not what your sentence said, which I said wasn't true.  You
said something much more radical:

  The implementation does not poll if there's a notification registered.

Moreover, the documentation says that "polling is used even if
notifications are enabled", and that is true regardless of whether the
file is actually reverted or not.

IOW, the documentation describes the usual case, where notifications
are enabled and the watch descriptor is valid.  It says nothing at all
about your situation.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-07 19:00                   ` Eli Zaretskii
@ 2023-10-08  8:24                     ` Michael Albinus
  2023-10-08 12:38                       ` Michael Albinus
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Albinus @ 2023-10-08  8:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 66381, Daniel Jacobowitz

Eli Zaretskii <eliz@gnu.org> writes:

Hi,

>>           (if buffer-file-name
>>               (and (or auto-revert-remote-files
>>                        (not (file-remote-p buffer-file-name)))
>>                    (or (not auto-revert-notify-watch-descriptor)
>>                        auto-revert-notify-modified-p)
>>                    (if auto-revert-tail-mode
>>                        (and (file-readable-p buffer-file-name)
>>                             (/= auto-revert-tail-pos
>>                                 (setq size
>>                                       (file-attribute-size
>>                                        (file-attributes buffer-file-name)))))
>>                      (funcall (or buffer-stale-function
>>                                   #'buffer-stale--default-function)
>>                               t)))
>>
>> When buffer-file-name, revert is true iff:
>>
>> 1. auto-revert-remote-files or the file is not remote
>> AND 2. there is no watch descriptor or a notification was received
>> AND 3. some details about auto-revert-tail-mode OR t
>>
>> If auto-revert-notify-watch-descriptor and not
>> auto-revert-notify-modified-p, then the file won't be reverted.
>> auto-revert-handler does get called by the polling timer, but it
>> doesn't revert.
>
> That's not what your sentence said, which I said wasn't true.  You
> said something much more radical:
>
>   The implementation does not poll if there's a notification registered.
>
> Moreover, the documentation says that "polling is used even if
> notifications are enabled", and that is true regardless of whether the
> file is actually reverted or not.
>
> IOW, the documentation describes the usual case, where notifications
> are enabled and the watch descriptor is valid.  It says nothing at all
> about your situation.

The problem is, that in Daniel's case we get no information that file
notification has ceased to work for the re-mounted file system (no
IN_IGNORED event from inotify). auto-revert-notify-watch-descriptor
still exists, and auto-revert-notify-modified-p isn't modified any
longer. Therefore the file is not reverted even while polling.

You have a wokaround which seems to work. Another approach would be use
polling only for the given mounted file system. Adding "^/google/src" to
auto-revert-notify-exclude-dir-regexp might work.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-08  8:24                     ` Michael Albinus
@ 2023-10-08 12:38                       ` Michael Albinus
  2023-10-09 18:17                         ` Daniel Jacobowitz
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Albinus @ 2023-10-08 12:38 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 66381, Daniel Jacobowitz

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

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

Hi,

> The problem is, that in Daniel's case we get no information that file
> notification has ceased to work for the re-mounted file system (no
> IN_IGNORED event from inotify). auto-revert-notify-watch-descriptor
> still exists, and auto-revert-notify-modified-p isn't modified any
> longer. Therefore the file is not reverted even while polling.

I've debugged further, and it looks like there is a gap in forwarding
the IN_IGNORED event from inotify.c to filenotify.el. Oops.

Daniel, could you pls try the appended patch? It shall enable this to
work, meaning auto-revert-mode is informed about the unmounted file
system. I've tested with an NFS mount, 'tho.

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1961 bytes --]

diff --git a/lisp/filenotify.el b/lisp/filenotify.el
index e9f8d4e515d..16a154723b8 100644
--- a/lisp/filenotify.el
+++ b/lisp/filenotify.el
@@ -138,7 +138,7 @@ file-notify--callback-inotify
                         ((memq action '(delete delete-self move-self)) 'deleted)
                         ((eq action 'moved-from) 'renamed-from)
                         ((eq action 'moved-to) 'renamed-to)
-                        ((eq action 'ignored) 'stopped)))
+                        ((memq action '(ignored unmount)) 'stopped)))
                      actions))
    file file1-or-cookie))

@@ -202,7 +202,7 @@ file-notify-callback
                  ((memq action '(delete delete-self move-self)) 'deleted)
                  ((eq action 'moved-from) 'renamed-from)
                  ((eq action 'moved-to) 'renamed-to)
-                 ((eq action 'ignored) 'stopped)))
+                 ((memq action '(ignored unmount)) 'stopped)))
               (if (consp actions) actions (list actions))))
    file file1-or-cookie))

@@ -339,7 +339,7 @@ file-notify--add-watch-inotify
   "Add a watch for FILE in DIR with FLAGS, using inotify."
   (inotify-add-watch dir
                      (append
-                      '(dont-follow)
+                      '(dont-follow ignored unmount)
                       (and (memq 'change flags)
                            '(create delete delete-self modify move-self move))
                       (and (memq 'attribute-change flags)
diff --git a/src/inotify.c b/src/inotify.c
index 105ff5a9d8a..247d9f03055 100644
--- a/src/inotify.c
+++ b/src/inotify.c
@@ -148,6 +148,11 @@ symbol_to_inotifymask (Lisp_Object symb)
   else if (EQ (symb, Qonlydir))
     return IN_ONLYDIR;

+  else if (EQ (symb, Qignored))
+    return IN_IGNORED;
+  else if (EQ (symb, Qunmount))
+    return IN_UNMOUNT;
+
   else if (EQ (symb, Qt) || EQ (symb, Qall_events))
     return IN_ALL_EVENTS;
   else

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-08 12:38                       ` Michael Albinus
@ 2023-10-09 18:17                         ` Daniel Jacobowitz
  2023-10-10 18:03                           ` Michael Albinus
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel Jacobowitz @ 2023-10-09 18:17 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 66381, Eli Zaretskii

Thanks for the follow-up!

First of all, for anyone who stumbles on this, inotifywait's
documentation says it listens to all events by default. But it doesn't
request IN_UNMOUNT unless you use `-e unmount`. That confused all of
my test results - that's why I thought the event was never generated.
In possession of this new knowledge, I verified that an unmount event
is delivered to inotifywait when my filesystem daemon restarts. So
that's great progress.

I built a patched emacs and tried unmounting the filesystem. Emacs was notified:

file-notify-handle-event (file-notify ((1 . 0) (unmount isdir)
"/google/src/cloud/dmj/emacs/google3" 0)
file-notify--callback-inotify)
file-notify-handle-event (file-notify ((1 . 0) stopped
"/google/src/cloud/dmj/emacs/google3/file.txt")
auto-revert-notify-handler)
auto-revert-notify-handler ((1 . 0) stopped
"/google/src/cloud/dmj/emacs/google3/file.txt")
file-notify-handle-event (file-notify ((1 . 0) stopped
"/google/src/cloud/dmj/emacs/google3/file.txt")
auto-revert-notify-handler)
auto-revert-notify-handler ((1 . 0) stopped
"/google/src/cloud/dmj/emacs/google3/file.txt")
file-notify-handle-event (file-notify ((1 . 0) (ignored)
"/google/src/cloud/dmj/emacs/google3" 0)
file-notify--callback-inotify)

It works! Thanks!

On Sun, Oct 8, 2023 at 8:38 AM Michael Albinus <michael.albinus@gmx.de> wrote:
>
> Michael Albinus <michael.albinus@gmx.de> writes:
>
> Hi,
>
> > The problem is, that in Daniel's case we get no information that file
> > notification has ceased to work for the re-mounted file system (no
> > IN_IGNORED event from inotify). auto-revert-notify-watch-descriptor
> > still exists, and auto-revert-notify-modified-p isn't modified any
> > longer. Therefore the file is not reverted even while polling.
>
> I've debugged further, and it looks like there is a gap in forwarding
> the IN_IGNORED event from inotify.c to filenotify.el. Oops.
>
> Daniel, could you pls try the appended patch? It shall enable this to
> work, meaning auto-revert-mode is informed about the unmounted file
> system. I've tested with an NFS mount, 'tho.
>
> Best regards, Michael.
>


-- 
Thanks,
Daniel





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-09 18:17                         ` Daniel Jacobowitz
@ 2023-10-10 18:03                           ` Michael Albinus
  2023-10-13 12:05                             ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Albinus @ 2023-10-10 18:03 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: 66381, Eli Zaretskii

Daniel Jacobowitz <daniel.jacobowitz@gmail.com> writes:

Hi Daniel,

> First of all, for anyone who stumbles on this, inotifywait's
> documentation says it listens to all events by default. But it doesn't
> request IN_UNMOUNT unless you use `-e unmount`.

Noted.

> I built a patched emacs and tried unmounting the filesystem. Emacs was notified:
>
> file-notify-handle-event (file-notify ((1 . 0) (unmount isdir)
> "/google/src/cloud/dmj/emacs/google3" 0)
> file-notify--callback-inotify)
> file-notify-handle-event (file-notify ((1 . 0) stopped
> "/google/src/cloud/dmj/emacs/google3/file.txt")
> auto-revert-notify-handler)
> auto-revert-notify-handler ((1 . 0) stopped
> "/google/src/cloud/dmj/emacs/google3/file.txt")
> file-notify-handle-event (file-notify ((1 . 0) stopped
> "/google/src/cloud/dmj/emacs/google3/file.txt")
> auto-revert-notify-handler)
> auto-revert-notify-handler ((1 . 0) stopped
> "/google/src/cloud/dmj/emacs/google3/file.txt")
> file-notify-handle-event (file-notify ((1 . 0) (ignored)
> "/google/src/cloud/dmj/emacs/google3" 0)
> file-notify--callback-inotify)
>
> It works! Thanks!

Thanks for the feedback.

I've checked the other file notification backends, and it turns out,
that none of them reports unmount events. So I have extended the patch,
the unmount event is propagated now in all backends (under different
event names), and it causes to stop the respective file notification
watch.

This works for all but the w32notify backend. I dont know whether this
library supports a kind of unmount events, and even in case it does, I
wouldn't be able to implement this. Eli, do we want to do something
here?

Everything is pushed to the master branch. Perhaps we could also add a
test case to filenotify-tests.el, but I have no idea how to emulate
unmounting in a test environment.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-10 18:03                           ` Michael Albinus
@ 2023-10-13 12:05                             ` Eli Zaretskii
  2023-10-15 13:58                               ` Michael Albinus
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2023-10-13 12:05 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 66381, daniel.jacobowitz

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Eli Zaretskii <eliz@gnu.org>,  66381@debbugs.gnu.org
> Date: Tue, 10 Oct 2023 20:03:48 +0200
> 
> I've checked the other file notification backends, and it turns out,
> that none of them reports unmount events. So I have extended the patch,
> the unmount event is propagated now in all backends (under different
> event names), and it causes to stop the respective file notification
> watch.
> 
> This works for all but the w32notify backend. I dont know whether this
> library supports a kind of unmount events, and even in case it does, I
> wouldn't be able to implement this. Eli, do we want to do something
> here?

I don't know, I will have to test this.

Could you please help me by showing some Lisp I could use to see if
unmounting a volume gets reported in some way to the
file-notifications machinery?  I have very little free time these
days, for more than one good reason, but would like not to postpone
investigating this for too long, which will happen if I need to find
the recipe myself.  TIA.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-13 12:05                             ` Eli Zaretskii
@ 2023-10-15 13:58                               ` Michael Albinus
  2023-10-16 12:38                                 ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Michael Albinus @ 2023-10-15 13:58 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 66381, daniel.jacobowitz

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

>> I've checked the other file notification backends, and it turns out,
>> that none of them reports unmount events. So I have extended the patch,
>> the unmount event is propagated now in all backends (under different
>> event names), and it causes to stop the respective file notification
>> watch.
>>
>> This works for all but the w32notify backend. I dont know whether this
>> library supports a kind of unmount events, and even in case it does, I
>> wouldn't be able to implement this. Eli, do we want to do something
>> here?
>
> I don't know, I will have to test this.
>
> Could you please help me by showing some Lisp I could use to see if
> unmounting a volume gets reported in some way to the
> file-notifications machinery?  I have very little free time these
> days, for more than one good reason, but would like not to postpone
> investigating this for too long, which will happen if I need to find
> the recipe myself.  TIA.

I would apply the following recipe:

- Use a mounted share. In my environment, "z:" is a Samba mount.

- Call

--8<---------------cut here---------------start------------->8---
$ emacs -l filenotify --eval '(setq file-notify-debug t)' --eval '(file-notify-add-watch "z:/123" (quote (change attribute-change)) (quote ignore))'
--8<---------------cut here---------------end--------------->8---

- In another shell outside Emacs, call

--8<---------------cut here---------------start------------->8---
$ touch z:/123
--8<---------------cut here---------------end--------------->8---

- You'll see in Emacs the messages

--8<---------------cut here---------------start------------->8---
file-notify-handle-event (file-notify (541405157192 modified "123") file-notify--callback-w32notify)
file-notify-callback 541405157192 changed "z:/123" nil #s(file-notify--watch "z:/" "123" ignore) "z:/123" "z:/"
--8<---------------cut here---------------end--------------->8---

- Unmount the share. There shall be another file notification, but there
  isn't.

In a similar scenario I use with Emacs/inotify, I see the meesages

--8<---------------cut here---------------start------------->8---
file-notify-handle-event (file-notify ((1 . 0) (attrib) "123" 0) file-notify--callback-inotify)
file-notify-callback (1 . 0) attribute-changed "/tmp/tramp.sshfs.detlef/home/albinus/123" nil #s(file-notify--watch "/tmp/tramp.sshfs.detlef/home/albinus" "123" ignore) "/tmp/tramp.sshfs.detlef/home/albinus/123" "/tmp/tramp.sshfs.detlef/home/albinus"
file-notify-handle-event (file-notify ((1 . 0) (unmount isdir) "/tmp/tramp.sshfs.detlef/home/albinus" 0) file-notify--callback-inotify)
file-notify-handle-event (file-notify ((1 . 0) stopped "/tmp/tramp.sshfs.detlef/home/albinus/123") ignore)
file-notify-handle-event (file-notify ((1 . 0) (ignored) "/tmp/tramp.sshfs.detlef/home/albinus" 0) file-notify--callback-inotify)
--8<---------------cut here---------------end--------------->8---

"/tmp/tramp.sshfs.detlef" is the mount point in that case.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-15 13:58                               ` Michael Albinus
@ 2023-10-16 12:38                                 ` Eli Zaretskii
  2023-10-16 14:29                                   ` Michael Albinus
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2023-10-16 12:38 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 66381, daniel.jacobowitz

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: daniel.jacobowitz@gmail.com,  66381@debbugs.gnu.org
> Date: Sun, 15 Oct 2023 15:58:56 +0200
> 
> - Use a mounted share. In my environment, "z:" is a Samba mount.
> 
> - Call
> 
> --8<---------------cut here---------------start------------->8---
> $ emacs -l filenotify --eval '(setq file-notify-debug t)' --eval '(file-notify-add-watch "z:/123" (quote (change attribute-change)) (quote ignore))'
> --8<---------------cut here---------------end--------------->8---
> 
> - In another shell outside Emacs, call
> 
> --8<---------------cut here---------------start------------->8---
> $ touch z:/123
> --8<---------------cut here---------------end--------------->8---
> 
> - You'll see in Emacs the messages
> 
> --8<---------------cut here---------------start------------->8---
> file-notify-handle-event (file-notify (541405157192 modified "123") file-notify--callback-w32notify)
> file-notify-callback 541405157192 changed "z:/123" nil #s(file-notify--watch "z:/" "123" ignore) "z:/123" "z:/"
> --8<---------------cut here---------------end--------------->8---
> 
> - Unmount the share. There shall be another file notification, but there
>   isn't.

On Windows, I cannot safely unmount the volume: Windows tells me that
it cannot be safely unmounted (because watching a filer on it has a
handle open on the volume).  If I unmount the volume forcibly, I see
what you expected me to see: nothing.

I guess this means the "cannot safely unmount" message will have to do
on MS-Windows.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#66381: 29.1; Auto-revert not polling files when notifications are enabled
  2023-10-16 12:38                                 ` Eli Zaretskii
@ 2023-10-16 14:29                                   ` Michael Albinus
  0 siblings, 0 replies; 19+ messages in thread
From: Michael Albinus @ 2023-10-16 14:29 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 66381-done, daniel.jacobowitz

Version: 30.1

Eli Zaretskii <eliz@gnu.org> writes:

Hi Eli,

> On Windows, I cannot safely unmount the volume: Windows tells me that
> it cannot be safely unmounted (because watching a filer on it has a
> handle open on the volume).  If I unmount the volume forcibly, I see
> what you expected me to see: nothing.
>
> I guess this means the "cannot safely unmount" message will have to do
> on MS-Windows.

Yes, might be OK. For inotify I have a similar issue with NFS
mounts. That's why I've used an FUSE mount in my example.

Nothing left to do, so I'm closing the bug.

Best regards, Michael.





^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-10-16 14:29 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-06 17:13 bug#66381: 29.1; Auto-revert not polling files when notifications are enabled Daniel Jacobowitz
2023-10-07  6:17 ` Eli Zaretskii
2023-10-07 15:15   ` Michael Albinus
2023-10-07 16:14     ` Daniel Jacobowitz
2023-10-07 17:02       ` Michael Albinus
     [not found]         ` <CAN9gPaEi7yPgC4cA9fR-TtSmsVg6CoTOZ=bbZFa1gjQA_B1vGA@mail.gmail.com>
2023-10-07 17:59           ` Michael Albinus
2023-10-07 18:14             ` Daniel Jacobowitz
2023-10-07 18:28               ` Eli Zaretskii
2023-10-07 18:41                 ` Daniel Jacobowitz
2023-10-07 18:48                   ` Daniel Jacobowitz
2023-10-07 19:00                   ` Eli Zaretskii
2023-10-08  8:24                     ` Michael Albinus
2023-10-08 12:38                       ` Michael Albinus
2023-10-09 18:17                         ` Daniel Jacobowitz
2023-10-10 18:03                           ` Michael Albinus
2023-10-13 12:05                             ` Eli Zaretskii
2023-10-15 13:58                               ` Michael Albinus
2023-10-16 12:38                                 ` Eli Zaretskii
2023-10-16 14:29                                   ` Michael Albinus

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).