all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Paul Eggert <eggert@cs.ucla.edu>
Cc: lekktu@gmail.com, 12450@debbugs.gnu.org
Subject: bug#12450: Remove configure's --without-sync-input option.
Date: Sat, 15 Sep 2012 14:03:22 +0300	[thread overview]
Message-ID: <837grvtuo5.fsf@gnu.org> (raw)
In-Reply-To: <5054551A.1070207@cs.ucla.edu>

> Date: Sat, 15 Sep 2012 03:14:50 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: bug-gnu-emacs@gnu.org, lekktu@gmail.com
> 
> On 09/15/2012 02:32 AM, Eli Zaretskii wrote:
> 
> > 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?  You don't just
remove "#ifdef SYNC_INPUT", you also make additional changes.  E.g.,
what is this about:

> -#ifdef REL_ALLOC
> -  malloc_hysteresis = 32;
> -#else
> -  malloc_hysteresis = 0;
> -#endif

?  If Emacs uses REL_ALLOC, won't it need this?  If this is related to
SYNC_INPUT, please explain how.

> > 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
> 
> It doesn't work well enough in my experience.  I often get crashes
> or near-crashes with Emacs.  It'll stop and ask me whether I want
> it to abort and dump core, say.  Or it'll just crash.

I never have such problems on MS-Windows (or anywhere else where I use
Emacs, including on GNU/Linux).  As MS-Windows uses the non-SYNC_INPUT
code, I don't see any evidence for that code being buggy, at least not
on MS-Windows.

> This sort of thing can be a real problem.  The problem is less
> common for me now than it was five years ago, and I credit
> SYNC_INPUT for some of that, but it's still too unreliable.

Or it could be some other, unrelated changes.  With the kind of high
rate of changes we see in Emacs, I don't think it's reasonable to
attribute changes in stability to a single change in the sources,
without a clear evidence.

> One way I can help improve things is to clean up signal handling,
> which is a huge pile of spaghetti at the low level -- it's so
> complicated that I expect that hardly anybody understands it.

If it works well, there's no need to understand it, is there?  OTOH,
whoever wants to make non-trivial changes in that code _must_
understand it very well.  And you just said you didn't.

> > Calling xmalloc was always safe to invoke from asynchronous entry into
> > the display engine, which happens, e.g., when mouse events are
> > processed.
> 
> That was the intent, yes, but in the non-SYNC_INPUT case it wasn't
> really safe.

Why not?  When is it not safe?

> 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?  You don't suggest that, if that assumption is false, we
prefer crashing to blocking input, do you?

> > 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.  For example:

  -#if defined SYNC_INPUT || defined USABLE_SIGIO
   static void
   handle_async_input (void)
   {
     interrupt_input_pending = 0;
  -#ifdef SYNC_INPUT
     pending_signals = pending_atimers;
  -#endif
  -/* Tell ns_read_socket() it is being called asynchronously so it can avoid
  -   doing anything dangerous.  */
  -#ifdef HAVE_NS            <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
  -  ++handling_signal;      <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
  -#endif                    <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
  +
     while (1)
       {
	 int nread;
  @@ -7199,13 +7178,8 @@
	 if (nread <= 0)
	  break;
       }
  -#ifdef HAVE_NS         <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
  -  --handling_signal;   <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
  -#endif                 <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
   }
  -#endif /* SYNC_INPUT || USABLE_SIGIO */

> > and what kinds of improvement will significantly
> > benefit from the proposed changes?
> 
> I've been trying to audit the signal handling of Emacs, to help close
> race conditions.  Doing this for the non-SYNC_INPUT case is so painful
> that I can't imagine anybody doing it.

Then don't do that.  Until we have problems that can reliably be
traced to that code, doing so would be a wasted effort, anyway.





  reply	other threads:[~2012-09-15 11:03 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-15  7:54 bug#12450: Remove configure's --without-sync-input option Paul Eggert
2012-09-15  9:32 ` Eli Zaretskii
2012-09-15 10:14   ` Paul Eggert
2012-09-15 11:03     ` Eli Zaretskii [this message]
2012-09-15 19:59       ` Paul Eggert
2012-09-15 20:15         ` Eli Zaretskii
2012-09-15 20:31           ` Paul Eggert
2012-09-16  6:33         ` Eli Zaretskii
2012-09-16  7:47           ` Paul Eggert
2012-09-16  8:05             ` Eli Zaretskii
2012-09-16  8:17               ` Paul Eggert
2012-09-16  8:21                 ` Eli Zaretskii
2012-09-16  8:24                 ` Eli Zaretskii
2012-09-16  8:34                   ` Paul Eggert
2012-09-16  8:53                     ` Eli Zaretskii
2012-09-15 21:12     ` Stefan Monnier
2012-09-16  5:55       ` Eli Zaretskii
2012-09-16 14:58         ` Stefan Monnier
2012-09-16 15:45           ` Eli Zaretskii
2012-09-16 16:30             ` Paul Eggert
2012-09-16 18:40               ` Eli Zaretskii
2012-09-16 19:55                 ` Jan Djärv
2012-09-16 18:37             ` Stefan Monnier
2012-09-16  9:33   ` Daniel Colascione
2012-09-16 10:43     ` Eli Zaretskii
2012-09-16 15:10       ` Stefan Monnier
2012-09-16 15:40         ` Eli Zaretskii
2012-09-15 22:18 ` Richard Stallman
2012-09-16  3:15   ` Paul Eggert
2012-09-16  6:10     ` Eli Zaretskii
2012-09-16  8:23       ` Paul Eggert
2012-09-16  8:32         ` Eli Zaretskii
2012-09-16 21:48           ` Paul Eggert
2012-09-17  7:42             ` Eli Zaretskii
2012-09-21 20:50               ` Paul Eggert
2012-09-22  9:03                 ` Eli Zaretskii
2012-09-22  9:34                   ` Paul Eggert
2012-09-22  9:50                     ` Eli Zaretskii
2012-09-22 10:01                       ` Paul Eggert
2012-09-16  9:52         ` Daniel Colascione
2012-09-16 10:44           ` Eli Zaretskii
2012-09-16 10:56             ` Daniel Colascione
2012-09-17  7:41               ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=837grvtuo5.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=12450@debbugs.gnu.org \
    --cc=eggert@cs.ucla.edu \
    --cc=lekktu@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.