bug#12450: Remove configure's --without-sync-input option.

[Paul Eggert]

On 09/15/2012 04:03 AM, Eli Zaretskii wrote:

>> But [BLOCK_INPUT around malloc calls] shouldn't be needed on
>> non-Windows platforms.
> 
> Are we absolutely sure?  Can we prove that malloc is never called by
> async code?

Yes.  We know where our signal handlers are, and we can audit them
to make sure they can't call non-async-signal-safe code.  And
that's the way Emacs has been running for years, on GNU and
POSIXish platforms: it has not been putting BLOCK_INPUT around
malloc calls.  On these platforms this patch is nothing new --
it's simply ratifying standard practice.  I'm not saying that the
SYNC_INPUT code is 100% bug-free -- it's not, and that's why I'm
auditing it -- but it's much, much better than the non-SYNC_INPUT
code is.

Out of curiosity, what happens in MS-Windows if you take the
current trunk and change nt/config.nt's "#undef SYNC_INPUT" to
"#define SYNC_INPUT 1"?  If that works, it would help simplify the
code overall.

>>> If someone can describe in detail what SYNC_INPUT means
>>
>> Sorry, as far as I know, the only detailed description is the source
>> code itself.  Perhaps Stefan wrote up something sometime....
> 
> Then how do we know that the changes are correct?

First, we're using SYNC_INPUT already.  All non-Windows platform use
SYNC_INPUT, and have done so since 2008.

Second, we know that it's not safe to invoke random system
functions from signal handlers.  Emacs used to get away with this,
sort of, but the situation has gotten worse (i.e., flakier) with
time, partly due to the issue of multithreading.  So there are
sound software engineering reasons to go with SYNC_INPUT.  See,
for example, the brief discussion in <http://bugs.gnu.org/11080#16>.

> When is it not safe?

There is a small set of standard functions that signal handlers
can safely call.  Calling functions outside this list is unsafe
because it leads to race conditions.  Usually the unsafe functions
work, but sometimes they don't, and bugs are hard to track down
and fix.

The exact list of safe functions depends on the platform.
For OpenBSD, for example, there's a list in
<http://www.openbsd.org/cgi-bin/man.cgi?query=signal>.
POSIX has a standard list in
<http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_04_03_03>.

Code that limits itself to the POSIX list should be safe in
practice.  Emacs probably needs to go beyond the POSIX list, on a
platform-by-platform basis, but it needs to take some care when
doing so.

> what is this about:
> 
>> -#ifdef REL_ALLOC
>> -  malloc_hysteresis = 32;
>> -#else
>> -  malloc_hysteresis = 0;
>> -#endif

malloc_hysteresis is used only by the non-SYNC_INPUT code.
Removing the non-SYNC_INPUT code lets us remove cruft that is
needed only because of the non-SYNC_INPUT code.

> I never have such problems on MS-Windows

I don't use MS-Windows.  Apparently our experiences on GNU/Linux
differ.  I expect that I am a heavier use of that platform and so
am more likely to run into Emacs bugs there.

I'm not the only one to think that the non-SYNC_INPUT code is
questionable -- it's been a longstanding issue.  Since the
MS-Windows port does not exercise the interrupt-based input
handling defined by the non-SYNC_INPUT code, that could well
explain why you're not observing problems with MS-Windows.  And
this would also suggest that the MS-Windows code does not care
whether SYNC_INPUT is used, so it can also profitably change to
use SYNC_INPUT.

> With the kind of high rate of changes we see in Emacs,

I've been seeing problems for years.  It's not due to any recent changes.

> If it works well,

The non-SYNC_INPUT code doesn't work well at all.  The SYNC_INPUT
code is better, but it can still use an audit, which I'm doing.

> whoever wants to make non-trivial changes in that code _must_
> understand it very well.

I understand the non-Microsoft side well, and from that point of
view this change is a good one, because it removes unused and
confusing code that complicates the job of Emacs maintenance.  I
do not understand the Microsoft side, but that should be a
separate issue.

>>> it looks like the NS port also uses the !SYNC_INPUT code
>>
>> Sorry, I don't see why.
> 
> Simply because there's HAVE_NS code in there.

Yes, and there's non-HAVE_NS code in there too.  But that's not
relevant.  The point is that SYNC_INPUT is always 1 for the NextStep
port, just as it's always 1 for all ports other than MS-Windows.