Re: SIGIO and the NS port

[Alan Third <alan@idiocy.org>]

[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]

On Mon, Mar 13, 2017 at 04:38:17PM +0000, Alan Third wrote:
> On Fri, Mar 10, 2017 at 12:41:17PM +0000, Alan Third wrote:
> > The NS port doesn’t use SIGIO to signal for input (I think it polls
> > using ns_select), and if I comment these two defines out I can remove
> > block_input/unblock_input from various places with no ill effects.
> 
> It looks like this isn’t right. The NS port works without SIGIO most
> of the time, but if wait_reading_process_output is called with
> read_kbd=0 then it goes into an infinite loop. (At least, I think
> that’s what’s happening.)

So I’ve been using the attached patch for a few weeks now with no
issues.

I’ve stopped the signal handler from responding to SIGIO, and the two
places where the NS GUI used to raise SIGIO have been replaced with
direct calls to the SIGIO handler code.

I’m not sure if this is a sensible approach. It fixes at least one
crash, and we could remove some HAVE_NS‐only code, so I think it’s
desirable. I’m just not sure about my implementation. Any thoughts,
anyone?
-- 
Alan Third

[-- Attachment #2: 0001-Don-t-use-SIGIO-in-NS.patch --]
[-- Type: text/plain, Size: 2831 bytes --]

From bff3dbfe43a24ee924edd777a7a876b627f94f11 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sat, 29 Apr 2017 23:35:16 +0100
Subject: [PATCH] Don't use SIGIO in NS

* src/keyboard.c (handle_user_signal) [HAVE_NS]: Don't handle SIGIO.
* src/keyboard.h (handle_input_available_signal) [HAVE_NS]: Make
available to external code.
* src/nsterm.m (hold_event, ns_Select): Call handle_input_available_signal
directly.
* src/sysdep.c (emacs_sigaction_init) [HAVE_NS]: Ignore SIGIO.
---
 src/keyboard.c |  3 ++-
 src/keyboard.h | 11 +++++++++++
 src/nsterm.m   |  4 ++--
 src/sysdep.c   |  2 +-
 4 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/keyboard.c b/src/keyboard.c
index c9fa2a9f5e..d22f7b08c6 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -7264,7 +7264,8 @@ handle_user_signal (int sig)
           }
 
 	p->npending++;
-#ifdef USABLE_SIGIO
+#if defined (USABLE_SIGIO) && !defined (HAVE_NS)
+        /* Dont use the interrupt handler in NS. */
 	if (interrupt_input)
 	  handle_input_available_signal (sig);
 	else
diff --git a/src/keyboard.h b/src/keyboard.h
index 2219c01135..29279bef86 100644
--- a/src/keyboard.h
+++ b/src/keyboard.h
@@ -495,6 +495,17 @@ extern void mark_kboards (void);
 extern const char *const lispy_function_keys[];
 #endif
 
+#ifdef HAVE_NS
+/* The NS port only uses SIGIO internally, and even then only in two
+   places in the code, but it causes crashes if certain code is not
+   properly wrapped in block_input/unblock_input.
+
+   Since it's only used internally, we should be able to disable it,
+   and call the SIGIO handler directly.
+ */
+extern void handle_input_available_signal (int sig);
+#endif
+
 extern char const DEV_TTY[];
 
 INLINE_HEADER_END
diff --git a/src/nsterm.m b/src/nsterm.m
index c22c5a70ba..00b7a89472 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -472,7 +472,7 @@ - (NSColor *)colorUsingDefaultColorSpace
 
   hold_event_q.q[hold_event_q.nr++] = *event;
   /* Make sure ns_read_socket is called, i.e. we have input.  */
-  raise (SIGIO);
+  handle_input_available_signal (SIGIO);
   send_appdefined = YES;
 }
 
@@ -4289,7 +4289,7 @@ in certain situations (rapid incoming events).
   if (hold_event_q.nr > 0)
     {
       /* We already have events pending. */
-      raise (SIGIO);
+      handle_input_available_signal (SIGIO);
       errno = EINTR;
       return -1;
     }
diff --git a/src/sysdep.c b/src/sysdep.c
index 91b2a5cb94..70fdf92964 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -1606,7 +1606,7 @@ emacs_sigaction_init (struct sigaction *action, signal_handler_t handler)
     {
       sigaddset (&action->sa_mask, SIGINT);
       sigaddset (&action->sa_mask, SIGQUIT);
-#ifdef USABLE_SIGIO
+#if defined (USABLE_SIGIO) && !defined (HAVE_NS)
       sigaddset (&action->sa_mask, SIGIO);
 #endif
     }
-- 
2.12.0