bug#35418: [PATCH] Don't poll auto-revert files that use notification

[Mattias Engdegård]

27 apr. 2019 kl. 11.27 skrev Michael Albinus <michael.albinus@gmx.de>:
> 
> Well, in inotify you still get undesired notifications. Like this:
> 
> --8<---------------cut here---------------start------------->8---
> (write-region "foo" nil "/tmp/foo")
> (add-name-to-file "/tmp/foo" "/tmp/bar" 'ok)
> 
> (inotify-add-watch "/tmp/foo" t (lambda (event) (message "inotify %S" event)))
> => (1 . 0)
> (inotify-add-watch "/tmp/bar" t (lambda (event) (message "inotify %S" event)))
> => (1 . 1)
> (write-region "foo" nil "/tmp/foo")
> => inotify ((1 . 0) (modify) "/tmp/foo" 0)
>   inotify ((1 . 1) (modify) "/tmp/bar" 0)

Thanks for the example!

I wouldn't call this undesired. Create a hard link to a file, ask for 
notification on both links, and then modify the file. Then both notifiers 
trigger, because someone has modified the file they were watching. The kqueue 
back-end behaves similarly.

> However, in filenotify this is fixed:
> 
> --8<---------------cut here---------------start------------->8---
> (file-notify-add-watch "/tmp/foo" '(change attribute-change)
>                       (lambda (event) (message "file-notify %S" event)))
> => (2 . 0)
> (file-notify-add-watch "/tmp/bar" '(change attribute-change)
>                       (lambda (event) (message "file-notify %S" event)))
> => (2 . 1)
> (write-region "foo" nil "/tmp/foo")
> => file-notify ((2 . 0) changed "/tmp/foo")
>   inotify ((1 . 0) (modify) "/tmp/foo" 0)
>   inotify ((1 . 1) (modify) "/tmp/bar" 0)

Actually, it is (arguably) a bug. With two buffers referring to distinct hard 
links for the same file, surely we want a change in that file to trigger 
notification for both! (It's quite an exotic case, not the least because Emacs 
normally recognises hard links as if they were the same file name.)

However, with the kqueue back-end, file-notify watches do trigger for both, as 
expected.

The reason is that file-notify does not call inotify-add-watch on individual 
files, as in your example above, but on their containing directory ("/tmp" in 
your example). When monitoring a directory with two hard links to the same 
file, and the file is changed, inotify (sensibly) only reports a change to one 
of the links (the one employed for the change). Thus, the logic is in the Linux 
kernel, not in filenotify.

For kqueue it is different: here, changes to files are not reported when a 
watch is monitoring their directory, so filenotify.el sets kqueue watches on 
each file instead. The same could be done with inotify (and w32notify, if I 
read the code right), but watching directories has certain advantages.

> Unrelated events for "/tmp/bar" are filtered out in
> `file-notify-callback'. So yes, the inotify problems seem to be fixed.

Are you really sure that the inotify problems were really about changes to 
files with multiple hard links? It sounds very unlikely, and as showed above, 
the behaviour differs between back-ends.

If I were to guess, the problem was rather that the inotify back-end used to 
return the kernel-provided descriptor number, which is the same for the same 
directory: when /dir/a and /dir/b (distinct files, not hard links) both were 
watched, inotify would monitor /dir twice and give the same descriptor for 
both, with the ensuing chaos. This was subsequently fixed by making inotify 
return unique descriptors.

> We might extend this variable. *If* this regexp matches a file name, we
> know that we need polling. But it is clear, that we cannot catch all
> cases by just parsing file names.
> 
> (Btw, we should use the value of `mounted-file-systems', introduced in
> Emacs 26.1, when initializing `auto-revert-notify-exclude-dir-regexp'.)

That variable contains "^//[^/]+/" on Windows, so we need to make up our minds 
about it.

> One alternative approach could be to analyze the file system device
> number, as returned by `file-attributes'. By this, we could detect
> mounted file systems.

Sort of; the interpretation is tricky, and as Eli commented, quite 
platform-specific.

> But I don't believe that this information is always trustworty, given it
> isn't used anywhere. And at least for remote files it doesn't tell you
> anything. Furthermore, mounted file systems are not the only reason that
> file notification doesn't work, and we need to poll.

What other reasons are you thinking about?