* SIGIO and the NS port @ 2017-03-10 12:41 Alan Third 2017-03-10 13:35 ` Eli Zaretskii 2017-03-13 16:38 ` Alan Third 0 siblings, 2 replies; 6+ messages in thread From: Alan Third @ 2017-03-10 12:41 UTC (permalink / raw) To: Emacs-Devel devel I think I’ve just found a simple solution to a whole class of bugs in the NS port. We’ve had a number of bug reports where Emacs crashes while doing UI related things, for example minimizing the frame, and generally we’ve found the best way to deal with these is by wrapping the relevant bit in block_input/unblock_input. Today I discovered that config.h on my machine has two defines related to SIGIO: #define INTERRUPT_INPUT 1 #define USABLE_SIGIO 1 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. Does anyone know of any reason not to remove these two defines? How would I go about doing that? I assume config.h is generated by ./configure, but I don’t know anything about that side of things. Thanks! -- Alan Third ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: SIGIO and the NS port 2017-03-10 12:41 SIGIO and the NS port Alan Third @ 2017-03-10 13:35 ` Eli Zaretskii 2017-03-13 16:38 ` Alan Third 1 sibling, 0 replies; 6+ messages in thread From: Eli Zaretskii @ 2017-03-10 13:35 UTC (permalink / raw) To: Alan Third; +Cc: emacs-devel > Date: Fri, 10 Mar 2017 12:41:17 +0000 > From: Alan Third <alan@idiocy.org> > > Does anyone know of any reason not to remove these two defines? > > How would I go about doing that? I assume config.h is generated by > ./configure, but I don’t know anything about that side of things. Yes, it's generated by configure, but you can override what it does in conf_post.h, with suitable preprocessor conditionals. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: SIGIO and the NS port 2017-03-10 12:41 SIGIO and the NS port Alan Third 2017-03-10 13:35 ` Eli Zaretskii @ 2017-03-13 16:38 ` Alan Third 2017-05-20 23:24 ` Alan Third 1 sibling, 1 reply; 6+ messages in thread From: Alan Third @ 2017-03-13 16:38 UTC (permalink / raw) To: Emacs-Devel devel On Fri, Mar 10, 2017 at 12:41:17PM +0000, Alan Third wrote: > Today I discovered that config.h on my machine has two defines related > to SIGIO: > > #define INTERRUPT_INPUT 1 > #define USABLE_SIGIO 1 > > 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.) I was hoping this would work as it would mean I could remove block_input/unblock_input from ns_select, which appears to be the main problem with concurrency. -- Alan Third ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: SIGIO and the NS port 2017-03-13 16:38 ` Alan Third @ 2017-05-20 23:24 ` Alan Third 2017-05-21 7:46 ` Paul Eggert 0 siblings, 1 reply; 6+ messages in thread From: Alan Third @ 2017-05-20 23:24 UTC (permalink / raw) To: Emacs-Devel devel [-- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: SIGIO and the NS port 2017-05-20 23:24 ` Alan Third @ 2017-05-21 7:46 ` Paul Eggert 2017-05-21 12:11 ` Alan Third 0 siblings, 1 reply; 6+ messages in thread From: Paul Eggert @ 2017-05-21 7:46 UTC (permalink / raw) To: Alan Third, Emacs-Devel devel Alan Third wrote: > I’ve stopped the signal handler from responding to SIGIO, I'm not following, as sigaction (SIGIO, ...) is still being called, so the signal handler deliver_input_available_signal is still responding to SIGIO, and is setting pending_signals to true. The changes you made affect SIGUSR1 and SIGUSR2 handling, since they patch handle_user_signal: they cause the SIGUSR1 and SIGUSR2 handler to not process SIGIO events. Also, they prevent SIGIO from being masked out during non-SIGIO signal handlers. Is that what you intended? I'm not getting the connection between these non-SIGIO handlers and interrupt_input. > and the two > places where the NS GUI used to raise SIGIO have been replaced with > direct calls to the SIGIO handler code. That sounds like a reasonable thing to do in any event. I don't see why the code needs to do a raise (SIGIO) when it can do a handle_input_available_signal (SIGIO); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: SIGIO and the NS port 2017-05-21 7:46 ` Paul Eggert @ 2017-05-21 12:11 ` Alan Third 0 siblings, 0 replies; 6+ messages in thread From: Alan Third @ 2017-05-21 12:11 UTC (permalink / raw) To: Paul Eggert; +Cc: Emacs-Devel devel On Sun, May 21, 2017 at 12:46:53AM -0700, Paul Eggert wrote: > Alan Third wrote: > > I’ve stopped the signal handler from responding to SIGIO, > > I'm not following, as sigaction (SIGIO, ...) is still being called, so the > signal handler deliver_input_available_signal is still responding to SIGIO, > and is setting pending_signals to true. > > The changes you made affect SIGUSR1 and SIGUSR2 handling, since they patch > handle_user_signal: they cause the SIGUSR1 and SIGUSR2 handler to not > process SIGIO events. Also, they prevent SIGIO from being masked out during > non-SIGIO signal handlers. Is that what you intended? I'm not getting the > connection between these non-SIGIO handlers and interrupt_input. I’ve probably misunderstood what the code was doing. Would it be best to just leave the signal handlers as‐is, but replace the calls to raise(SIGIO) with direct calls as below? > > and the two > > places where the NS GUI used to raise SIGIO have been replaced with > > direct calls to the SIGIO handler code. > > That sounds like a reasonable thing to do in any event. I don't see why the > code needs to do a raise (SIGIO) when it can do a > handle_input_available_signal (SIGIO); Thanks! -- Alan Third ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-21 12:11 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-03-10 12:41 SIGIO and the NS port Alan Third 2017-03-10 13:35 ` Eli Zaretskii 2017-03-13 16:38 ` Alan Third 2017-05-20 23:24 ` Alan Third 2017-05-21 7:46 ` Paul Eggert 2017-05-21 12:11 ` Alan Third
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.