bug#53432: [PATCH] Avoid losing keyboard input when inotify is too busy [and 1 more messages]

[Ian Jackson]

Michael Albinus writes ("Re: bug#53432: [PATCH] Avoid losing keyboard input 
when inotify is too busy [and 1 more messages]"):
> Ian Jackson <ijackson@chiark.greenend.org.uk> writes:
> > I think we are just having a semantic difference here.  To my mind
> > "handling" lost events *consists of* notifying the next layer up that
> > "some events may have been lost".  Eventually this informaton will get
> > to somewhere that can do something sensible with it.
> 
> Yes, if we are speaking about the file notification backends, that's
> all. And that's what I meant (sometimes, you must forgive me if I use
> unprecise English).

Oh, no need to apologise!  It is very easy when discussing such
complicated things for people to misunderstand each other.  I just
meant to clarify.

> > The file notification code must have a "check all files we are
> > polling" facility, for when file events are not available.  Perhaps
> > this is buried in a polling loop, but it could be made separately
> > callable.
> 
> I don't believe we can implement something at filenotify.el level. It is
> up to the applications to handle lost events. autorevert could fall back
> to polling, and it could continue to trust file notification events when
> the situation becomes normal. How it is implement shall be the
> responsibility of the application.

Right, I think you are suggesting that the "some file event got lost"
notification should bubble up to autorevert.  Either directly, or via
filenotify, I guess - not sure which you are suggesting would be
better.

That sounds correct to me.

> > I think that the correct response to discovering that file events have
> > been lost, is to do a one-off poll of every file.  Perhaps we would
> > want to rate limit this, but that should be done by *deferring* a
> > poll-all, not by *skipping* one, as skipping would introduce a race
> > that might lose a buffer revert.  But I don't think this is actually
> > necessary.  In any case no keyboard events will be lost.
> 
> For autorevert (as example), using polling would sync all buffers
> towards the real state of the file on disk. Restarting file notification
> later on wouldn't be a problem.

Precisely.

> > So there would want to be some kind of timer there, and the system
> > would have two modes.  It seems simpler to me to have only one mode.
> > Writing the code to switch modes and coordinate everything seems
> > error-prone to me.
> 
> Timer doesn't sound good to me. A better approach might be, if the file
> notification libraries send a "back to normal" triger, perhaps also via
> a callback.

Let me consider the semantics of these notifications you are
suggesting in a bit more detail.  I hope you will forgive the
resulting screed:

As I understand your proposal, the system starts in "normal" state.
In "normal" state, the file notification system should promise that if
  1. a file notification is set up
  2. subsequently, that file is modified
then the file notification system will make one of these callbacks
  A. file notification event
  B. "file notifications not working" callback
(And, I gyuess, that this will be done "reasonably promptly" in some
sense; I'm not sure if we need to define this.)

If B happens then the system is now in "non-working" state.  In
non-working state, all bets are off.  Users like autorevert must take
alternative measures.

At some point, the file notification system will make a "file
notifications working again now" callback.  I think this has to
promise that, if:
  1a. a file notification was set up
  1b. the "notifications working" callback was made more recently
       than any "notifications not working" callback
(in either order) and then *subsequently*
  2. a relevant file is notified
then a notification will occur, as above.  Obviously it can't sensibly
make guarantees about what happened before the "working" notifcation.

How must a user of this API respond ?  Consider autorevert.  Suppose
the notifications were declared "non-working" and autorevert resorted
to polling.

When autorevert is told things are working again, it wants to go back
to not polling.  But it doesn't know whether notifications were lost
between its last poll, and the file notifications becoming normal
again.  There might be a few leftover lost events.

So autorevert must poll every file again, once.  I think it makes most
sense to do this immediately, rather than wait for the next poll
interval.  That allows autorevert to avoid a confusing special-case
transitional state where it is not polling but still has one poll to
do - and it means the user gets the buffers reverted as soon as
possible rather than after a timeout.

Now, consider the behaviour of the file notification system.

In practice, inotify.c will probably want to declare itself working as
soon as the event buffer it is writing to becomes empty.  Doing so
sooner merely imposes more load on a busy emacs; doing so later
degrades the user experience by having the emacs unnecessarily wait
before catching up with the work that needs to be done.

What this means is that the file notification system is only in the
"non-working" state when emacs is too busy handling important events
to catch its breath and handle file notifications.  Ie, only when
emacs is not idle.

In this state, we do not actually want to poll for file notifications!
It doesn't make sense to make this fire off a timer.

So autorevert does not actually need to do anything with the "not
working" notification.  It should wait for the "working again"
notification, and respond by polling all the files exactly once each.

I think a similar situation will arise for other kinds of event
generation.  So we don't actually need two separate notifications.
We only need one, which means: "err, sorry, it turns out things
weren't working, but I think they're OK now - and I'll let you know
again if not".

The result of my approach as I describe above will be an emacs which,
when overloaded, suspends file event handling in order to handle user
input, but which promptly catches up with deferred work when it can.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.  

Pronouns: they/he.  If I emailed you from @fyvzl.net or @evade.org.uk,
that is a private address which bypasses my fierce spamfilter.