read-event in batch mode

read-event in batch mode

[Eli Zaretskii]

Currently, read-event waits forever when called in batch mode.  This
is because of the following fragment from kbd_buffer_get_event:

  #ifndef HAVE_DBUS  /* We want to read D-Bus events in batch mode.  */
    if (noninteractive
	/* In case we are running as a daemon, only do this before
	   detaching from the terminal.  */
	|| (IS_DAEMON && daemon_pipe[1] >= 0))
      {
	int c = getchar ();
	XSETINT (obj, c);
	*kbp = current_kboard;
	return obj;
      }
  #endif	/* ! HAVE_DBUS  */

The call to getchar will wait indefinitely, until something is typed
on the keyboard.

Worse, if Emacs is built with D-Bus, this fragment is bypassed
(including if Emacs is run in daemon mode!), and then read-event does
honor the timeout.  The net effect is that writing code that waits for
events and works on all platforms is tricky at best.

This was discovered while debugging the problems of
file-notify-tests.el, see bug #16519 for more details and context.

Does any port for a supported platform need this fragment, or can we
safely delete it?  What about the daemon -- does it need this "when
detaching from the terminal", and if so, why?

In any case, I think we need to document this deviant behavior, at
least for now, because there's no hint of it anywhere in the docs.  Of
course, sit-for is also affected by that.

Comments?

Re: read-event in batch mode

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

> In any case, I think we need to document this deviant behavior, at
> least for now, because there's no hint of it anywhere in the docs.  Of
> course, sit-for is also affected by that.
>
> Comments?

For 24.4, we shouldn't change it. Documentation might be sufficient (but
we must say, that the behaviour is undefined, depending on the
configuration options).

In general, I believe that events arriving from D-Bus or file change
notifications and alike, shouldn't go via the keyboard buffer. Such
events shall have their own handling in the main loop.

Best regards, Michael.

Re: read-event in batch mode

[Stefan Monnier]

> Does any port for a supported platform need this fragment, or can we
> safely delete it?  What about the daemon -- does it need this "when
> detaching from the terminal", and if so, why?

I suspect this code was written so that "emacs --batch" can read input
from stdin.  If the other code works for that, than we can presumably
get rid of this code.

The "|| (IS_DAEMON && daemon_pipe[1] >= 0" part looks to me like "let's
make sure the daemon mode works like --batch until we fork&detach".
IOW if it's not needed for --batch, then it's not needed for the daemon
case either.


        Stefan

Re: read-event in batch mode

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: emacs-devel@gnu.org
> Date: Fri, 31 Jan 2014 14:43:43 +0100
> 
> In general, I believe that events arriving from D-Bus or file change
> notifications and alike, shouldn't go via the keyboard buffer. Such
> events shall have their own handling in the main loop.

The "keyboard buffer" is a misnomer: that's actually the Emacs event
queue, where all kinds of events end up, and from which they are read
and processed.  Why is it a good idea to have more than one event
queue?

Re: read-event in batch mode

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Fri, 31 Jan 2014 09:09:14 -0500
> 
> > Does any port for a supported platform need this fragment, or can we
> > safely delete it?  What about the daemon -- does it need this "when
> > detaching from the terminal", and if so, why?
> 
> I suspect this code was written so that "emacs --batch" can read input
> from stdin.  If the other code works for that, than we can presumably
> get rid of this code.

Well, the systems that support D-Bus certainly don't need it, as they
haven't been using that code since last May.  Windows doesn't need
that, either.

As for others, I hope their users will speak up.  If nothing else
turns up, I suggest to ifdef away this code after the release branch
is cut, and see if someone hollers.

> The "|| (IS_DAEMON && daemon_pipe[1] >= 0" part looks to me like "let's
> make sure the daemon mode works like --batch until we fork&detach".
> IOW if it's not needed for --batch, then it's not needed for the daemon
> case either.

That was my guess as well.

Thanks.

Re: read-event in batch mode

[Stefan Monnier]

> The "keyboard buffer" is a misnomer: that's actually the Emacs event
> queue, where all kinds of events end up, and from which they are read
> and processed.  Why is it a good idea to have more than one event
> queue?

Because some events should be processed in a particular order and others
in a different order.  They correspond to different "threads" of
execution.  The "keyboard buffer" normally corresponds to events coming
from the user, so their relative ordering is very important and should
not be changed.

But D-Bus events or file-notification events may be triggered by
processing that's completely independent/asynchronous from the user's
actions, so they should be processed by Emacs without having to wait for
previous user-events to be processed.

The typical use case: user types a command which involves communication
over D-Bus, then hit key "b", then the first command starts, sends
some D-Bus message, waits for the answer: if the answer comes after key
"b", we're in a "deadlock" where "b" won't be processed before the
command is completed, but the command won't complete before the D-Bus
answer is received and processed.


        Stefan

Re: read-event in batch mode

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Michael Albinus <michael.albinus@gmx.de>,  emacs-devel@gnu.org
> Date: Fri, 31 Jan 2014 10:34:23 -0500
> 
> > The "keyboard buffer" is a misnomer: that's actually the Emacs event
> > queue, where all kinds of events end up, and from which they are read
> > and processed.  Why is it a good idea to have more than one event
> > queue?
> 
> Because some events should be processed in a particular order and others
> in a different order.  They correspond to different "threads" of
> execution.  The "keyboard buffer" normally corresponds to events coming
> from the user, so their relative ordering is very important and should
> not be changed.
> 
> But D-Bus events or file-notification events may be triggered by
> processing that's completely independent/asynchronous from the user's
> actions, so they should be processed by Emacs without having to wait for
> previous user-events to be processed.

We already have process_special_events that does what you describe,
just add more events to what it handles.

Re: read-event in batch mode

[Nix]

On 31 Jan 2014, Eli Zaretskii said:

>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: emacs-devel@gnu.org
>> Date: Fri, 31 Jan 2014 09:09:14 -0500
>> 
>> > Does any port for a supported platform need this fragment, or can we
>> > safely delete it?  What about the daemon -- does it need this "when
>> > detaching from the terminal", and if so, why?
>> 
>> I suspect this code was written so that "emacs --batch" can read input
>> from stdin.  If the other code works for that, than we can presumably
>> get rid of this code.
>
> Well, the systems that support D-Bus certainly don't need it, as they
> haven't been using that code since last May.  Windows doesn't need
> that, either.

That would be around the time when emacs --daemon started "acting up" by
going into an unresponsive CPU-chewing loop whenever it asked a question
on the console during startup. I have to start Emacs these days by
starting a non-daemon Emacs, answering various questions (mostly from
desktop.el), quitting (and saving a fixed desktop file), then restarting
in daemon mode...

-- 
NULL && (void)

Re: read-event in batch mode

[Eli Zaretskii]

> From: Nix <nix@esperi.org.uk>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, emacs-devel@gnu.org
> Emacs: the prosecution rests its case.
> Date: Sun, 02 Feb 2014 12:11:28 +0000
> 
> > Well, the systems that support D-Bus certainly don't need it, as they
> > haven't been using that code since last May.  Windows doesn't need
> > that, either.
> 
> That would be around the time when emacs --daemon started "acting up" by
> going into an unresponsive CPU-chewing loop whenever it asked a question
> on the console during startup. I have to start Emacs these days by
> starting a non-daemon Emacs, answering various questions (mostly from
> desktop.el), quitting (and saving a fixed desktop file), then restarting
> in daemon mode...

Did you submit a bug report about that?  If so, please show the bug
number.  If not, please do report this, and thanks.

Re: read-event in batch mode

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Nix <nix@esperi.org.uk>
>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, emacs-devel@gnu.org
>> Emacs: the prosecution rests its case.
>> Date: Sun, 02 Feb 2014 12:11:28 +0000
>> 
>> > Well, the systems that support D-Bus certainly don't need it, as they
>> > haven't been using that code since last May.  Windows doesn't need
>> > that, either.
>> 
>> That would be around the time when emacs --daemon started "acting up" by
>> going into an unresponsive CPU-chewing loop whenever it asked a question
>> on the console during startup. I have to start Emacs these days by
>> starting a non-daemon Emacs, answering various questions (mostly from
>> desktop.el), quitting (and saving a fixed desktop file), then restarting
>> in daemon mode...
>
> Did you submit a bug report about that?  If so, please show the bug
> number.  If not, please do report this, and thanks.

Likely, "since last May" means May 2012:

2012-05-14  Michael Albinus  <michael.albinus@gmx.de>

	* keyboard.c (kbd_buffer_get_event): Read special events also in
	batch mode.  (Bug#11415)

Do we still see the same coincidence, that daemon goes into infloop?
Since May 2012?

Best regards, Michael.

Re: read-event in batch mode

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

>> > The "keyboard buffer" is a misnomer: that's actually the Emacs event
>> > queue, where all kinds of events end up, and from which they are read
>> > and processed.  Why is it a good idea to have more than one event
>> > queue?
>> 
>> Because some events should be processed in a particular order and others
>> in a different order.  They correspond to different "threads" of
>> execution.  The "keyboard buffer" normally corresponds to events coming
>> from the user, so their relative ordering is very important and should
>> not be changed.
>> 
>> But D-Bus events or file-notification events may be triggered by
>> processing that's completely independent/asynchronous from the user's
>> actions, so they should be processed by Emacs without having to wait for
>> previous user-events to be processed.
>
> We already have process_special_events that does what you describe,
> just add more events to what it handles.

Yes, that should work. But we would need more rearrangement, because
process_special_events seems to be called only after
kbd_buffer_get_event has returned already.

During feature freeze, we shouldn't rearrange that code. For the time
being I have extended the test in kbd_buffer_get_event, in order to read
special events also when not linked with D-Bus. This should solve the
problem also for the w32 case, I believe.

Best regards, Michael.

Re: read-event in batch mode

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  emacs-devel@gnu.org
> Date: Tue, 04 Feb 2014 12:54:08 +0100
> 
> For the time being I have extended the test in kbd_buffer_get_event,
> in order to read special events also when not linked with
> D-Bus. This should solve the problem also for the w32 case, I
> believe.

It did: the file-notify tests now all pass on w32.

Thanks.

Re: read-event in batch mode

[Stefan Monnier]

> We already have process_special_events that does what you describe,
> just add more events to what it handles.

Sure.  It's still 2 queues, conceptually, just implemented by
multiplexing them onto a single one.  Not sure why it's an advantage.


        Stefan