Subject: Re: Remove configure's --without-sync-input option.

From: Eli Zaretskii <[email-masked]>
Date: Sat, 15 Sep 2012 12:32:15 +0300

> Date: Sat, 15 Sep 2012 00:54:49 -0700
> From: Paul Eggert <[email-masked]>
> CC: Eli Zaretskii <[email-masked]>, Juanma Barranquero <[email-masked]>
> 
> When auditing signal-handling in preparation for cleaning it up, I
> found that configuring --without-sync-input has race conditions that
> look like they would be a real pain to fix.  Many of these involve
> memory allocation within a signal handler, which is tricky and
> dangerous.  Emacs attempts to work around the problems, but these
> workarounds are not sufficient and fixing them would not be easy.
> [...]
>  void *
>  xmalloc (size_t size)
>  {
> -  void *val;
> -
> -  MALLOC_BLOCK_INPUT;
> -  val = malloc (size);
> -  MALLOC_UNBLOCK_INPUT;
> -
> +  void *val = malloc (size);
>    if (!val && size)
>      memory_full (size);
>    return val;

Calling xmalloc was always safe to invoke from asynchronous entry into
the display engine, which happens, e.g., when mouse events are
processed.  Your patch removes that line of defense, which makes me
very nervous.  At the very least, let's have BLOCK_INPUT around the
call to malloc (and similarly in the other related functions).

> I see that nt/config.nt has "#undef SYNC_INPUT" but do the Microsoft
> ports care one way or another whether SYNC_INPUT is set?  If so, why?

I cannot answer that question, because the full semantics of
SYNC_INPUT is unclear to me.  All I can say now is that input events
on MS-Windows come from a separate thread that runs asynchronously to
the main thread (the latter runs the Lisp interpreter and all the
other parts of Emacs).  So, at least up front, Emacs on MS-Windows
does use non-SYNC_INPUT input method, if SYNC_INPUT is to be
understood literally.

OTOH, it could be that no one bothered or dared to define SYNC_INPUT
on MS-Windows when that method became the default on Posix platforms.
(The fact that config.nt undefines SYNC_INPUT is not indicative of
anything except that autogen/config.in does the same; the fact that
ms-w32.h does _not_ define SYNC_INPUT is the important detail.)

If someone can describe in detail what SYNC_INPUT means, what it
assumes of the target platform, and how is that reflected in the logic
of the related code, then I could try investigating the meaning of it
for the MS-Windows build.  As we no longer have on board people who
really understand the Emacs event handling on MS-Windows, such an
investigation will take a lot of time and effort, diverting my scarce
time from general-purpose Emacs development (such as menus on TTYs and
the remaining bits of bidi), and will necessarily be error-prone.  So
I'd really like to be sure there are very good reasons for removing
the non-SYNC_INPUT code.  It could be that a better alternative is to
leave that code alone, if only by replacing the conditionals with
WINDOWSNT or some such.

Btw, it looks like the NS port also uses the !SYNC_INPUT code; your
changes seem to remove that code without any substitute.

Btw2, your changes remove code conditioned on

  #if defined SYNC_INPUT || defined USABLE_SIGIO

but do not remove code conditioned on USABLE_SIGIO alone.  Is that
TRT?

> Since it's an undocumented and deprecated configure-time option, and
> has been that way for over four years, and the non-SYNC_INPUT code is
> a real mess that is getting in the way of improving Emacs, now seems
> like a good time to remove that code.

In what ways does the current SYNC_INPUT code get in the way of
improving Emacs, and what kinds of improvement will significantly
benefit from the proposed changes?  I was under the impression that
the low-level code to which SYNC_INPUT is relevant doesn't need any
improvements, because it works well enough for us to leave it alone
and concentrate our energy on adding new user-level features.

Really, the latest push on non-trivial low-level changes seems to me
like a drain on our resources, with little real gains, except perhaps
on some obscure platforms I never heard about or in even more obscure
use-cases.  The only real effect of this is a significant
destabilization of the development codebase, which people already
starting to complain about.  I could understand such loss of stability
when the goal is a significant improvement in some important feature,
like GC or the display engine.  But having that for the sake of some
mythical "ease of improving Emacs" sounds like a net loss to me.  It
reminds me of the sorry state of roads in my country, which are
permanently in a state of being "maintained for future improvements",
causing closure of some of the lanes and generally making the traffic
more jammed than it needs to be.

In general, Emacs has long ago reached the state where low-level code
is stable and should be left alone, with most of the maintenance
effort directed towards adding user-level features.  Low-level code
changes should be considered only when there are very good reasons for
them.  Am I the only one who thinks so?