Re: Asynchronous bug

[Stefan Monnier]

>     Isn't this going to cause the unwind-protect to be re-executed
>     if a signal is thrown while executing
>     (*specpdl_ptr->func) (specpdl_ptr->old_value);
>     now that specpdl_ptr is decremented afterwards ?
> 
> Yes, you're right.  How about this version?

That's what I've been using for a while and it seems to help in practice,
even though in theory it still has the same problem: specpdl_ptr
and specpdl_ptr->symbol are not aliased (and neither are they marked
as volatile) so the compiler can reorder accesses to those things and
generate the same code as it would have generated for something like:

        specpdl_ptr--;
        this_binding = *(specpdl_ptr+1);

where an async signal between the two statements could lead to an error.

Given the fact that there are already known problems with async
signals (they may access global structures in the middle of their
updating.  E.g. the mouse-face text-properties), the fact that
a previous bug took several months to Gerd and I to track down
(the signal handler was calling Fcons) and the fact that I get
`Xlib: unexpected async reply' messages every once in a while,
I think there's a clear need for the signal handler to be trimmed
down to the bare minimum.

I'm right now experimenting with the simple attached change
that moves all the signal handling code outside of the
signal handler (which now just sets a variable and returns).

Such a change would allow us to use BLOCK_INPUT less often,
to be less constrained in the operations we use in XTread_socket
and will generally give us more robust code.


        Stefan


--- lisp.h      1 May 2003 00:36:21 -0000       1.452
+++ lisp.h      2 May 2003 13:11:25 -0000
@@ -1768,13 +1773,18 @@
    This is a good thing to do around a loop that has no side effects
    and (in particular) cannot call arbitrary Lisp code.  */
 
+extern int async_input;
+extern void handle_async_input P_ ((void));
+
 #define QUIT                                           \
   do {                                                 \
     if (!NILP (Vquit_flag) && NILP (Vinhibit_quit))    \
       {                                                        \
        Vquit_flag = Qnil;                              \
        Fsignal (Qquit, Qnil);                          \
       }                                                        \
+    if (async_input)                                   \
+      handle_async_input();                            \
   } while (0)
 
 /* Nonzero if ought to quit now.  */

Index: keyboard.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/keyboard.c,v
retrieving revision 1.736
diff -u -r1.736 keyboard.c
--- keyboard.c  30 Apr 2003 21:04:13 -0000      1.736
+++ keyboard.c  2 May 2003 13:15:51 -0000
@@ -6689,15 +6719,14 @@
 #ifdef SIGIO   /* for entire page */
 /* Note SIGIO has been undef'd if FIONREAD is missing.  */
 
+int async_input;
+
 SIGTYPE
 input_available_signal (signo)
      int signo;
 {
   /* Must preserve main program's value of errno.  */
   int old_errno = errno;
-#ifdef BSD4_1
-  extern int select_alarmed;
-#endif
 
 #if defined (USG) && !defined (POSIX_SIGNALS)
   /* USG systems forget handlers when they are used;
@@ -6712,20 +6741,7 @@
   if (input_available_clear_time)
     EMACS_SET_SECS_USECS (*input_available_clear_time, 0, 0);
 
-  while (1)
-    {
-      int nread;
-      nread = read_avail_input (1);
-      /* -1 means it's not ok to read the input now.
-        UNBLOCK_INPUT will read it later; now, avoid infinite loop.
-        0 means there was no keyboard input available.  */
-      if (nread <= 0)
-       break;
-
-#ifdef BSD4_1
-      select_alarmed = 1;  /* Force the select emulator back to life */
-#endif
-    }
+  async_input = 1;
 
 #ifdef BSD4_1
   sigfree ();
@@ -6744,11 +6760,34 @@
 reinvoke_input_signal ()
 {
 #ifdef SIGIO
-  kill (getpid (), SIGIO);
+  handle_async_input ();
 #endif
 }
 
+void
+handle_async_input ()
+{
+#ifdef BSD4_1
+  extern int select_alarmed;
+#endif
 
+  async_input = 0;
+
+  while (1)
+    {
+      int nread;
+      nread = read_avail_input (1);
+      /* -1 means it's not ok to read the input now.
+        UNBLOCK_INPUT will read it later; now, avoid infinite loop.
+        0 means there was no keyboard input available.  */
+      if (nread <= 0)
+       break;
+
+#ifdef BSD4_1
+      select_alarmed = 1;  /* Force the select emulator back to life */
+#endif
+    }
+}
 
 static void menu_bar_item P_ ((Lisp_Object, Lisp_Object));
 static void menu_bar_one_keymap P_ ((Lisp_Object));

--- blockinput.h        4 Feb 2003 14:03:11 -0000       1.11
+++ blockinput.h        2 May 2003 13:13:11 -0000
@@ -84,7 +84,7 @@
       --interrupt_input_blocked;               \
       if (interrupt_input_blocked == 0)                \
        {                                       \
-         if (interrupt_input_pending)          \
+         if (interrupt_input_pending || async_input)   \
            reinvoke_input_signal ();           \
          if (pending_atimers)                  \
            do_pending_atimers ();              \

--- process.c   9 Mar 2003 22:48:08 -0000       1.403
+++ process.c   2 May 2003 13:10:26 -0000
@@ -4021,7 +4019,8 @@
                         (check_connect ? &Connecting : (SELECT_TYPE *)0),
                         (SELECT_TYPE *)0, &timeout);
        }
-
+      if (async_input)
+       handle_async_input ();
       xerrno = errno;
 
       /* Make C-g and alarm signals set flags again */