=== modified file 'src/ChangeLog' --- src/ChangeLog 2012-09-04 06:45:54 +0000 +++ src/ChangeLog 2012-09-04 08:00:16 +0000 @@ -1,5 +1,42 @@ 2012-09-04 Paul Eggert + Simplify and avoid races, now that we have sigaction. + * atimer.c (turn_on_atimers): + * dispnew.c (window_change_signal): + * emacs.c (memory_warning_signal): + Don't reestablish signal; not needed with sigaction. + * emacs.c (fatal_error_signal): Now always static. + (main): Block all signals while handling fatal error; that's safer. + * floatfns.c (DO_IN_FLOAT): New macro. + (IN_FLOAT, IN_FLOAT2): Use it. + (float_error) [FLOAT_CATCH_SIGILL]: Remove; unused. + (init_floatfns): Remove; no longer anything to do. + All callers removed. + * keyboard.c (USE_SIGIO): New macro, instead of #undef SIGIO + which is riskier. All uses of "defined SIGIO" changed. + (input_available_signal): Define a substitute when the + main routine is not available. + (handle_interrupt): Now takes bool IN_SIGNAL_HANDLER as arg. All + callers changed. + * lisp.h (init_floatfns): + (fatal_error_signal) [FLOAT_CATCH_SIGILL]: Remove. + * process.c (old_sigpipe_action): New static var. + (send_process_trap): Do not unblock signal here; that leads to a race. + (send_process): Unblock it here instead. Unblock them all, since + many are blocked now. + * sysdep.c (wait_for_termination_1) [(BSD_SYSTEM||HPUX) && !__GNU__]: + Remove cruft to that was speculated to work around a long-dead bug. + (wait_debugging) [!MSDOS]: Remove now-unused variable. + (emacs_sigaction_init): Block nonfatal system signals that + are caught by Emacs, to make race conditions less likely. + Move much code to emacs_sigaction_flags, and call it. + * syssignal.h (emacs_sigaction_flags): New function, containing + much of what used to be in emacs_sigaction_init. + * xterm.c (x_connection_signal): Remove; no longer needed + now that we use sigaction.. + (x_initialize): Merely set SIGPIPE's handler to SIG_IGN, + since the handler did nothing. + Signal-handler cleanup (Bug#12327). Emacs's signal handlers were written in the old 4.2BSD style with sigblock and sigmask and so forth, and this led to some === modified file 'src/atimer.c' --- src/atimer.c 2012-09-03 07:24:03 +0000 +++ src/atimer.c 2012-09-04 06:52:12 +0000 @@ -427,12 +427,7 @@ turn_on_atimers (bool on) { if (on) - { - struct sigaction action; - emacs_sigaction_init (&action, alarm_signal_handler); - sigaction (SIGALRM, &action, 0); - set_alarm (); - } + set_alarm (); else alarm (0); } === modified file 'src/dispnew.c' --- src/dispnew.c 2012-09-03 07:24:03 +0000 +++ src/dispnew.c 2012-09-04 06:52:12 +0000 @@ -5552,17 +5552,13 @@ #ifdef SIGWINCH static void -window_change_signal (int signalnum) /* If we don't have an argument, */ - /* some compilers complain in signal calls. */ +window_change_signal (int signalnum) { int width, height; int old_errno = errno; struct tty_display_info *tty; - struct sigaction action; - emacs_sigaction_init (&action, window_change_signal); - sigaction (SIGWINCH, &action, 0); if (forwarded_signal (signalnum)) return; === modified file 'src/emacs.c' --- src/emacs.c 2012-09-03 07:24:03 +0000 +++ src/emacs.c 2012-09-04 07:20:10 +0000 @@ -290,10 +290,7 @@ /* Handle bus errors, invalid instruction, etc. */ -#ifndef FLOAT_CATCH_SIGILL -static -#endif -void +static void fatal_error_signal (int sig) { sigset_t unblocked; @@ -332,12 +329,9 @@ #ifdef SIGDANGER /* Handler for SIGDANGER. */ -void +static void memory_warning_signal (int sig) { - struct sigaction action; - emacs_sigaction_init (&action, memory_warning_signal); - sigaction (sig, &action, 0); if (forwarded_signal (sig)) return; @@ -1119,7 +1113,9 @@ } init_signals (); - emacs_sigaction_init (&fatal_error_action, fatal_error_signal); + sigfillset (&fatal_error_action.sa_mask); + fatal_error_action.sa_handler = fatal_error_signal; + fatal_error_action.sa_flags = emacs_sigaction_flags (); /* Don't catch SIGHUP if dumping. */ if (1 @@ -1595,7 +1591,6 @@ init_fringe (); #endif /* HAVE_WINDOW_SYSTEM */ init_macros (); - init_floatfns (); init_window (); init_font (); === modified file 'src/floatfns.c' --- src/floatfns.c 2012-09-03 07:24:03 +0000 +++ src/floatfns.c 2012-09-04 07:26:16 +0000 @@ -37,9 +37,6 @@ Define FLOAT_CHECK_ERRNO if the float library routines set errno. This has no effect if HAVE_MATHERR is defined. - Define FLOAT_CATCH_SIGILL if the float library routines signal SIGILL. - (What systems actually do this? Please let us know.) - Define FLOAT_CHECK_DOMAIN if the float library doesn't handle errors by either setting errno, or signaling SIGFPE/SIGILL. Otherwise, domain and range checking will happen before calling the float routines. This has @@ -98,14 +95,15 @@ # include #endif -#ifdef FLOAT_CATCH_SIGILL -static void float_error (); -#endif - +#ifdef HAVE_MATHERR /* Nonzero while executing in floating point. - This tells float_error what to do. */ + This tells matherr what to do. */ static int in_float; +#define DO_IN_FLOAT(d) ((void) (in_float = 1, d, in_float = 0)) +#else +#define DO_IN_FLOAT(d) (void) (d)) +#endif /* If an argument is out of range for a mathematical function, here is the actual argument value to use in the error message. @@ -130,7 +128,7 @@ do { \ float_error_arg = num; \ float_error_fn_name = name; \ - in_float = 1; errno = 0; (d); in_float = 0; \ + errno = 0; DO_IN_FLOAT (d); \ switch (errno) { \ case 0: break; \ case EDOM: domain_error (float_error_fn_name, float_error_arg); \ @@ -143,7 +141,7 @@ float_error_arg = num; \ float_error_arg2 = num2; \ float_error_fn_name = name; \ - in_float = 1; errno = 0; (d); in_float = 0; \ + errno = 0; DO_IN_FLOAT (d); \ switch (errno) { \ case 0: break; \ case EDOM: domain_error (float_error_fn_name, float_error_arg); \ @@ -152,8 +150,8 @@ } \ } while (0) #else -#define IN_FLOAT(d, name, num) (in_float = 1, (d), in_float = 0) -#define IN_FLOAT2(d, name, num, num2) (in_float = 1, (d), in_float = 0) +#define IN_FLOAT(d, name, num) DO_IN_FLOAT (d) +#define IN_FLOAT2(d, name, num, num2) DO_IN_FLOAT (d) #endif /* Convert float to Lisp_Int if it fits, else signal a range error @@ -946,36 +944,6 @@ return make_float (d); } -#ifdef FLOAT_CATCH_SIGILL -static void -float_error (int signo) -{ - if (! in_float) - fatal_error_signal (signo); - -#ifdef BSD_SYSTEM - pthread_sigmask (SIG_SETMASK, &empty_mask, 0); -#else - /* Must reestablish handler each time it is called. */ - { - struct sigaction action; - emacs_sigaction_init (&action, float_error); - sigaction (SIGILL, &action, 0); - } -#endif /* BSD_SYSTEM */ - - if (forwarded_signal (signo)) - return; - in_float = 0; - - xsignal1 (Qarith_error, float_error_arg); -} - -/* Another idea was to replace the library function `infnan' - where SIGILL is signaled. */ - -#endif /* FLOAT_CATCH_SIGILL */ - #ifdef HAVE_MATHERR int matherr (struct exception *x) @@ -1008,17 +976,6 @@ #endif /* HAVE_MATHERR */ void -init_floatfns (void) -{ -#ifdef FLOAT_CATCH_SIGILL - struct sigaction action; - emacs_sigaction_init (&action, float_error); - sigaction (SIGILL, &action, 0); -#endif - in_float = 0; -} - -void syms_of_floatfns (void) { defsubr (&Sacos); === modified file 'src/keyboard.c' --- src/keyboard.c 2012-09-03 07:24:03 +0000 +++ src/keyboard.c 2012-09-04 07:18:15 +0000 @@ -391,11 +391,14 @@ #endif /* We are unable to use interrupts if FIONREAD is not available, - so flush SIGIO so we won't try. */ -#if !defined (FIONREAD) -#ifdef SIGIO -#undef SIGIO + so we won't try. */ +#if defined SIGIO && defined FIONREAD +# define USE_SIGIO 1 +#else +# define USE_SIGIO 0 #endif +#ifndef SIGIO +# define SIGIO 0 /* placeholder */ #endif /* If we support a window system, turn on the code to poll periodically @@ -449,10 +452,12 @@ static void clear_event (struct input_event *); static Lisp_Object restore_kboard_configuration (Lisp_Object); static void interrupt_signal (int signalnum); -#ifdef SIGIO +#if USE_SIGIO static void input_available_signal (int signo); +#else +# define input_available_signal 0 /* placeholder */ #endif -static void handle_interrupt (void); +static void handle_interrupt (bool); static _Noreturn void quit_throw_to_read_char (int); static void process_special_events (void); static void timer_start_idle (void); @@ -3622,7 +3627,8 @@ } last_event_timestamp = event->timestamp; - handle_interrupt (); + + handle_interrupt (0); return; } @@ -3658,10 +3664,8 @@ /* Don't read keyboard input until we have processed kbd_buffer. This happens when pasting text longer than KBD_BUFFER_SIZE/2. */ hold_keyboard_input (); -#ifdef SIGIO - if (!noninteractive) + if (USE_SIGIO && !noninteractive) signal (SIGIO, SIG_IGN); -#endif stop_polling (); } #endif /* subprocesses */ @@ -3830,14 +3834,12 @@ /* Start reading input again, we have processed enough so we can accept new events again. */ unhold_keyboard_input (); -#ifdef SIGIO - if (!noninteractive) + if (USE_SIGIO && !noninteractive) { struct sigaction action; emacs_sigaction_init (&action, input_available_signal); sigaction (SIGIO, &action, 0); } -#endif /* SIGIO */ start_polling (); } #endif /* subprocesses */ @@ -3879,10 +3881,8 @@ /* One way or another, wait until input is available; then, if interrupt handlers have not read it, read it now. */ -/* Note SIGIO has been undef'd if FIONREAD is missing. */ -#ifdef SIGIO - gobble_input (0); -#endif /* SIGIO */ + if (USE_SIGIO) + gobble_input (0); if (kbd_fetch_ptr != kbd_store_ptr) break; #if defined (HAVE_MOUSE) || defined (HAVE_GPM) @@ -6781,8 +6781,7 @@ void gobble_input (int expected) { -#ifdef SIGIO - if (interrupt_input) + if (USE_SIGIO && interrupt_input) { sigset_t blocked, procmask; sigemptyset (&blocked); @@ -6807,7 +6806,6 @@ } else #endif -#endif read_avail_input (expected); } @@ -6836,8 +6834,7 @@ return; /* Make sure no interrupt happens while storing the event. */ -#ifdef SIGIO - if (interrupt_input) + if (USE_SIGIO && interrupt_input) { sigset_t blocked, procmask; sigemptyset (&blocked); @@ -6847,7 +6844,6 @@ pthread_sigmask (SIG_SETMASK, &procmask, 0); } else -#endif { stop_polling (); kbd_buffer_store_event (&event); @@ -7202,7 +7198,7 @@ return nread; } -#if defined SYNC_INPUT || defined SIGIO +#if defined SYNC_INPUT || USE_SIGIO static void handle_async_input (void) { @@ -7229,7 +7225,7 @@ --handling_signal; #endif } -#endif /* SYNC_INPUT || SIGIO */ +#endif #ifdef SYNC_INPUT void @@ -7241,9 +7237,7 @@ } #endif -#ifdef SIGIO /* for entire page */ -/* Note SIGIO has been undef'd if FIONREAD is missing. */ - +#if USE_SIGIO static void input_available_signal (int signo) { @@ -7266,7 +7260,7 @@ errno = old_errno; } -#endif /* SIGIO */ +#endif /* Send ourselves a SIGIO. @@ -7277,9 +7271,8 @@ void reinvoke_input_signal (void) { -#ifdef SIGIO - handle_async_input (); -#endif + if (USE_SIGIO) + handle_async_input (); } @@ -7355,11 +7348,9 @@ } p->npending++; -#ifdef SIGIO - if (interrupt_input) + if (USE_SIGIO && interrupt_input) kill (getpid (), SIGIO); else -#endif { /* Tell wait_reading_process_output that it needs to wake up and look around. */ @@ -10823,7 +10814,7 @@ from the controlling tty. */ internal_last_event_frame = terminal->display_info.tty->top_frame; - handle_interrupt (); + handle_interrupt (1); } errno = old_errno; @@ -10846,7 +10837,7 @@ non-nil, it stops the job right away. */ static void -handle_interrupt (void) +handle_interrupt (bool in_signal_handler) { char c; @@ -10855,13 +10846,16 @@ /* XXX This code needs to be revised for multi-tty support. */ if (!NILP (Vquit_flag) && get_named_tty ("/dev/tty")) { - /* If SIGINT isn't blocked, don't let us be interrupted by - another SIGINT, it might be harmful due to non-reentrancy - in I/O functions. */ - sigset_t blocked; - sigemptyset (&blocked); - sigaddset (&blocked, SIGINT); - pthread_sigmask (SIG_BLOCK, &blocked, 0); + if (! in_signal_handler) + { + /* If SIGINT isn't blocked, don't let us be interrupted by + a SIGINT. It might be harmful due to non-reentrancy + in I/O functions. */ + sigset_t blocked; + sigemptyset (&blocked); + sigaddset (&blocked, SIGINT); + pthread_sigmask (SIG_BLOCK, &blocked, 0); + } fflush (stdout); reset_all_sys_modes (); @@ -11025,8 +11019,7 @@ (Lisp_Object interrupt) { int new_interrupt_input; -#ifdef SIGIO -/* Note SIGIO has been undef'd if FIONREAD is missing. */ +#if USE_SIGIO #ifdef HAVE_X_WINDOWS if (x_display_list != NULL) { @@ -11037,9 +11030,9 @@ else #endif /* HAVE_X_WINDOWS */ new_interrupt_input = !NILP (interrupt); -#else /* not SIGIO */ +#else new_interrupt_input = 0; -#endif /* not SIGIO */ +#endif if (new_interrupt_input != interrupt_input) { @@ -11434,15 +11427,12 @@ sigaction (SIGQUIT, &action, 0); #endif /* not DOS_NT */ } -/* Note SIGIO has been undef'd if FIONREAD is missing. */ -#ifdef SIGIO - if (!noninteractive) + if (USE_SIGIO && !noninteractive) { struct sigaction action; emacs_sigaction_init (&action, input_available_signal); sigaction (SIGIO, &action, 0); } -#endif /* SIGIO */ /* Use interrupt input by default, if it works and noninterrupt input has deficiencies. */ === modified file 'src/lisp.h' --- src/lisp.h 2012-09-02 17:10:35 +0000 +++ src/lisp.h 2012-09-04 07:20:16 +0000 @@ -2695,7 +2695,6 @@ /* Defined in floatfns.c */ extern double extract_float (Lisp_Object); -extern void init_floatfns (void); extern void syms_of_floatfns (void); extern Lisp_Object fmod_float (Lisp_Object x, Lisp_Object y); @@ -3263,9 +3262,6 @@ extern Lisp_Object decode_env_path (const char *, const char *); extern Lisp_Object empty_unibyte_string, empty_multibyte_string; extern Lisp_Object Qfile_name_handler_alist; -#ifdef FLOAT_CATCH_SIGILL -extern void fatal_error_signal (int); -#endif extern Lisp_Object Qkill_emacs; #if HAVE_SETLOCALE void fixup_locale (void); === modified file 'src/process.c' --- src/process.c 2012-09-04 06:45:54 +0000 +++ src/process.c 2012-09-04 06:52:46 +0000 @@ -5412,6 +5412,7 @@ /* Sending data to subprocess */ static jmp_buf send_process_frame; +struct sigaction old_sigpipe_action; static Lisp_Object process_sent_to; #ifndef FORWARD_SIGNAL_TO_MAIN_THREAD @@ -5421,12 +5422,8 @@ static void send_process_trap (int ignore) { - sigset_t unblocked; if (forwarded_signal (SIGPIPE)) return; - sigemptyset (&unblocked); - sigaddset (&unblocked, SIGPIPE); - pthread_sigmask (SIG_UNBLOCK, &unblocked, 0); _longjmp (send_process_frame, 1); } @@ -5520,7 +5517,6 @@ struct Lisp_Process *p = XPROCESS (proc); ssize_t rv; struct coding_system *coding; - struct sigaction old_sigpipe_action; if (p->raw_status_new) update_status (p); @@ -5757,7 +5753,7 @@ } else { - sigaction (SIGPIPE, &old_sigpipe_action, 0); + pthread_sigmask (SIG_SETMASK, &empty_mask, 0); proc = process_sent_to; p = XPROCESS (proc); p->raw_status_new = 0; === modified file 'src/sysdep.c' --- src/sysdep.c 2012-09-03 07:38:38 +0000 +++ src/sysdep.c 2012-09-04 07:55:33 +0000 @@ -283,10 +283,6 @@ -/* Set nonzero to make following function work under dbx - (at least for bsd). */ -int wait_debugging EXTERNALLY_VISIBLE; - #ifndef MSDOS static void @@ -294,30 +290,6 @@ { while (1) { -#if (defined (BSD_SYSTEM) || defined (HPUX)) && !defined (__GNU__) - /* Note that kill returns -1 even if the process is just a zombie now. - But inevitably a SIGCHLD interrupt should be generated - and child_sig will do waitpid and make the process go away. */ - /* There is some indication that there is a bug involved with - termination of subprocesses, perhaps involving a kernel bug too, - but no idea what it is. Just as a hunch we signal SIGCHLD to see - if that causes the problem to go away or get worse. */ - sigset_t sigchild_mask; - sigemptyset (&sigchild_mask); - sigaddset (&sigchild_mask, SIGCHLD); - pthread_sigmask (SIG_SETMASK, &sigchild_mask, 0); - - if (0 > kill (pid, 0)) - { - pthread_sigmask (SIG_SETMASK, &empty_mask, 0); - kill (getpid (), SIGCHLD); - break; - } - if (wait_debugging) - sleep (1); - else - sigsuspend (&empty_mask); -#else /* not BSD_SYSTEM, and not HPUX version >= 6 */ #ifdef WINDOWSNT wait (0); break; @@ -335,7 +307,6 @@ sigsuspend (&empty_mask); #endif /* not WINDOWSNT */ -#endif /* not BSD_SYSTEM, and not HPUX version >= 6 */ if (interruptible) QUIT; } @@ -1494,26 +1465,32 @@ emacs_sigaction_init (struct sigaction *action, signal_handler_t handler) { sigemptyset (&action->sa_mask); + + /* When handling a signal, block nonfatal system signals that are caught + by Emacs. This makes race conditions less likely. */ + sigaddset (&action->sa_mask, SIGALRM); +#ifdef SIGCHLD + sigaddset (&action->sa_mask, SIGCHLD); +#endif +#ifdef SIGDANGER + sigaddset (&action->sa_mask, SIGDANGER); +#endif + sigaddset (&action->sa_mask, SIGFPE); + sigaddset (&action->sa_mask, SIGPIPE); +#ifdef SIGWINCH + sigaddset (&action->sa_mask, SIGWINCH); +#endif + if (! noninteractive) + { + sigaddset (&action->sa_mask, SIGINT); + sigaddset (&action->sa_mask, SIGQUIT); +#ifdef SIGIO + sigaddset (&action->sa_mask, SIGIO); +#endif + } + action->sa_handler = handler; - action->sa_flags = 0; -#if defined (SA_RESTART) - /* Emacs mostly works better with restartable system services. If this - flag exists, we probably want to turn it on here. - However, on some systems (only hpux11 at present) this resets the - timeout of `select' which means that `select' never finishes if - it keeps getting signals. - We define BROKEN_SA_RESTART on those systems. */ - /* It's not clear why the comment above says "mostly works better". --Stef - When SYNC_INPUT is set, we don't want SA_RESTART because we need to poll - for pending input so we need long-running syscalls to be interrupted - after a signal that sets the interrupt_input_pending flag. */ - /* Non-interactive keyboard input goes through stdio, where we always - want restartable system calls. */ -# if defined (BROKEN_SA_RESTART) || defined (SYNC_INPUT) - if (noninteractive) -# endif - action->sa_flags = SA_RESTART; -#endif + action->sa_flags = emacs_sigaction_flags (); } #ifdef FORWARD_SIGNAL_TO_MAIN_THREAD === modified file 'src/syssignal.h' --- src/syssignal.h 2012-09-03 07:38:38 +0000 +++ src/syssignal.h 2012-09-04 06:52:12 +0000 @@ -40,6 +40,33 @@ extern void emacs_sigaction_init (struct sigaction *, signal_handler_t); +SYSSIGNAL_INLINE int +emacs_sigaction_flags (void) +{ +#ifdef SA_RESTART + /* Emacs mostly works better with restartable system services. If this + flag exists, we probably want to turn it on here. + However, on some systems (only hpux11 at present) this resets the + timeout of `select' which means that `select' never finishes if + it keeps getting signals. + We define BROKEN_SA_RESTART on those systems. */ + /* It's not clear why the comment above says "mostly works better". --Stef + When SYNC_INPUT is set, we don't want SA_RESTART because we need to poll + for pending input so we need long-running syscalls to be interrupted + after a signal that sets the interrupt_input_pending flag. */ + /* Non-interactive keyboard input goes through stdio, where we always + want restartable system calls. */ +# if defined BROKEN_SA_RESTART || defined SYNC_INPUT + bool use_SA_RESTART = noninteractive; +#else + bool use_SA_RESTART = 1; +# endif + if (use_SA_RESTART) + return SA_RESTART; +#endif + return 0; +} + #if ! (defined TIOCNOTTY || defined USG5 || defined CYGWIN) _Noreturn void croak (char *); #endif === modified file 'src/xterm.c' --- src/xterm.c 2012-09-03 07:24:03 +0000 +++ src/xterm.c 2012-09-04 07:17:47 +0000 @@ -7749,26 +7749,6 @@ #endif /* ! 0 */ -/* Handle SIGPIPE, which can happen when the connection to a server - simply goes away. SIGPIPE is handled by x_connection_signal. - Don't need to do anything, because the write which caused the - SIGPIPE will fail, causing Xlib to invoke the X IO error handler, - which will do the appropriate cleanup for us. */ - -static void -x_connection_signal (int signalnum) /* If we don't have an argument, */ - /* some compilers complain in signal calls. */ -{ -#ifdef USG - /* USG systems forget handlers when they are used; - must reestablish each time */ - struct sigaction action; - emacs_sigaction_init (&action, x_connection_signal); - sigaction (signalnum, &action, 0); -#endif /* USG */ -} - - /************************************************************************ Handling X errors ************************************************************************/ @@ -10760,8 +10740,6 @@ void x_initialize (void) { - struct sigaction action; - baud_rate = 19200; x_noop_count = 0; @@ -10808,8 +10786,7 @@ XSetErrorHandler (x_error_handler); XSetIOErrorHandler (x_io_error_quitter); - emacs_sigaction_init (&action, x_connection_signal); - sigaction (SIGPIPE, &action, 0); + signal (SIGPIPE, SIG_IGN); }