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

[Michael Albinus]

Ian Jackson <ijackson@chiark.greenend.org.uk> writes:

Hi Ian,

>> Yes. But if we divide the keyboard buffer (also good for mouse events
>> and other events which do not harm) from the D-Bus and file notification
>> events, such a check is needed at fewer places.
>
> I don't follow.
>
> Any buffer that we have can fill up.  So each thing that generates
> events needs to be able to handle the buffer (that it is writing to)
> becomeing full; and to be able to handle the buffer becoming
> no-longer-full, so that it can restart.
>
> So each event-generator has a "stop" hook and a "start" hook, each of
> which has one call site, in the code that handles the buffer for that
> event generator.  The number of these calls is the same either way.
>
> The difference is just whether there are two buffers, which call
> disjoint sets of start/stop hooks, or one buffer, which calls all the
> start/stop hooks.

My proposal was to keep the keyboard buffer as-it-is for key strokes and
alike. No need to change anything. A new buffer for D-Bus and file
notification events would need an additional handling for a burst of
events.

But as Eli has replied, this brings further problems. So I don't follow
any longer this approach.

>> The advantage of splitting into "keyboard" and "other things" buffers
>> would be, that the keyboard buffer doesn't overrun, whatever burst of
>> D-Bus or file notification events arrives.
>
> The two sets of code would seem very similar.  It would probably be
> sensible to actually use the very same code and data formats and
> everything.  The advantage is precisely that we can prioritise them
> differently: the keyboard buffer could keep working, even while the
> other buffer is full.  In practice I don't think this is likely to be
> particularly important since we don't usually expect emacs to be
> overloaded in this way.  And when it is, there is an argument that
> processing events in order is likely to be better in the sense that it
> is less confusing to the user.

Again, two different buffers are out of discussion ...

>> Yes. But I believe we don't need to handle lost events. If we have a
>> mean to inform the upper libraries, filenotify.el and dbus.el, about
>> this case, it would be sufficient. A simple check whether the event
>> buffer is full.
>
> 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).

>> I don't believe that the native backends, like inotify.c, deserve too
>> much intelligence. They shall do stupid event receiving and reporting,
>> with a callback invoked in case of problems. This callback must be
>> clever then.
>
> If the buffer overflowed, it is quite easy to discard now-superfluous
> events at the lower layer (thus coalescing events into the single
> "some may have been lost" callback).  At higher levels the state
> machine needed to do this becomes more complicated.

Agreed.

> I suggest introducing a version of kbd_buffer_store_event which is to
> be called by anyone that doesn't do flow control properly.  It would
> stop putting things in the buffer when the buffer is (say) 3/4 full.

That's what I have answered to Eli, so we're in agreement :-)

> 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.

> 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.

> An alternative would be to turn off inotify entirely and just switch
> back to polling.  But we wouldn't want to do that permanently; when
> things have calmed down we want emacs to back to quiescently waiting
> for file events.

Exactly.

> 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.

> Ian.

Best regards, Michael.