GNU bug report logs - #23980
25.0.95; Events put in `unread-command-events' can be wrongly handled

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 23980 in the body.
You can then email your comments to 23980 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Chris Feng <chris.w.feng <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.0.95; Events put in `unread-command-events' can be wrongly handled
Date: Thu, 14 Jul 2016 22:03:34 +0800

I encountered a bug in the command loop that when I put an event of the
form (t . EVENT) into `unread-command-events', there is a chance the
event won't get recognized with the message "<t> is undefined".
I did a bit research and found that in keyboard.c `read_char' calls
`read_decoded_event_from_main_queue' (which in turn calls
`read_event_from_main_queue' and `kbd_buffer_get_event') but does not
check if the returned event is from `Vunread_command_events'.
The problem in `read_char' can be demonstrated by the following form:

  (progn
    (run-with-timer 1 nil (lambda () (push '(t . 1) unread-command-events)))
    (read-event nil nil 2))

    => '(t . 1)

There is actually another related issue, that for an event returned by
`read_decoded_event_from_main_queue' other than the form (t . EVENT), we
can't tell whether it's from `Vunread_command_events' or not.
If it is, then we should not put it into `this_command_keys'.

Any ideas on how to fix these?

Message #10 received at 23980-done <at> debbugs.gnu.org (full text, mbox):

From: Chris Feng <chris.w.feng <at> gmail.com>
To: 23980-done <at> debbugs.gnu.org
Subject: Re: bug#23980: 25.0.95; Events put in `unread-command-events' can be
 wrongly handled
Date: Sun, 19 Nov 2017 17:39:28 +0800

[Message part 1 (text/plain, inline)]

The bug was fixed by 1f3f4b1296.

[Message part 2 (text/html, inline)]

Message #17 received at 23980 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: 23980 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#23980: 25.0.95; Events put in `unread-command-events' can be
 wrongly handled
Date: Tue, 20 Feb 2018 17:35:15 +0200

[Message part 1 (text/plain, inline)]

On 11/19/17 11:39 AM, Chris Feng wrote:
> The bug was fixed by 1f3f4b1296.

Eli, any change to have this backported to emacs-26?

This reason I'm asking is, this fixed a rather nasty bug resulting from 
the interaction between the newest company-mode (0.9.5) and 
flyspell-mode, both of which use sit-for. This particular combination 
results in a timer being run inside a sit-for, which also calls sit-for, 
which in emacs-26 ends up putting e.g. (t t . 45) into 
unread-command-events.

Here's how to reproduce:

- Visit the attached file.

- M-x eval-buffer.

- Start typing.

- See "<t> is undefined" in the minibuffer for every other keypress.

Originally reported here: 
https://github.com/company-mode/company-mode/issues/760

[sit-for-bug.el (text/x-emacs-lisp, attachment)]

Message #20 received at 23980 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>, Chris Feng <chris.w.feng <at> gmail.com>
Cc: 23980 <at> debbugs.gnu.org
Subject: Re: bug#23980: 25.0.95; Events put in `unread-command-events' can be
 wrongly handled
Date: Tue, 20 Feb 2018 20:37:54 +0200

> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Tue, 20 Feb 2018 17:35:15 +0200
> 
> On 11/19/17 11:39 AM, Chris Feng wrote:
> > The bug was fixed by 1f3f4b1296.
> 
> Eli, any change to have this backported to emacs-26?

I'd need someone (Chris?) to tell which situations cause (t SOMETHING)
be put into unread-command-events.  If these situations are rare
enough, maybe we can risk it.

Message #23 received at 23980 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>, Chris Feng <chris.w.feng <at> gmail.com>
Cc: 23980 <at> debbugs.gnu.org
Subject: Re: bug#23980: 25.0.95; Events put in `unread-command-events' can be
 wrongly handled
Date: Tue, 20 Feb 2018 23:16:47 +0200

On 2/20/18 8:37 PM, Eli Zaretskii wrote:

> I'd need someone (Chris?) to tell which situations cause (t SOMETHING)
> be put into unread-command-events.  If these situations are rare
> enough, maybe we can risk it.

At least one of the places that do that is sit-for. See the long comment 
at the end of its definition (I don't really understand its last 
sentence, FWIW).

Message #26 received at 23980 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 23980 <at> debbugs.gnu.org, chris.w.feng <at> gmail.com
Subject: Re: bug#23980: 25.0.95; Events put in `unread-command-events' can be
 wrongly handled
Date: Wed, 21 Feb 2018 06:22:55 +0200

> Cc: 23980 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Tue, 20 Feb 2018 23:16:47 +0200
> 
> On 2/20/18 8:37 PM, Eli Zaretskii wrote:
> 
> > I'd need someone (Chris?) to tell which situations cause (t SOMETHING)
> > be put into unread-command-events.  If these situations are rare
> > enough, maybe we can risk it.
> 
> At least one of the places that do that is sit-for. See the long comment 
> at the end of its definition (I don't really understand its last 
> sentence, FWIW).

But then how come you have (t t . 45) there?

Message #29 received at 23980 <at> debbugs.gnu.org (full text, mbox):

From: Chris Feng <chris.w.feng <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 23980 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#23980: 25.0.95;
 Events put in `unread-command-events' can be wrongly handled
Date: Wed, 21 Feb 2018 15:40:35 +0800

Eli Zaretskii <eliz <at> gnu.org> writes:

>> On 2/20/18 8:37 PM, Eli Zaretskii wrote:
>>
>> > I'd need someone (Chris?) to tell which situations cause (t SOMETHING)
>> > be put into unread-command-events.  If these situations are rare
>> > enough, maybe we can risk it.
>>
>> At least one of the places that do that is sit-for. See the long comment
>> at the end of its definition (I don't really understand its last
>> sentence, FWIW).

It seems to be the only use case in Emacs's source tree, but we can't
tell how many packages do this trick in the wild nor can we predict
that.

> But then how come you have (t t . 45) there?

Probably shortly after (t . 45) is unread another call to `sit-for'
happens to read it out directly from `unread-command-events', and the
event is further unread as (cons t '(t . 45)).  Perhaps we may even get
something like (t t t . 45) if lucky enough.

PS. Many users will stick with the pre-26 releases in the next upcoming
years so backporting this fix does not necessarily resolve the problem
for company-mode.  At least a workaround has to be come up with to deal
with the situations where this fix is absent.

Message #32 received at 23980 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Chris Feng <chris.w.feng <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 23980 <at> debbugs.gnu.org
Subject: Re: bug#23980: 25.0.95; Events put in `unread-command-events' can be
 wrongly handled
Date: Wed, 21 Feb 2018 12:47:20 +0200

On 2/21/18 9:40 AM, Chris Feng wrote:

> It seems to be the only use case in Emacs's source tree, but we can't
> tell how many packages do this trick in the wild nor can we predict
> that.

They only do that with one goal, though (so we are dealing with a protocol).

>> But then how come you have (t t . 45) there?
> 
> Probably shortly after (t . 45) is unread another call to `sit-for'
> happens to read it out directly from `unread-command-events', and the
> event is further unread as (cons t '(t . 45)).

More or less, this. Except we don't get this effect from two (or more) 
consecutive calls to sit-for.

What triggers it, is effectively recursive sit-for calls, when the 
second sit-for is called inside a timer, which are allowed to run inside 
the first sit-for.

> PS. Many users will stick with the pre-26 releases in the next upcoming
> years so backporting this fix does not necessarily resolve the problem
> for company-mode.  At least a workaround has to be come up with to deal
> with the situations where this fix is absent.

I've been trying to. So far, the options are:

- Advise users to lower flyspell-delay to (* 0.9 company-idle-delay). 
That actually leads to freezes, which I should file as a separate bug 
report.

- Ask people to disable flyspell-mode :)

- Use sleep-for instead of sit-for in the relevant place, forgoing the 
significant latency improvement that comes with the latter. Maybe 
predicated on Emacs version, and flyspell-mode being enabled.

So far the last option is winning, but I'd be much more content to use 
it if the Emacs 26 are not going to need it.

Message #35 received at 23980 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Chris Feng <chris.w.feng <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 23980 <at> debbugs.gnu.org
Subject: Re: bug#23980: 25.0.95; Events put in `unread-command-events' can be
 wrongly handled
Date: Wed, 21 Feb 2018 14:23:35 +0200

On 2/21/18 12:47 PM, Dmitry Gutov wrote:

> - Use sleep-for instead of sit-for in the relevant place, forgoing the 
> significant latency improvement that comes with the latter<...>

Or maybe significant-ish. To the tune of 30ms.

Message #38 received at 23980 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 23980 <at> debbugs.gnu.org, chris.w.feng <at> gmail.com
Subject: Re: bug#23980: 25.0.95; Events put in `unread-command-events' can be
 wrongly handled
Date: Wed, 21 Feb 2018 19:48:56 +0200

> Cc: 23980 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Wed, 21 Feb 2018 12:47:20 +0200
> 
> So far the last option is winning, but I'd be much more content to use 
> it if the Emacs 26 are not going to need it.

I'm _really_ uneasy about putting this on the release branch.  sit-for
is notorious for subtle problems that take a lot of time to be
exposed.

Message #41 received at 23980 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 23980 <at> debbugs.gnu.org, chris.w.feng <at> gmail.com
Subject: Re: bug#23980: 25.0.95; Events put in `unread-command-events' can be
 wrongly handled
Date: Wed, 21 Feb 2018 23:45:30 +0200

On 2/21/18 7:48 PM, Eli Zaretskii wrote:
>> Cc: 23980 <at> debbugs.gnu.org
>> From: Dmitry Gutov <dgutov <at> yandex.ru>
>> Date: Wed, 21 Feb 2018 12:47:20 +0200
>>
>> So far the last option is winning, but I'd be much more content to use
>> it if the Emacs 26 are not going to need it.
> 
> I'm _really_ uneasy about putting this on the release branch.  sit-for
> is notorious for subtle problems that take a lot of time to be
> exposed.

Well, ok. I can live with that.

Maybe consider it for inclusion if we end up having more pretests? This 
patch has been in master for a few months, and it's two lines long, so 
it must be *somewhat* safe.

I've pushed the workaround (last option) to company-mode in the meantime.

Message #44 received at 23980 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 23980 <at> debbugs.gnu.org, chris.w.feng <at> gmail.com
Subject: Re: bug#23980: 25.0.95; Events put in `unread-command-events' can be
 wrongly handled
Date: Thu, 22 Feb 2018 09:39:37 +0200

> Cc: chris.w.feng <at> gmail.com, 23980 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Wed, 21 Feb 2018 23:45:30 +0200
> 
> > I'm _really_ uneasy about putting this on the release branch.  sit-for
> > is notorious for subtle problems that take a lot of time to be
> > exposed.
> 
> Well, ok. I can live with that.
> 
> Maybe consider it for inclusion if we end up having more pretests? This 
> patch has been in master for a few months, and it's two lines long, so 
> it must be *somewhat* safe.

Yes, a good idea.

Message #51 received at 23980 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 23980 <at> debbugs.gnu.org, chris.w.feng <at> gmail.com
Subject: Re: bug#23980: 25.0.95; Events put in `unread-command-events' can be
 wrongly handled
Date: Wed, 19 Dec 2018 15:54:31 +0200

Hi Eli,

On 22.02.2018 9:39, Eli Zaretskii wrote:

>> Maybe consider it for inclusion if we end up having more pretests? This
>> patch has been in master for a few months, and it's two lines long, so
>> it must be *somewhat* safe.
> 
> Yes, a good idea.

Any chance we can backport it now? I have hard-to-reproduce bug report 
from users of third-party code (company-mode plus elpy) which, I am 
told, is definitely fixed by applying this patch to emacs-26:

https://github.com/jorgenschaefer/elpy/issues/1396#issuecomment-448142207

Message #54 received at 23980 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 23980 <at> debbugs.gnu.org, chris.w.feng <at> gmail.com
Subject: Re: bug#23980: 25.0.95; Events put in `unread-command-events' can be
 wrongly handled
Date: Wed, 19 Dec 2018 17:27:14 +0200

> Cc: 23980 <at> debbugs.gnu.org, chris.w.feng <at> gmail.com
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Wed, 19 Dec 2018 15:54:31 +0200
> 
> On 22.02.2018 9:39, Eli Zaretskii wrote:
> 
> >> Maybe consider it for inclusion if we end up having more pretests? This
> >> patch has been in master for a few months, and it's two lines long, so
> >> it must be *somewhat* safe.
> > 
> > Yes, a good idea.
> 
> Any chance we can backport it now? I have hard-to-reproduce bug report 
> from users of third-party code (company-mode plus elpy) which, I am 
> told, is definitely fixed by applying this patch to emacs-26:
> 
> https://github.com/jorgenschaefer/elpy/issues/1396#issuecomment-448142207

Risky... but okay, let's do it.

Thanks.

Message #57 received at 23980 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 23980 <at> debbugs.gnu.org, chris.w.feng <at> gmail.com
Subject: Re: bug#23980: 25.0.95; Events put in `unread-command-events' can be
 wrongly handled
Date: Wed, 19 Dec 2018 18:07:50 +0200

On 19.12.2018 17:27, Eli Zaretskii wrote:

>> Any chance we can backport it now? I have hard-to-reproduce bug report
>> from users of third-party code (company-mode plus elpy) which, I am
>> told, is definitely fixed by applying this patch to emacs-26:
>>
>> https://github.com/jorgenschaefer/elpy/issues/1396#issuecomment-448142207
> 
> Risky... but okay, let's do it.
Thank you, done!

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.