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

["Mattias Engdegård" <mattiase@acm.org>]

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?