* bug#12327: Signal-handler cleanup for Emacs @ 2012-09-01 22:38 Paul Eggert 2012-09-02 17:51 ` Eli Zaretskii 2012-09-07 1:35 ` Paul Eggert 0 siblings, 2 replies; 23+ messages in thread From: Paul Eggert @ 2012-09-01 22:38 UTC (permalink / raw) To: 12327; +Cc: Juanma Barranquero Tags: patch Here's a patch to clean up Emacs's signal-handling code somewhat. As described below, it shrinks the size of Emacs's text segment by 0.6% on my platform. It also removes a few dozen lines from the source code. This patch affects only the mainline code. Some tweaks will be needed to the Windows port, which will need to define the functions 'sighandler' and 'set_sighandler'; these are specialized replacements for 'sys_signal' that avoid the overhead of setting/getting when it's not needed. I'm planning to install this into the trunk and am publishing it now as a head-up for the Windows side. === modified file 'ChangeLog' --- ChangeLog 2012-09-01 18:54:38 +0000 +++ ChangeLog 2012-09-01 21:51:42 +0000 @@ -1,5 +1,10 @@ 2012-09-01 Paul Eggert <eggert@cs.ucla.edu> + Signal-handler cleanup. + * configure.ac (PTY_OPEN, PTY_TTY_NAME_SPRINTF): + Adjust to syssignal.h changes. + (SIGNAL_H_AB): Remove; no longer needed. + * configure.ac (_FORTIFY_SOURCE): Define only when optimizing. This ports to glibc 2.15 or later, when configured with --enable-gcc-warnings. See Eric Blake in === modified file 'configure.ac' --- configure.ac 2012-09-01 18:54:38 +0000 +++ configure.ac 2012-09-01 21:37:48 +0000 @@ -3445,7 +3445,7 @@ cygwin ) AC_DEFINE(PTY_ITERATION, [int i; for (i = 0; i < 1; i++)]) dnl multi-line AC_DEFINEs are hard. :( - AC_DEFINE(PTY_OPEN, [ do { int dummy; SIGMASKTYPE mask; mask = sigblock (sigmask (SIGCHLD)); if (-1 == openpty (&fd, &dummy, pty_name, 0, 0)) fd = -1; sigsetmask (mask); if (fd >= 0) emacs_close (dummy); } while (0)]) + AC_DEFINE(PTY_OPEN, [ do { int dummy; sigset_t mask; get_sigmask_block_signal (SIGCHLD, &mask); if (-1 == openpty (&fd, &dummy, pty_name, 0, 0)) fd = -1; set_signal_mask (&mask); if (fd >= 0) emacs_close (dummy); } while (0)]) AC_DEFINE(PTY_NAME_SPRINTF, []) AC_DEFINE(PTY_TTY_NAME_SPRINTF, []) ;; @@ -3474,7 +3474,7 @@ AC_DEFINE(PTY_ITERATION, [int i; for (i = 0; i < 1; i++)]) dnl Note that grantpt and unlockpt may fork. We must block SIGCHLD dnl to prevent sigchld_handler from intercepting the child's death. - AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptyname; sigblock (sigmask (SIGCHLD)); if (grantpt (fd) == -1 || unlockpt (fd) == -1 || !(ptyname = ptsname(fd))) { sigunblock (sigmask (SIGCHLD)); close (fd); return -1; } snprintf (pty_name, sizeof pty_name, "%s", ptyname); sigunblock (sigmask (SIGCHLD)); }]) + AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptyname; block_signal (SIGCHLD); if (grantpt (fd) == -1 || unlockpt (fd) == -1 || !(ptyname = ptsname(fd))) { unblock_signal (SIGCHLD); close (fd); return -1; } snprintf (pty_name, sizeof pty_name, "%s", ptyname); unblock_signal (SIGCHLD); }]) dnl if HAVE_POSIX_OPENPT if test "x$ac_cv_func_posix_openpt" = xyes; then AC_DEFINE(PTY_OPEN, [fd = posix_openpt (O_RDWR | O_NOCTTY)]) @@ -3525,12 +3525,12 @@ dnl On SysVr4, grantpt(3) forks a subprocess, so keep sigchld_handler() dnl from intercepting that death. If any child but grantpt's should die dnl within, it should be caught after sigrelse(2). - AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptsname (int), *ptyname; sigblock (sigmask (SIGCLD)); if (grantpt (fd) == -1) { emacs_close (fd); return -1; } sigunblock (sigmask (SIGCLD)); if (unlockpt (fd) == -1) { emacs_close (fd); return -1; } if (!(ptyname = ptsname (fd))) { emacs_close (fd); return -1; } snprintf (pty_name, sizeof pty_name, "%s", ptyname); }]) + AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptsname (int), *ptyname; block_signal (SIGCLD); if (grantpt (fd) == -1) { emacs_close (fd); return -1; } unblock_signal (SIGCLD); if (unlockpt (fd) == -1) { emacs_close (fd); return -1; } if (!(ptyname = ptsname (fd))) { emacs_close (fd); return -1; } snprintf (pty_name, sizeof pty_name, "%s", ptyname); }]) ;; unixware ) dnl Comments are as per sol2*. - AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptsname (int), *ptyname; sigblock(sigmask(SIGCLD)); if (grantpt(fd) == -1) fatal("could not grant slave pty"); sigunblock(sigmask(SIGCLD)); if (unlockpt(fd) == -1) fatal("could not unlock slave pty"); if (!(ptyname = ptsname(fd))) fatal ("could not enable slave pty"); snprintf (pty_name, sizeof pty_name, "%s", ptyname); }]) + AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptsname (int), *ptyname; block_signal (SIGCLD); if (grantpt(fd) == -1) fatal("could not grant slave pty"); unblock_signal (SIGCLD); if (unlockpt(fd) == -1) fatal("could not unlock slave pty"); if (!(ptyname = ptsname(fd))) fatal ("could not enable slave pty"); snprintf (pty_name, sizeof pty_name, "%s", ptyname); }]) ;; esac @@ -3806,13 +3806,6 @@ AC_DEFINE(XOS_NEEDS_TIME_H, 1, [Compensate for a bug in Xos.h on some systems, where it requires time.h.]) ;; - - netbsd | openbsd ) - dnl Greg A. Woods <woods@weird.com> says we must include signal.h - dnl before syssignal.h is included, to work around interface conflicts - dnl that are handled with CPP __RENAME() macro in signal.h. - AC_DEFINE(SIGNAL_H_AHB, 1, [Define if AH_BOTTOM should include signal.h.]) - ;; esac === modified file 'src/ChangeLog' --- src/ChangeLog 2012-09-01 13:54:27 +0000 +++ src/ChangeLog 2012-09-01 21:50:12 +0000 @@ -1,3 +1,66 @@ +2012-09-01 Paul Eggert <eggert@cs.ucla.edu> + + Signal-handler cleanup. + Emacs's signal handlers were written in the old 4.2BSD style with + sigblock and sigmask and so forth, and this led to some + inefficiencies and confusion. Rewrite these to use + pthread_sigmask etc. without copying signal sets around. Also, + get rid of the confusing macros 'SIGNAL_THREAD_CHECK' and + 'signal', and instead use functions that do not attempt to take + over the system name space. This patch causes Emacs's text + segment to shrink by 0.6% on my platform, Fedora 17 x86-64. + * alloc.c, emacsgtkfixed.c, nsfns.m, widget.c, xmenu.c: + Do not include <signal.h> or "syssignal.h", as these + modules do not use signals. + * atimer.c, callproc.c, data.c, dispnew.c, emacs.c, floatfns.c: + * gtkutil.c, keyboard.c, process.c, sound.c, sysdep.c, term.c, xterm.c: + Do not include <signal.h>, as "syssignal.h" does that for us now. + * callproc.c (Fcall_process) [HAVE_WORKING_VFORK]: + Use new get_sigmask_block_signal and set_sigmask functions instead + of repeating the code inline. + * conf_post.h [SIGNAL_H_AHB]: Do not include <signal.h>; + no longer needed here. + * emacs.c (main): Use new sighandler function, so that there's + no need to block and unblock SIGHUP. + * sysdep.c (SYSSIGNAL_INLINE): New macro. + (empty_mask): Remove. All uses replaced to create their own + masks, as this avoids static storage and offers the compiler more + optimization opportunities. + (sighandler, get_set_sighandler, set_sighandler) [!WINDOWSNT]: + New functions. All users of 'signal' modified to use sighandler + if they're readonly, to set_sighandler if they're writeonly, and to + sys_signal if they're read+write. + (sys_signal): Reimplement in terms of get_set_sighandler. + (sys_sigmask) [!__GNUC__]: Remove; no longer needed. + (sys_sigblock): Remove; replaced by block_signal and + get_sigmask_block_signal. All uses changed. + (sys_sigunblock): Remove; replaced by unblock_signal. All uses changed. + (sys_sigsetmask): Remove; replaced by set_sigmask. All uses changed. + * syssignal.h: Include <signal.h>, since we need its types, + and <stdbool.h> since we need bool. + (SIGMASKTYPE): Remove. All uses replaced by its definiens, sigset_t. + (SIGEMPTYMASK, empty_mask): Remove as described above. + (sigmask, sys_sigmask): Remove; no longer needed. + (sigpause): Remove. All uses replaced by its definiens, sigsuspend. + (sigblock): Remove. All uses replaced by block_signal. + (sigunblock): Remove. All uses replaced by unblock_signal. + (sigsetmask) [!defined sigsetmask]: Remove. All uses replaced by + set_sigmask. + (signal): Remove. All uses replaced by sighandler, set_sighandler, + or sys_signal as described above. + (init_signals): Now ATTRIBUTE_CONST in the usual case. + (get_sigmask_block_signal, block_signal, unblock_signal, set_sigmask): + New functions. + (sigfree): Reimplement in terms of set_sigmask. + (sighandler, set_sighandler): New decls. + (sys_sigblock, sys_sigunblock, sys_sigsetmask): Remove decls. + (sys_sigdel): Remove; unused. + (NSIG): Remove a FIXME; the code's fine. Remove an unnecessary ifdef. + (forwarded_signal): Rename from SIGNAL_THREAD_CHECK, and turn into + a boolean function. This is cleaner, because the caller can now + see that the check might immediately return from the caller. + All uses changed. + 2012-09-01 Eli Zaretskii <eliz@gnu.org> * w32uniscribe.c (uniscribe_shape): Handle correctly the case of === modified file 'src/alloc.c' --- src/alloc.c 2012-08-31 10:53:19 +0000 +++ src/alloc.c 2012-09-01 21:37:07 +0000 @@ -26,8 +26,6 @@ #include <limits.h> /* For CHAR_BIT. */ #include <setjmp.h> -#include <signal.h> - #ifdef HAVE_PTHREAD #include <pthread.h> #endif @@ -42,7 +40,6 @@ #include "keyboard.h" #include "frame.h" #include "blockinput.h" -#include "syssignal.h" #include "termhooks.h" /* For struct terminal. */ #include <setjmp.h> #include <verify.h> === modified file 'src/atimer.c' --- src/atimer.c 2012-08-23 08:27:08 +0000 +++ src/atimer.c 2012-09-01 21:37:07 +0000 @@ -17,7 +17,6 @@ along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */ #include <config.h> -#include <signal.h> #include <stdio.h> #include <setjmp.h> #include "lisp.h" @@ -51,8 +50,8 @@ /* Block/unblock SIGALRM. */ -#define BLOCK_ATIMERS sigblock (sigmask (SIGALRM)) -#define UNBLOCK_ATIMERS sigunblock (sigmask (SIGALRM)) +#define BLOCK_ATIMERS block_signal (SIGALRM) +#define UNBLOCK_ATIMERS unblock_signal (SIGALRM) /* Function prototypes. */ @@ -378,7 +377,8 @@ alarm_signal_handler (int signo) { #ifndef SYNC_INPUT - SIGNAL_THREAD_CHECK (signo); + if (forwarded_signal (signo)) + return; #endif pending_atimers = 1; @@ -412,7 +412,7 @@ { if (on) { - signal (SIGALRM, alarm_signal_handler); + set_sighandler (SIGALRM, alarm_signal_handler); set_alarm (); } else @@ -426,5 +426,5 @@ free_atimers = stopped_atimers = atimers = NULL; pending_atimers = 0; /* pending_signals is initialized in init_keyboard.*/ - signal (SIGALRM, alarm_signal_handler); + set_sighandler (SIGALRM, alarm_signal_handler); } === modified file 'src/callproc.c' --- src/callproc.c 2012-08-25 03:11:12 +0000 +++ src/callproc.c 2012-09-01 21:37:07 +0000 @@ -19,7 +19,6 @@ #include <config.h> -#include <signal.h> #include <errno.h> #include <stdio.h> #include <setjmp.h> @@ -500,15 +499,11 @@ int fd_error = fd1; #ifdef HAVE_WORKING_VFORK sigset_t procmask; - sigset_t blocked; struct sigaction sigpipe_action; #endif if (fd_output >= 0) fd1 = fd_output; -#if 0 /* Some systems don't have sigblock. */ - mask = sigblock (sigmask (SIGCHLD)); -#endif /* Record that we're about to create a synchronous process. */ synch_process_alive = 1; @@ -597,10 +592,8 @@ this sets the parent's signal handlers as well as the child's. So delay all interrupts whose handlers the child might munge, and record the current handlers so they can be restored later. */ - sigemptyset (&blocked); - sigaddset (&blocked, SIGPIPE); sigaction (SIGPIPE, 0, &sigpipe_action); - pthread_sigmask (SIG_BLOCK, &blocked, &procmask); + get_sigmask_block_signal (&procmask, SIGPIPE); #endif BLOCK_INPUT; @@ -649,9 +642,9 @@ /* GConf causes us to ignore SIGPIPE, make sure it is restored in the child. */ - signal (SIGPIPE, SIG_DFL); + set_sighandler (SIGPIPE, SIG_DFL); #ifdef HAVE_WORKING_VFORK - pthread_sigmask (SIG_SETMASK, &procmask, 0); + set_sigmask (&procmask); #endif child_setup (filefd, fd1, fd_error, (char **) new_argv, @@ -663,7 +656,7 @@ #ifdef HAVE_WORKING_VFORK /* Restore the signal state. */ sigaction (SIGPIPE, &sigpipe_action, 0); - pthread_sigmask (SIG_SETMASK, &procmask, 0); + set_sigmask (&procmask); #endif #endif /* not WINDOWSNT */ === modified file 'src/conf_post.h' --- src/conf_post.h 2012-08-20 16:48:10 +0000 +++ src/conf_post.h 2012-09-01 21:37:07 +0000 @@ -40,11 +40,6 @@ #endif #endif -#ifdef SIGNAL_H_AHB -#undef SIGNAL_H_AHB -#include <signal.h> -#endif - /* This silences a few compilation warnings on FreeBSD. */ #ifdef BSD_SYSTEM_AHB #undef BSD_SYSTEM_AHB === modified file 'src/data.c' --- src/data.c 2012-08-27 17:23:48 +0000 +++ src/data.c 2012-09-01 21:37:07 +0000 @@ -19,7 +19,6 @@ #include <config.h> -#include <signal.h> #include <stdio.h> #include <setjmp.h> @@ -3213,9 +3212,9 @@ static void arith_error (int signo) { - sigsetmask (SIGEMPTYMASK); - - SIGNAL_THREAD_CHECK (signo); + sigfree (); + if (forwarded_signal (signo)) + return; xsignal0 (Qarith_error); } @@ -3230,5 +3229,5 @@ if (!initialized) return; #endif /* CANNOT_DUMP */ - signal (SIGFPE, arith_error); + set_sighandler (SIGFPE, arith_error); } === modified file 'src/dispnew.c' --- src/dispnew.c 2012-09-01 06:38:52 +0000 +++ src/dispnew.c 2012-09-01 21:37:07 +0000 @@ -21,7 +21,6 @@ #define DISPEXTERN_INLINE EXTERN_INLINE -#include <signal.h> #include <stdio.h> #include <setjmp.h> #include <unistd.h> @@ -5561,8 +5560,9 @@ struct tty_display_info *tty; - signal (SIGWINCH, window_change_signal); - SIGNAL_THREAD_CHECK (signalnum); + set_sighandler (SIGWINCH, window_change_signal); + if (forwarded_signal (signalnum)) + return; /* The frame size change obviously applies to a single termcap-controlled terminal, but we can't decide which. @@ -6173,7 +6173,7 @@ #ifndef CANNOT_DUMP if (initialized) #endif /* CANNOT_DUMP */ - signal (SIGWINCH, window_change_signal); + set_sighandler (SIGWINCH, window_change_signal); #endif /* SIGWINCH */ /* If running as a daemon, no need to initialize any frames/terminal. */ === modified file 'src/emacs.c' --- src/emacs.c 2012-09-01 08:01:36 +0000 +++ src/emacs.c 2012-09-01 21:37:07 +0000 @@ -20,7 +20,6 @@ #include <config.h> -#include <signal.h> #include <errno.h> #include <stdio.h> @@ -297,9 +296,10 @@ void fatal_error_signal (int sig) { - SIGNAL_THREAD_CHECK (sig); + if (forwarded_signal (sig)) + return; fatal_error_code = sig; - signal (sig, SIG_DFL); + set_sighandler (sig, SIG_DFL); TOTALLY_UNBLOCK_INPUT; @@ -319,7 +319,7 @@ going to send is probably blocked, so we have to unblock it if we want to really receive it. */ #ifndef MSDOS - sigunblock (sigmask (fatal_error_code)); + unblock_signal (fatal_error_code); #endif kill (getpid (), fatal_error_code); @@ -331,8 +331,9 @@ void memory_warning_signal (int sig) { - signal (sig, memory_warning_signal); - SIGNAL_THREAD_CHECK (sig); + set_sighandler (sig, memory_warning_signal); + if (forwarded_signal (sig)) + return; malloc_warning ("Operating system warns that virtual memory is running low.\n"); @@ -1119,13 +1120,10 @@ #endif ) { - sigblock (sigmask (SIGHUP)); /* In --batch mode, don't catch SIGHUP if already ignored. That makes nohup work. */ - if (! noninteractive - || signal (SIGHUP, SIG_IGN) != SIG_IGN) - signal (SIGHUP, fatal_error_signal); - sigunblock (sigmask (SIGHUP)); + if (! (noninteractive && sighandler (SIGHUP) == SIG_IGN)) + set_sighandler (SIGHUP, fatal_error_signal); } if ( @@ -1139,9 +1137,9 @@ /* Don't catch these signals in batch mode if dumping. On some machines, this sets static data that would make signal fail to work right when the dumped Emacs is run. */ - signal (SIGQUIT, fatal_error_signal); - signal (SIGILL, fatal_error_signal); - signal (SIGTRAP, fatal_error_signal); + set_sighandler (SIGQUIT, fatal_error_signal); + set_sighandler (SIGILL, fatal_error_signal); + set_sighandler (SIGTRAP, fatal_error_signal); #ifdef SIGUSR1 add_user_signal (SIGUSR1, "sigusr1"); #endif @@ -1149,68 +1147,69 @@ add_user_signal (SIGUSR2, "sigusr2"); #endif #ifdef SIGABRT - signal (SIGABRT, fatal_error_signal); + set_sighandler (SIGABRT, fatal_error_signal); #endif #ifdef SIGHWE - signal (SIGHWE, fatal_error_signal); + set_sighandler (SIGHWE, fatal_error_signal); #endif #ifdef SIGPRE - signal (SIGPRE, fatal_error_signal); + set_sighandler (SIGPRE, fatal_error_signal); #endif #ifdef SIGORE - signal (SIGORE, fatal_error_signal); + set_sighandler (SIGORE, fatal_error_signal); #endif #ifdef SIGUME - signal (SIGUME, fatal_error_signal); + set_sighandler (SIGUME, fatal_error_signal); #endif #ifdef SIGDLK - signal (SIGDLK, fatal_error_signal); + set_sighandler (SIGDLK, fatal_error_signal); #endif #ifdef SIGCPULIM - signal (SIGCPULIM, fatal_error_signal); + set_sighandler (SIGCPULIM, fatal_error_signal); #endif #ifdef SIGIOT /* This is missing on some systems - OS/2, for example. */ - signal (SIGIOT, fatal_error_signal); + set_sighandler (SIGIOT, fatal_error_signal); #endif #ifdef SIGEMT - signal (SIGEMT, fatal_error_signal); + set_sighandler (SIGEMT, fatal_error_signal); #endif - signal (SIGFPE, fatal_error_signal); + set_sighandler (SIGFPE, fatal_error_signal); #ifdef SIGBUS - signal (SIGBUS, fatal_error_signal); + set_sighandler (SIGBUS, fatal_error_signal); #endif - signal (SIGSEGV, fatal_error_signal); + set_sighandler (SIGSEGV, fatal_error_signal); #ifdef SIGSYS - signal (SIGSYS, fatal_error_signal); + set_sighandler (SIGSYS, fatal_error_signal); #endif /* May need special treatment on MS-Windows. See http://lists.gnu.org/archive/html/emacs-devel/2010-09/msg01062.html Please update the doc of kill-emacs, kill-emacs-hook, and NEWS if you change this. */ - if (noninteractive) signal (SIGINT, fatal_error_signal); - signal (SIGTERM, fatal_error_signal); + if (noninteractive) + set_sighandler (SIGINT, fatal_error_signal); + set_sighandler (SIGTERM, fatal_error_signal); #ifdef SIGXCPU - signal (SIGXCPU, fatal_error_signal); + set_sighandler (SIGXCPU, fatal_error_signal); #endif #ifdef SIGXFSZ - signal (SIGXFSZ, fatal_error_signal); + set_sighandler (SIGXFSZ, fatal_error_signal); #endif /* SIGXFSZ */ #ifdef SIGDANGER /* This just means available memory is getting low. */ - signal (SIGDANGER, memory_warning_signal); + set_sighandler (SIGDANGER, memory_warning_signal); #endif #ifdef AIX /* 20 is SIGCHLD, 21 is SIGTTIN, 22 is SIGTTOU. */ - signal (SIGXCPU, fatal_error_signal); - signal (SIGIOINT, fatal_error_signal); - signal (SIGGRANT, fatal_error_signal); - signal (SIGRETRACT, fatal_error_signal); - signal (SIGSOUND, fatal_error_signal); - signal (SIGMSG, fatal_error_signal); + set_sighandler (SIGXCPU, fatal_error_signal); + set_sighandler (SIGIOINT, fatal_error_signal); + set_sighandler (SIGGRANT, fatal_error_signal); + set_sighandler (SIGRETRACT, fatal_error_signal); + set_sighandler (SIGSOUND, fatal_error_signal); + set_sighandler (SIGMSG, fatal_error_signal); #endif /* AIX */ } @@ -2041,7 +2040,7 @@ /* There is a tendency for a SIGIO signal to arrive within exit, and cause a SIGHUP because the input descriptor is already closed. */ unrequest_sigio (); - signal (SIGIO, SIG_IGN); + set_sighandler (SIGIO, SIG_IGN); #endif #ifdef WINDOWSNT === modified file 'src/emacsgtkfixed.c' --- src/emacsgtkfixed.c 2012-04-23 07:34:29 +0000 +++ src/emacsgtkfixed.c 2012-09-01 21:37:07 +0000 @@ -21,7 +21,6 @@ #include <config.h> #include "emacsgtkfixed.h" -#include <signal.h> #include <stdio.h> #include <setjmp.h> #include "lisp.h" === modified file 'src/floatfns.c' --- src/floatfns.c 2012-07-17 02:56:00 +0000 +++ src/floatfns.c 2012-09-01 21:37:07 +0000 @@ -48,7 +48,6 @@ */ #include <config.h> -#include <signal.h> #include <setjmp.h> #include "lisp.h" #include "syssignal.h" @@ -955,13 +954,14 @@ fatal_error_signal (signo); #ifdef BSD_SYSTEM - sigsetmask (SIGEMPTYMASK); + sigfree (); #else /* Must reestablish handler each time it is called. */ - signal (SIGILL, float_error); + set_sighandler (SIGILL, float_error); #endif /* BSD_SYSTEM */ - SIGNAL_THREAD_CHECK (signo); + if (forwarded_signal (signo)) + return; in_float = 0; xsignal1 (Qarith_error, float_error_arg); @@ -1007,7 +1007,7 @@ init_floatfns (void) { #ifdef FLOAT_CATCH_SIGILL - signal (SIGILL, float_error); + set_sighandler (SIGILL, float_error); #endif in_float = 0; } === modified file 'src/gtkutil.c' --- src/gtkutil.c 2012-08-30 16:07:44 +0000 +++ src/gtkutil.c 2012-09-01 21:37:07 +0000 @@ -21,7 +21,6 @@ #ifdef USE_GTK #include <float.h> -#include <signal.h> #include <stdio.h> #include <setjmp.h> @@ -1979,7 +1978,7 @@ /* I really don't know why this is needed, but without this the GLIBC add on library linuxthreads hangs when the Gnome file chooser backend creates threads. */ - sigblock (sigmask (__SIGRTMIN)); + block_signal (__SIGRTMIN); #endif /* HAVE_PTHREAD */ #ifdef HAVE_GTK_FILE_SELECTION_NEW @@ -2001,7 +2000,7 @@ filesel_done = xg_dialog_run (f, w); #if defined (HAVE_PTHREAD) && defined (__SIGRTMIN) - sigunblock (sigmask (__SIGRTMIN)); + unblock_signal (__SIGRTMIN); #endif if (filesel_done == GTK_RESPONSE_OK) @@ -2061,7 +2060,7 @@ Lisp_Object font = Qnil; #if defined (HAVE_PTHREAD) && defined (__SIGRTMIN) - sigblock (sigmask (__SIGRTMIN)); + block_signal (__SIGRTMIN); #endif /* HAVE_PTHREAD */ w = gtk_font_chooser_dialog_new @@ -2090,7 +2089,7 @@ done = xg_dialog_run (f, w); #if defined (HAVE_PTHREAD) && defined (__SIGRTMIN) - sigunblock (sigmask (__SIGRTMIN)); + unblock_signal (__SIGRTMIN); #endif if (done == GTK_RESPONSE_OK) === modified file 'src/keyboard.c' --- src/keyboard.c 2012-09-01 06:38:52 +0000 +++ src/keyboard.c 2012-09-01 21:37:07 +0000 @@ -21,7 +21,6 @@ #define KEYBOARD_INLINE EXTERN_INLINE -#include <signal.h> #include <stdio.h> #include <setjmp.h> #include "lisp.h" @@ -3661,7 +3660,7 @@ hold_keyboard_input (); #ifdef SIGIO if (!noninteractive) - signal (SIGIO, SIG_IGN); + set_sighandler (SIGIO, SIG_IGN); #endif stop_polling (); } @@ -3833,7 +3832,7 @@ unhold_keyboard_input (); #ifdef SIGIO if (!noninteractive) - signal (SIGIO, input_available_signal); + set_sighandler (SIGIO, input_available_signal); #endif /* SIGIO */ start_polling (); } @@ -6782,10 +6781,10 @@ #ifdef SIGIO if (interrupt_input) { - SIGMASKTYPE mask; - mask = sigblock (sigmask (SIGIO)); + sigset_t mask; + get_sigmask_block_signal (&mask, SIGIO); read_avail_input (expected); - sigsetmask (mask); + set_sigmask (&mask); } else #ifdef POLL_FOR_INPUT @@ -6794,10 +6793,10 @@ it's always set. */ if (!interrupt_input && poll_suppress_count == 0) { - SIGMASKTYPE mask; - mask = sigblock (sigmask (SIGALRM)); + sigset_t mask; + get_sigmask_block_signal (&mask, SIGALRM); read_avail_input (expected); - sigsetmask (mask); + set_sigmask (&mask); } else #endif @@ -6833,10 +6832,10 @@ #ifdef SIGIO if (interrupt_input) { - SIGMASKTYPE mask; - mask = sigblock (sigmask (SIGIO)); + sigset_t mask; + get_sigmask_block_signal (&mask, SIGIO); kbd_buffer_store_event (&event); - sigsetmask (mask); + set_sigmask (&mask); } else #endif @@ -7241,7 +7240,8 @@ { /* Must preserve main program's value of errno. */ int old_errno = errno; - SIGNAL_THREAD_CHECK (signo); + if (forwarded_signal (signo)) + return; #ifdef SYNC_INPUT interrupt_input_pending = 1; @@ -7311,7 +7311,7 @@ p->next = user_signals; user_signals = p; - signal (sig, handle_user_signal); + set_sighandler (sig, handle_user_signal); } static void @@ -7321,7 +7321,8 @@ struct user_signal_info *p; const char *special_event_name = NULL; - SIGNAL_THREAD_CHECK (sig); + if (forwarded_signal (sig)) + return; if (SYMBOLP (Vdebug_on_event)) special_event_name = SSDATA (SYMBOL_NAME (Vdebug_on_event)); @@ -7382,7 +7383,7 @@ for (p = user_signals; p; p = p->next) if (p->npending > 0) { - SIGMASKTYPE mask; + sigset_t mask; if (nstored == 0) { @@ -7392,7 +7393,7 @@ } nstored += p->npending; - mask = sigblock (sigmask (p->sig)); + get_sigmask_block_signal (&mask, p->sig); do { buf.code = p->sig; @@ -7400,7 +7401,7 @@ p->npending--; } while (p->npending > 0); - sigsetmask (mask); + set_sigmask (&mask); } return nstored; @@ -8445,7 +8446,7 @@ /* Append entries from tool_bar_item_properties to the end of tool_bar_items_vector. */ - vcopy (tool_bar_items_vector, ntool_bar_items, + vcopy (tool_bar_items_vector, ntool_bar_items, XVECTOR (tool_bar_item_properties)->contents, TOOL_BAR_ITEM_NSLOTS); ntool_bar_items += TOOL_BAR_ITEM_NSLOTS; } @@ -10784,7 +10785,8 @@ int old_errno = errno; struct terminal *terminal; - SIGNAL_THREAD_CHECK (signalnum); + if (forwarded_signal (signalnum)) + return; /* See if we have an active terminal on our controlling tty. */ terminal = get_named_tty ("/dev/tty"); @@ -10841,7 +10843,7 @@ /* 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. */ - sigblock (sigmask (SIGINT)); + block_signal (SIGINT); fflush (stdout); reset_all_sys_modes (); @@ -11405,17 +11407,17 @@ SIGINT. There is special code in interrupt_signal to exit Emacs on SIGINT when there are no termcap frames on the controlling terminal. */ - signal (SIGINT, interrupt_signal); + set_sighandler (SIGINT, interrupt_signal); #ifndef DOS_NT /* For systems with SysV TERMIO, C-g is set up for both SIGINT and SIGQUIT and we can't tell which one it will give us. */ - signal (SIGQUIT, interrupt_signal); + set_sighandler (SIGQUIT, interrupt_signal); #endif /* not DOS_NT */ } /* Note SIGIO has been undef'd if FIONREAD is missing. */ #ifdef SIGIO if (!noninteractive) - signal (SIGIO, input_available_signal); + set_sighandler (SIGIO, input_available_signal); #endif /* SIGIO */ /* Use interrupt input by default, if it works and noninterrupt input === modified file 'src/nsfns.m' --- src/nsfns.m 2012-08-17 23:38:43 +0000 +++ src/nsfns.m 2012-09-01 21:37:07 +0000 @@ -30,7 +30,6 @@ interpretation of even the system includes. */ #include <config.h> -#include <signal.h> #include <math.h> #include <setjmp.h> #include <c-strcase.h> === modified file 'src/process.c' --- src/process.c 2012-09-01 06:38:52 +0000 +++ src/process.c 2012-09-01 21:37:07 +0000 @@ -23,7 +23,6 @@ #define PROCESS_INLINE EXTERN_INLINE -#include <signal.h> #include <stdio.h> #include <errno.h> #include <setjmp.h> @@ -1766,12 +1765,6 @@ int xforkin = forkin; int xforkout = forkout; -#if 0 /* This was probably a mistake--it duplicates code later on, - but fails to handle all the cases. */ - /* Make sure SIGCHLD is not blocked in the child. */ - sigsetmask (SIGEMPTYMASK); -#endif - /* Make the pty be the controlling terminal of the process. */ #ifdef HAVE_PTYS /* First, disconnect its current controlling terminal. */ @@ -1880,18 +1873,18 @@ /* On AIX, we've disabled SIGHUP above once we start a child on a pty. Now reenable it in the child, so it will die when we want it to. */ if (pty_flag) - signal (SIGHUP, SIG_DFL); + set_sighandler (SIGHUP, SIG_DFL); #endif #endif /* HAVE_PTYS */ - signal (SIGINT, SIG_DFL); - signal (SIGQUIT, SIG_DFL); + set_sighandler (SIGINT, SIG_DFL); + set_sighandler (SIGQUIT, SIG_DFL); /* GConf causes us to ignore SIGPIPE, make sure it is restored in the child. */ - signal (SIGPIPE, SIG_DFL); + set_sighandler (SIGPIPE, SIG_DFL); /* Stop blocking signals in the child. */ - pthread_sigmask (SIG_SETMASK, &procmask, 0); + set_sigmask (&procmask); if (pty_flag) child_setup_tty (xforkout); @@ -1982,7 +1975,7 @@ #endif #endif /* HAVE_WORKING_VFORK */ /* Stop blocking signals in the parent. */ - pthread_sigmask (SIG_SETMASK, &procmask, 0); + set_sigmask (&procmask); /* Now generate the error if vfork failed. */ if (pid < 0) @@ -5439,8 +5432,9 @@ static void send_process_trap (int ignore) { - SIGNAL_THREAD_CHECK (SIGPIPE); - sigunblock (sigmask (SIGPIPE)); + if (forwarded_signal (SIGPIPE)) + return; + unblock_signal (SIGPIPE); longjmp (send_process_frame, 1); } @@ -5534,7 +5528,7 @@ struct Lisp_Process *p = XPROCESS (proc); ssize_t rv; struct coding_system *coding; - void (*volatile old_sigpipe) (int); + signal_handler_t volatile old_sigpipe; if (p->raw_status_new) update_status (p); @@ -5673,7 +5667,7 @@ /* Send this batch, using one or more write calls. */ ptrdiff_t written = 0; int outfd = p->outfd; - old_sigpipe = (void (*) (int)) signal (SIGPIPE, send_process_trap); + old_sigpipe = sys_signal (SIGPIPE, send_process_trap); #ifdef DATAGRAM_SOCKETS if (DATAGRAM_CHAN_P (outfd)) { @@ -5684,7 +5678,7 @@ written = rv; else if (errno == EMSGSIZE) { - signal (SIGPIPE, old_sigpipe); + set_sighandler (SIGPIPE, old_sigpipe); report_file_error ("sending datagram", Fcons (proc, Qnil)); } @@ -5709,7 +5703,7 @@ } #endif } - signal (SIGPIPE, old_sigpipe); + set_sighandler (SIGPIPE, old_sigpipe); if (rv < 0) { @@ -5769,7 +5763,7 @@ } else { - signal (SIGPIPE, old_sigpipe); + set_sighandler (SIGPIPE, old_sigpipe); proc = process_sent_to; p = XPROCESS (proc); p->raw_status_new = 0; @@ -6414,7 +6408,8 @@ Lisp_Object proc; struct Lisp_Process *p; - SIGNAL_THREAD_CHECK (signo); + if (forwarded_signal (signo)) + return; while (1) { @@ -7397,7 +7392,7 @@ #ifndef CANNOT_DUMP if (! noninteractive || initialized) #endif - signal (SIGCHLD, sigchld_handler); + set_sighandler (SIGCHLD, sigchld_handler); #endif FD_ZERO (&input_wait_mask); === modified file 'src/sound.c' --- src/sound.c 2012-07-29 08:18:29 +0000 +++ src/sound.c 2012-09-01 21:37:07 +0000 @@ -48,7 +48,6 @@ #include "lisp.h" #include "dispextern.h" #include "atimer.h" -#include <signal.h> #include "syssignal.h" /* END: Common Includes */ @@ -316,7 +315,7 @@ turn_on_atimers (1); #ifdef SIGIO - sigunblock (sigmask (SIGIO)); + unblock_signal (SIGIO); #endif if (saved_errno != 0) error ("%s: %s", msg, strerror (saved_errno)); @@ -736,7 +735,7 @@ troubles. */ turn_on_atimers (0); #ifdef SIGIO - sigblock (sigmask (SIGIO)); + block_signal (SIGIO); #endif val = sd->format; @@ -770,7 +769,7 @@ turn_on_atimers (1); #ifdef SIGIO - sigunblock (sigmask (SIGIO)); + unblock_signal (SIGIO); #endif } @@ -786,7 +785,7 @@ be interrupted by a signal. Block the ones we know to cause troubles. */ #ifdef SIGIO - sigblock (sigmask (SIGIO)); + block_signal (SIGIO); #endif turn_on_atimers (0); @@ -795,7 +794,7 @@ turn_on_atimers (1); #ifdef SIGIO - sigunblock (sigmask (SIGIO)); + unblock_signal (SIGIO); #endif /* Close the device. */ === modified file 'src/sysdep.c' --- src/sysdep.c 2012-09-01 01:13:50 +0000 +++ src/sysdep.c 2012-09-01 21:37:07 +0000 @@ -19,9 +19,9 @@ #include <config.h> +#define SYSSIGNAL_INLINE EXTERN_INLINE #define SYSTIME_INLINE EXTERN_INLINE -#include <signal.h> #include <stdio.h> #include <setjmp.h> #ifdef HAVE_PWD_H @@ -302,31 +302,43 @@ 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. */ - sigsetmask (sigmask (SIGCHLD)); + sigset_t sigchild_mask; + sigemptyset (&sigchild_mask); + sigaddset (&sigchild_mask, SIGCHLD); + set_sigmask (&sigchild_mask); + if (0 > kill (pid, 0)) { - sigsetmask (SIGEMPTYMASK); + sigfree (); kill (getpid (), SIGCHLD); break; } if (wait_debugging) sleep (1); else - sigpause (SIGEMPTYMASK); + { + sigset_t empty_mask; + sigemptyset (&empty_mask); + sigsuspend (&empty_mask); + } #else /* not BSD_SYSTEM, and not HPUX version >= 6 */ #ifdef WINDOWSNT wait (0); break; #else /* not WINDOWSNT */ - sigblock (sigmask (SIGCHLD)); + block_signal (SIGCHLD); errno = 0; if (kill (pid, 0) == -1 && errno == ESRCH) { - sigunblock (sigmask (SIGCHLD)); + unblock_signal (SIGCHLD); break; } - sigsuspend (&empty_mask); + { + sigset_t empty_mask; + sigemptyset (&empty_mask); + sigsuspend (&empty_mask); + } #endif /* not WINDOWSNT */ #endif /* not BSD_SYSTEM, and not HPUX version >= 6 */ if (interruptible) @@ -460,7 +472,7 @@ struct save_signal { int code; - void (*handler) (int); + signal_handler_t handler; }; static void save_signal_handlers (struct save_signal *); @@ -618,8 +630,7 @@ { while (saved_handlers->code) { - saved_handlers->handler - = (void (*) (int)) signal (saved_handlers->code, SIG_IGN); + saved_handlers->handler = sys_signal (saved_handlers->code, SIG_IGN); saved_handlers++; } } @@ -629,7 +640,7 @@ { while (saved_handlers->code) { - signal (saved_handlers->code, saved_handlers->handler); + set_sighandler (saved_handlers->code, saved_handlers->handler); saved_handlers++; } } @@ -690,9 +701,9 @@ return; #ifdef SIGWINCH - sigunblock (sigmask (SIGWINCH)); + unblock_signal (SIGWINCH); #endif - sigunblock (sigmask (SIGIO)); + unblock_signal (SIGIO); interrupts_deferred = 0; } @@ -709,9 +720,9 @@ #endif #ifdef SIGWINCH - sigblock (sigmask (SIGWINCH)); + block_signal (SIGWINCH); #endif - sigblock (sigmask (SIGIO)); + block_signal (SIGIO); interrupts_deferred = 1; } @@ -1473,16 +1484,27 @@ /* POSIX signals support - DJB */ /* Anyone with POSIX signals should have ANSI C declarations */ -sigset_t empty_mask; - #ifndef WINDOWSNT +/* Return the current signal handler for signal number SIGNO. */ signal_handler_t -sys_signal (int signal_number, signal_handler_t action) -{ - struct sigaction new_action, old_action; +sighandler (int signo) +{ + struct sigaction old_action; + sigaction (signo, NULL, &old_action); + return old_action.sa_handler; +} + +/* Copy the old signal action for SIGNO into *OLD_ACTION if OLD_ACTION + is non-null. Then change the signal action to be one whose handler + is HANDLER. */ +static void +get_set_sighandler (int signo, struct sigaction *old_action, + signal_handler_t handler) +{ + struct sigaction new_action; sigemptyset (&new_action.sa_mask); - new_action.sa_handler = action; + new_action.sa_handler = handler; new_action.sa_flags = 0; #if defined (SA_RESTART) /* Emacs mostly works better with restartable system services. If this @@ -1502,54 +1524,26 @@ # endif new_action.sa_flags = SA_RESTART; #endif - sigaction (signal_number, &new_action, &old_action); - return (old_action.sa_handler); + sigaction (signo, &new_action, old_action); +} + +/* Change SIGNO's handler to be HANDLER. */ +void +set_sighandler (int signo, signal_handler_t handler) +{ + get_set_sighandler (signo, NULL, handler); +} + +/* Change SIGNO's handler to be HANDLER. Return the old handler. */ +signal_handler_t +sys_signal (int signo, signal_handler_t handler) +{ + struct sigaction old_action; + get_set_sighandler (signo, &old_action, handler); + return old_action.sa_handler; } #endif /* WINDOWSNT */ - -#ifndef __GNUC__ -/* If we're compiling with GCC, we don't need this function, since it - can be written as a macro. */ -sigset_t -sys_sigmask (int sig) -{ - sigset_t mask; - sigemptyset (&mask); - sigaddset (&mask, sig); - return mask; -} -#endif - -/* I'd like to have these guys return pointers to the mask storage in here, - but there'd be trouble if the code was saving multiple masks. I'll be - safe and pass the structure. It normally won't be more than 2 bytes - anyhow. - DJB */ - -sigset_t -sys_sigblock (sigset_t new_mask) -{ - sigset_t old_mask; - pthread_sigmask (SIG_BLOCK, &new_mask, &old_mask); - return (old_mask); -} - -sigset_t -sys_sigunblock (sigset_t new_mask) -{ - sigset_t old_mask; - pthread_sigmask (SIG_UNBLOCK, &new_mask, &old_mask); - return (old_mask); -} - -sigset_t -sys_sigsetmask (sigset_t new_mask) -{ - sigset_t old_mask; - pthread_sigmask (SIG_SETMASK, &new_mask, &old_mask); - return (old_mask); -} - \f #if !defined HAVE_STRSIGNAL && !HAVE_DECL_SYS_SIGLIST static char *my_sys_siglist[NSIG]; @@ -1562,8 +1556,6 @@ void init_signals (void) { - sigemptyset (&empty_mask); - #if !defined HAVE_STRSIGNAL && !HAVE_DECL_SYS_SIGLIST if (! initialized) { === modified file 'src/syssignal.h' --- src/syssignal.h 2012-07-13 01:19:06 +0000 +++ src/syssignal.h 2012-09-01 21:37:07 +0000 @@ -17,7 +17,8 @@ You should have received a copy of the GNU General Public License along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */ -extern void init_signals (void); +#include <signal.h> +#include <stdbool.h> #ifdef HAVE_PTHREAD #include <pthread.h> @@ -26,63 +27,65 @@ #define FORWARD_SIGNAL_TO_MAIN_THREAD #endif -/* Don't #include <signal.h>. That header should always be #included - before "config.h", because some configuration files (like s/hpux.h) - indicate that SIGIO doesn't work by #undef-ing SIGIO. If this file - #includes <signal.h>, then that will re-#define SIGIO and confuse - things. */ -/* XXX This is not correct anymore, there is a BROKEN_SIGIO macro. */ - -#define SIGMASKTYPE sigset_t - -#define SIGEMPTYMASK (empty_mask) -extern sigset_t empty_mask; - -/* POSIX pretty much destroys any possibility of writing sigmask as a - macro in standard C. We always define our own version because the - predefined macro in Glibc 2.1 is only provided for compatibility for old - programs that use int as signal mask type. */ -#undef sigmask -#ifdef __GNUC__ -#define sigmask(SIG) \ - ({ \ - sigset_t _mask; \ - sigemptyset (&_mask); \ - sigaddset (&_mask, SIG); \ - _mask; \ - }) -#else /* ! defined (__GNUC__) */ -extern sigset_t sys_sigmask (); -#define sigmask(SIG) (sys_sigmask (SIG)) -#endif /* ! defined (__GNUC__) */ - -#undef sigpause -#define sigpause(MASK) sigsuspend (&(MASK)) - -#define sigblock(SIG) sys_sigblock (SIG) -#define sigunblock(SIG) sys_sigunblock (SIG) -#ifndef sigsetmask -#define sigsetmask(SIG) sys_sigsetmask (SIG) -#endif -#undef signal -#define signal(SIG,ACT) sys_signal(SIG,ACT) - -/* Whether this is what all systems want or not, this is what - appears to be assumed in the source, for example data.c:arith_error. */ +INLINE_HEADER_BEGIN +#ifndef SYSSIGNAL_INLINE +# define SYSSIGNAL_INLINE INLINE +#endif + +extern void init_signals (void) +#if defined HAVE_STRSIGNAL || HAVE_DECL_SYS_SIGLIST + ATTRIBUTE_CONST +#endif + ; + +SYSSIGNAL_INLINE void +get_sigmask_block_signal (sigset_t *old_mask, int sig) +{ + sigset_t mask; + sigemptyset (&mask); + sigaddset (&mask, sig); + pthread_sigmask (SIG_BLOCK, &mask, old_mask); +} + +SYSSIGNAL_INLINE void +block_signal (int sig) +{ + get_sigmask_block_signal (NULL, sig); +} + +SYSSIGNAL_INLINE void +unblock_signal (int sig) +{ + sigset_t mask; + sigemptyset (&mask); + sigaddset (&mask, sig); + pthread_sigmask (SIG_UNBLOCK, &mask, NULL); +} + +SYSSIGNAL_INLINE void +set_sigmask (sigset_t *mask) +{ + pthread_sigmask (SIG_SETMASK, mask, NULL); +} + +SYSSIGNAL_INLINE void +sigfree (void) +{ + sigset_t empty_mask; + sigemptyset (&empty_mask); + set_sigmask (&empty_mask); +} + typedef void (*signal_handler_t) (int); -signal_handler_t sys_signal (int signal_number, signal_handler_t action); -sigset_t sys_sigblock (sigset_t new_mask); -sigset_t sys_sigunblock (sigset_t new_mask); -sigset_t sys_sigsetmask (sigset_t new_mask); +signal_handler_t sighandler (int); +void set_sighandler (int, signal_handler_t); +signal_handler_t sys_signal (int, signal_handler_t); + #if ! (defined TIOCNOTTY || defined USG5 || defined CYGWIN) _Noreturn void croak (char *); #endif -#define sys_sigdel(MASK,SIG) sigdelset (&MASK,SIG) - -#define sigfree() sigsetmask (SIGEMPTYMASK) - #if defined (SIGIO) && defined (BROKEN_SIGIO) # undef SIGIO #endif @@ -97,12 +100,8 @@ #undef SIGPTY #endif - -/* FIXME? Emacs only defines NSIG_MINIMUM on some platforms? */ #if NSIG < NSIG_MINIMUM -# ifdef NSIG -# undef NSIG -# endif +# undef NSIG # define NSIG NSIG_MINIMUM #endif @@ -133,24 +132,20 @@ #ifdef FORWARD_SIGNAL_TO_MAIN_THREAD extern pthread_t main_thread; -#define SIGNAL_THREAD_CHECK(signo) \ - do { \ - if (!pthread_equal (pthread_self (), main_thread)) \ - { \ - /* POSIX says any thread can receive the signal. On GNU/Linux \ - that is not true, but for other systems (FreeBSD at least) \ - it is. So direct the signal to the correct thread and block \ - it from this thread. */ \ - sigset_t new_mask; \ - \ - sigemptyset (&new_mask); \ - sigaddset (&new_mask, signo); \ - pthread_sigmask (SIG_BLOCK, &new_mask, 0); \ - pthread_kill (main_thread, signo); \ - return; \ - } \ - } while (0) - -#else /* not FORWARD_SIGNAL_TO_MAIN_THREAD */ -#define SIGNAL_THREAD_CHECK(signo) -#endif /* not FORWARD_SIGNAL_TO_MAIN_THREAD */ +#endif + +SYSSIGNAL_INLINE bool +forwarded_signal (int signo) +{ +#ifdef FORWARD_SIGNAL_TO_MAIN_THREAD + if (! pthread_equal (pthread_self (), main_thread)) + { + block_signal (signo); + pthread_kill (main_thread, signo); + return 1; + } +#endif + return 0; +} + +INLINE_HEADER_END === modified file 'src/term.c' --- src/term.c 2012-08-31 10:53:19 +0000 +++ src/term.c 2012-09-01 21:37:07 +0000 @@ -25,7 +25,6 @@ #include <sys/file.h> #include <sys/time.h> #include <unistd.h> -#include <signal.h> #include <setjmp.h> #include "lisp.h" @@ -2932,7 +2931,7 @@ no_controlling_tty = 1; #else #ifdef TIOCNOTTY /* Try BSD ioctls. */ - sigblock (sigmask (SIGTTOU)); + block_signal (SIGTTOU); fd = emacs_open (DEV_TTY, O_RDWR, 0); if (fd != -1 && ioctl (fd, TIOCNOTTY, 0) != -1) { @@ -2940,7 +2939,7 @@ } if (fd != -1) emacs_close (fd); - sigunblock (sigmask (SIGTTOU)); + unblock_signal (SIGTTOU); #else /* Unknown system. */ croak (); @@ -3074,9 +3073,9 @@ /* On some systems, tgetent tries to access the controlling terminal. */ - sigblock (sigmask (SIGTTOU)); + block_signal (SIGTTOU); status = tgetent (tty->termcap_term_buffer, terminal_type); - sigunblock (sigmask (SIGTTOU)); + unblock_signal (SIGTTOU); if (status < 0) { === modified file 'src/widget.c' --- src/widget.c 2012-07-10 21:48:34 +0000 +++ src/widget.c 2012-09-01 21:37:07 +0000 @@ -50,9 +50,6 @@ #include <X11/ShellP.h> #include "../lwlib/lwlib.h" -#include <signal.h> -#include "syssignal.h" - #include "character.h" #include "font.h" === modified file 'src/xmenu.c' --- src/xmenu.c 2012-08-17 21:52:15 +0000 +++ src/xmenu.c 2012-09-01 21:37:07 +0000 @@ -32,11 +32,6 @@ #include <config.h> -#if 0 /* Why was this included? And without syssignal.h? */ -/* On 4.3 this loses if it comes after xterm.h. */ -#include <signal.h> -#endif - #include <stdio.h> #include <setjmp.h> === modified file 'src/xterm.c' --- src/xterm.c 2012-08-25 20:31:04 +0000 +++ src/xterm.c 2012-09-01 21:37:07 +0000 @@ -21,7 +21,6 @@ /* Xt features made by Fred Pierresteguy. */ #include <config.h> -#include <signal.h> #include <stdio.h> #include <setjmp.h> @@ -29,9 +28,6 @@ #include "lisp.h" #include "blockinput.h" - -/* Need syssignal.h for various externs and definitions that may be required - by some configurations for calls to signal later in this source file. */ #include "syssignal.h" /* This may include sys/types.h, and that somehow loses @@ -7773,7 +7769,7 @@ #ifdef USG /* USG systems forget handlers when they are used; must reestablish each time */ - signal (signalnum, x_connection_signal); + set_sighandler (signalnum, x_connection_signal); #endif /* USG */ } @@ -7884,9 +7880,9 @@ /* Ordinary stack unwind doesn't deal with these. */ #ifdef SIGIO - sigunblock (sigmask (SIGIO)); + unblock_signal (SIGIO); #endif - sigunblock (sigmask (SIGALRM)); + unblock_signal (SIGALRM); TOTALLY_UNBLOCK_INPUT; unbind_to (idx, Qnil); @@ -10814,7 +10810,7 @@ XSetErrorHandler (x_error_handler); XSetIOErrorHandler (x_io_error_quitter); - signal (SIGPIPE, x_connection_signal); + set_sighandler (SIGPIPE, x_connection_signal); } ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-01 22:38 bug#12327: Signal-handler cleanup for Emacs Paul Eggert @ 2012-09-02 17:51 ` Eli Zaretskii 2012-09-02 18:37 ` Eli Zaretskii 2012-09-02 19:01 ` Paul Eggert 2012-09-07 1:35 ` Paul Eggert 1 sibling, 2 replies; 23+ messages in thread From: Eli Zaretskii @ 2012-09-02 17:51 UTC (permalink / raw) To: Paul Eggert; +Cc: lekktu, 12327 > Date: Sat, 01 Sep 2012 15:38:15 -0700 > From: Paul Eggert <eggert@cs.ucla.edu> > CC: Eli Zaretskii <eliz@gnu.org>, Juanma Barranquero <lekktu@gmail.com> > > Here's a patch to clean up Emacs's signal-handling code somewhat. > As described below, it shrinks the size of Emacs's text segment > by 0.6% on my platform. It also removes a few dozen lines from > the source code. FWIW, I cannot see any advantage in these changes. They just replace one system of macros by another. The advantage of the current code is that it invokes widely known standard APIs, such as 'signal', 'sigemptyset', and 'sigblock' (never mind that we redefine some of them to call their more modern equivalents: that doesn't impede code reading and understanding in any way). OTOH, the changes you propose replace that with APIs that AFAIK are not known to anyone and, with a couple of exceptions, not used anywhere else. I could go with replacing 'signal' etc. with their modern Posix replacements, such as 'sigaction', directly in the code. That will clean up old APIs while still maintaining clarity for anyone who has ever written signal handlers. But I fail to see any good reasons for changes that, e.g., hide a pair of calls to well-known library functions, such as 'sigemptyset' and 'sigaddset', behind 'sigsetmask' (which AFAIK is a BSD-ism), or behind newly-invented functions such as 'get_sigmask_block_signal'. As for 0.6% reduction in the size of .text: what kind of humongous .text size do you have that makes 0.6% a significant value? On 2 different platforms to which I have access, one of them a x86_64 GNU/Linux, I see ~3.5MB in an unoptimized build and about half that much in an optimized one, which makes the savings around 2KB, too small to justify any change whatsoever. What am I missing? ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-02 17:51 ` Eli Zaretskii @ 2012-09-02 18:37 ` Eli Zaretskii 2012-09-02 19:01 ` Paul Eggert 1 sibling, 0 replies; 23+ messages in thread From: Eli Zaretskii @ 2012-09-02 18:37 UTC (permalink / raw) To: eggert, lekktu; +Cc: 12327 > Date: Sun, 02 Sep 2012 20:51:51 +0300 > From: Eli Zaretskii <eliz@gnu.org> > Cc: lekktu@gmail.com, 12327@debbugs.gnu.org > > As for 0.6% reduction in the size of .text: what kind of humongous > .text size do you have that makes 0.6% a significant value? On 2 > different platforms to which I have access, one of them a x86_64 > GNU/Linux, I see ~3.5MB in an unoptimized build and about half that > much in an optimized one, which makes the savings around 2KB, too > small to justify any change whatsoever. ^^^ Err, make that 21KB, still too small. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-02 17:51 ` Eli Zaretskii 2012-09-02 18:37 ` Eli Zaretskii @ 2012-09-02 19:01 ` Paul Eggert 2012-09-02 21:20 ` Eli Zaretskii 1 sibling, 1 reply; 23+ messages in thread From: Paul Eggert @ 2012-09-02 19:01 UTC (permalink / raw) To: Eli Zaretskii; +Cc: lekktu, 12327 On 09/02/2012 10:51 AM, Eli Zaretskii wrote: > that doesn't impede code reading and understanding in any way Sure it does, because Emacs defines symbols incompatibly with their normal meanings. E.g., Emacs redefines 'sigblock' to be a macro whose type signature is incompatible with the 'sigblock' that is found on GNU/Linux. This sort of thing makes Emacs's signal-handling code more confusing than it needs to be, and is the main motivation for the patch. > I could go with replacing 'signal' etc. with their modern Posix > replacements, such as 'sigaction', directly in the code. That would make the mainline code significantly less readable. Instead of this, for example: unblock_signal (SIGPIPE); send_process_trap would have to do something like this: sigset_t mask; ... sigemptyset (&mask); sigaddset (&mask, SIGPIPE); pthread_sigmask (SIG_UNBLOCK, &mask, NULL); which is not an improvement. I suspect the main reason that Emacs currently uses macros reminiscent but incompatible with the obsolete 4.2BSD interface, rather than the standard POSIX interface, is because the 4.2BSD interface was simpler. Unfortunately, the 4.2BSD interface does not scale well to modern systems with lots of signals. > I fail to see any good reasons for changes that, e.g., hide a > pair of calls to well-known library functions, such as 'sigemptyset' > and 'sigaddset', behind 'sigsetmask' (which AFAIK is a BSD-ism) There must be some confusion here. The new code does not use sigsetmask. > As for 0.6% reduction in the size of .text: what kind of humongous > .text size do you have that makes 0.6% a significant value? Every little bit helps, no? But the main point of measuring text size is as a rough gauge of the efficiency implications. If the text segment had grown, that would have been a bad sign. The fact that it shrank is a good sign. Fewer instructions mean faster CPU performance, partly due to lower pressure on the instruction cache. I didn't bother to measure CPU performance earlier, since that takes more work, but I just now did that with an artificial benchmark that simply blocks and then unblocks SIGCHLD, and on my platform the proposed patch speeds up this benchmark by 15%. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-02 19:01 ` Paul Eggert @ 2012-09-02 21:20 ` Eli Zaretskii 2012-09-03 8:16 ` Paul Eggert 0 siblings, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2012-09-02 21:20 UTC (permalink / raw) To: Paul Eggert; +Cc: lekktu, 12327 > Date: Sun, 02 Sep 2012 12:01:11 -0700 > From: Paul Eggert <eggert@cs.ucla.edu> > CC: 12327@debbugs.gnu.org, lekktu@gmail.com > > On 09/02/2012 10:51 AM, Eli Zaretskii wrote: > > > that doesn't impede code reading and understanding in any way > > Sure it does, because Emacs defines symbols incompatibly with their > normal meanings. E.g., Emacs redefines 'sigblock' to be a macro > whose type signature is incompatible with the 'sigblock' > that is found on GNU/Linux. If that's the problem, then fix that, by changing the type. Why do more than is strictly needed? Gratuitous changes are BAAAD! > > I could go with replacing 'signal' etc. with their modern Posix > > replacements, such as 'sigaction', directly in the code. > > That would make the mainline code significantly less readable. > Instead of this, for example: > > unblock_signal (SIGPIPE); > > send_process_trap would have to do something like this: > > sigset_t mask; > ... > sigemptyset (&mask); > sigaddset (&mask, SIGPIPE); > pthread_sigmask (SIG_UNBLOCK, &mask, NULL); > > which is not an improvement. Of course, it's an improvement! That kind of code is a pattern programmers are used to see when signals are being masked. By contrast, when I see a call to unblock_signal, I need to look it up. > I suspect the main reason that Emacs currently uses macros reminiscent > but incompatible with the obsolete 4.2BSD interface, rather than the > standard POSIX interface, is because the 4.2BSD interface was simpler. No, it's just because the BSD interface was there first. > Unfortunately, the 4.2BSD interface does not scale well to modern > systems with lots of signals. I suggested to use sigaction and friends, not go back to the old APIs. > > I fail to see any good reasons for changes that, e.g., hide a > > pair of calls to well-known library functions, such as 'sigemptyset' > > and 'sigaddset', behind 'sigsetmask' (which AFAIK is a BSD-ism) > > There must be some confusion here. Yeah, did I say the new code is confusing? > > As for 0.6% reduction in the size of .text: what kind of humongous > > .text size do you have that makes 0.6% a significant value? > > Every little bit helps, no? Not every little bit can be justified by such non-trivial changes. Changes come at a cost, which should be justified. > But the main point of measuring text size > is as a rough gauge of the efficiency implications. If the text > segment had grown, that would have been a bad sign. Not necessarily. And certainly not at fractions of a percent. > Fewer instructions mean faster CPU performance, partly due to lower > pressure on the instruction cache. This all smells premature optimization to me. Text segment size has no direct relation to pressure on the instruction cache. What matters is the structure of the code in terms of branches and loops, and its locality. The size alone is almost meaningless. I'm sure you know that. > I didn't bother to measure CPU performance earlier, since that takes > more work, but I just now did that with an artificial benchmark that > simply blocks and then unblocks SIGCHLD, and on my platform the > proposed patch speeds up this benchmark by 15%. A meaningless benchmark, IMO. You need to measure an Emacs loop that does these block and unblock operations (if there is such a loop), and see the effect on that. _That_ would be interesting. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-02 21:20 ` Eli Zaretskii @ 2012-09-03 8:16 ` Paul Eggert 2012-09-03 8:49 ` Andreas Schwab 2012-09-03 15:32 ` Eli Zaretskii 0 siblings, 2 replies; 23+ messages in thread From: Paul Eggert @ 2012-09-03 8:16 UTC (permalink / raw) To: Eli Zaretskii; +Cc: lekktu, 12327 [-- Attachment #1: Type: text/plain, Size: 967 bytes --] On 09/02/2012 02:20 PM, Eli Zaretskii wrote: > If that's the problem, then fix that, by changing the type. If I understand this suggestion correctly, it's not practical. GNU/Linux sigblock returns 'int', but its signal masks require 1024 bits, too wide for 'int'. This is why sigblock has been removed from POSIX and is documented as obsolete in GNU/Linux. > Changes come at a cost, which should be justified. My point was that the new version is smaller and faster. How much smaller and faster doesn't matter much, as the primary justification for the change is to reduce the confusion caused by the current setup. > I could go with replacing 'signal' etc. with their modern Posix > replacements OK, thanks, I reworked the patch to implement that suggestion. I also modified it a bit in other ways, to boost the performance a bit (as that suggestion caused a small performance hit). The result is attached. I expect the Windows port will need a few tweaks. [-- Attachment #2: syssignal.txt --] [-- Type: text/plain, Size: 49938 bytes --] === modified file 'ChangeLog' --- ChangeLog 2012-09-02 11:13:24 +0000 +++ ChangeLog 2012-09-03 01:55:39 +0000 @@ -1,3 +1,10 @@ +2012-09-03 Paul Eggert <eggert@cs.ucla.edu> + + Signal-handler cleanup (Bug#12327). + * configure.ac (PTY_OPEN, PTY_TTY_NAME_SPRINTF): + Adjust to syssignal.h changes. + (SIGNAL_H_AB): Remove; no longer needed. + 2012-09-02 Jan Djärv <jan.h.d@swipnet.se> * configure.ac (HAVE_GOBJECT): Check for gobject-2.0 (Bug#12332). === modified file 'configure.ac' --- configure.ac 2012-09-02 11:13:24 +0000 +++ configure.ac 2012-09-03 01:56:28 +0000 @@ -3450,7 +3450,7 @@ cygwin ) AC_DEFINE(PTY_ITERATION, [int i; for (i = 0; i < 1; i++)]) dnl multi-line AC_DEFINEs are hard. :( - AC_DEFINE(PTY_OPEN, [ do { int dummy; SIGMASKTYPE mask; mask = sigblock (sigmask (SIGCHLD)); if (-1 == openpty (&fd, &dummy, pty_name, 0, 0)) fd = -1; sigsetmask (mask); if (fd >= 0) emacs_close (dummy); } while (0)]) + AC_DEFINE(PTY_OPEN, [ do { int dummy; sigset_t blocked, procmask; sigemptyset (&blocked); sigaddset (&blocked, SIGCHLD); pthread_sigmask (SIG_BLOCK, &blocked, &procmask); if (-1 == openpty (&fd, &dummy, pty_name, 0, 0)) fd = -1; pthread_sigmask (SIG_SETMASK, &procmask, 0); if (fd >= 0) emacs_close (dummy); } while (0)]) AC_DEFINE(PTY_NAME_SPRINTF, []) AC_DEFINE(PTY_TTY_NAME_SPRINTF, []) ;; @@ -3479,7 +3479,7 @@ AC_DEFINE(PTY_ITERATION, [int i; for (i = 0; i < 1; i++)]) dnl Note that grantpt and unlockpt may fork. We must block SIGCHLD dnl to prevent sigchld_handler from intercepting the child's death. - AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptyname; sigblock (sigmask (SIGCHLD)); if (grantpt (fd) == -1 || unlockpt (fd) == -1 || !(ptyname = ptsname(fd))) { sigunblock (sigmask (SIGCHLD)); close (fd); return -1; } snprintf (pty_name, sizeof pty_name, "%s", ptyname); sigunblock (sigmask (SIGCHLD)); }]) + AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptyname = 0; sigset_t blocked, procmask; sigemptyset (&blocked); sigaddset (&blocked, SIGCHLD); pthread_sigmask (SIG_BLOCK, &blocked, &procmask); if (grantpt (fd) != -1 && unlockpt (fd) != -1) ptyname = ptsname(fd); pthread_sigmask (SIG_SETMASK, &procmask, 0); if (!ptyname) { close (fd); return -1; } snprintf (pty_name, sizeof pty_name, "%s", ptyname); }]) dnl if HAVE_POSIX_OPENPT if test "x$ac_cv_func_posix_openpt" = xyes; then AC_DEFINE(PTY_OPEN, [fd = posix_openpt (O_RDWR | O_NOCTTY)]) @@ -3524,18 +3524,15 @@ ;; sol2* ) - dnl Uses sigblock/sigunblock rather than sighold/sigrelse, - dnl which appear to be BSD4.1 specific. It may also be appropriate - dnl for SVR4.x (x<2) but I'm not sure. fnf@cygnus.com dnl On SysVr4, grantpt(3) forks a subprocess, so keep sigchld_handler() dnl from intercepting that death. If any child but grantpt's should die dnl within, it should be caught after sigrelse(2). - AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptsname (int), *ptyname; sigblock (sigmask (SIGCLD)); if (grantpt (fd) == -1) { emacs_close (fd); return -1; } sigunblock (sigmask (SIGCLD)); if (unlockpt (fd) == -1) { emacs_close (fd); return -1; } if (!(ptyname = ptsname (fd))) { emacs_close (fd); return -1; } snprintf (pty_name, sizeof pty_name, "%s", ptyname); }]) + AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptsname (int), *ptyname; int grantpt_result; sigset_t blocked, procmask; sigemptyset (&blocked); sigaddset (&blocked, SIGCLD); pthread_sigmask (SIG_BLOCK, &blocked, &procmask); grantpt_result = grantpt (fd); pthread_sigmask (SIG_SETMASK, &procmask, 0); if (grantpt_result == -1 || unlockpt (fd) == -1 || !(ptyname = ptsname (fd))) { emacs_close (fd); return -1; } snprintf (pty_name, sizeof pty_name, "%s", ptyname); }]) ;; unixware ) dnl Comments are as per sol2*. - AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptsname (int), *ptyname; sigblock(sigmask(SIGCLD)); if (grantpt(fd) == -1) fatal("could not grant slave pty"); sigunblock(sigmask(SIGCLD)); if (unlockpt(fd) == -1) fatal("could not unlock slave pty"); if (!(ptyname = ptsname(fd))) fatal ("could not enable slave pty"); snprintf (pty_name, sizeof pty_name, "%s", ptyname); }]) + AC_DEFINE(PTY_TTY_NAME_SPRINTF, [{ char *ptsname (int), *ptyname; int grantpt_result; sigset_t blocked, procmask; sigemptyset (&blocked); sigaddset (&blocked, SIGCLD); pthread_sigmask (SIG_BLOCK, &blocked, &procmask); grantpt_result = grantpt (fd); pthread_sigmask (SIG_BLOCK, &blocked, &procmask); if (grantpt_result == -1) fatal("could not grant slave pty"); if (unlockpt(fd) == -1) fatal("could not unlock slave pty"); if (!(ptyname = ptsname(fd))) fatal ("could not enable slave pty"); snprintf (pty_name, sizeof pty_name, "%s", ptyname); }]) ;; esac @@ -3811,13 +3808,6 @@ AC_DEFINE(XOS_NEEDS_TIME_H, 1, [Compensate for a bug in Xos.h on some systems, where it requires time.h.]) ;; - - netbsd | openbsd ) - dnl Greg A. Woods <woods@weird.com> says we must include signal.h - dnl before syssignal.h is included, to work around interface conflicts - dnl that are handled with CPP __RENAME() macro in signal.h. - AC_DEFINE(SIGNAL_H_AHB, 1, [Define if AH_BOTTOM should include signal.h.]) - ;; esac === modified file 'src/ChangeLog' --- src/ChangeLog 2012-09-02 17:10:35 +0000 +++ src/ChangeLog 2012-09-03 08:02:33 +0000 @@ -1,3 +1,62 @@ +2012-09-03 Paul Eggert <eggert@cs.ucla.edu> + + 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 + inefficiencies and confusion. Rewrite these to use + pthread_sigmask etc. without copying signal sets around. Also, + get rid of the confusing macros 'SIGNAL_THREAD_CHECK' and + 'signal', and instead use functions that do not attempt to take + over the system name space. This patch causes Emacs's text + segment to shrink by 0.7% on my platform, Fedora 17 x86-64. + * alloc.c, emacsgtkfixed.c, nsfns.m, widget.c, xmenu.c: + Do not include <signal.h> or "syssignal.h", as these + modules do not use signals. + * atimer.c, callproc.c, data.c, dispnew.c, emacs.c, floatfns.c: + * gtkutil.c, keyboard.c, process.c, sound.c, sysdep.c, term.c, xterm.c: + Do not include <signal.h>, as "syssignal.h" does that for us now. + * atimer.c (sigmask_atimers): New function. + (block_atimers, unblock_atimers): New functions, + replacing the old macros BLOCK_ATIMERS and UNBLOCK_ATIMERS. + All uses replaced. + * conf_post.h [SIGNAL_H_AHB]: Do not include <signal.h>; + no longer needed here. + * emacs.c (main): Inspect existing signal handler with sigaction, + so that there's no need to block and unblock SIGHUP. + * sysdep.c, syssignal.h (SYSSIGNAL_INLINE): New macro. + * sysdep.c (struct save_signal): New member 'action', replacing + old member 'handler'. + (save_signal_handlers, restore_signal_handlers): + Use sigaction instead of 'signal' to save and restore. + (get_set_sighandler, set_sighandler) [!WINDOWSNT]: + New function. All users of 'signal' modified to use set_sighandler + if they're writeonly, and to use sys_signal if they're read+write. + (emacs_sigaction_init, forwarded_signal): New functions. + (sys_signal): Remove. All uses replaced by calls to sigaction + and emacs_sigaction_init, or by direct calls to 'signal'. + (sys_sigmask) [!__GNUC__]: Remove; no longer needed. + (sys_sigblock, sys_sigunblock, sys_sigsetmask): Remove; + all uses replaced by pthread_sigmask etc. calls. + * syssignal.h: Include <signal.h> and <stdbool.h>. + Use INLINE_HEADER_BEGIN, INLINE_HEADER_END. + (emacs_sigaction_init, forwarded_signal): New decls. + (SIGMASKTYPE): Remove. All uses replaced by its definiens, sigset_t. + (SIGEMPTYMASK): Remove; all uses replaced by its definiens, empty_mask. + (sigmask, sys_sigmask): Remove; no longer needed. + (sigpause): Remove. All uses replaced by its definiens, sigsuspend. + (sigblock, sigunblock, sigfree): + (sigsetmask) [!defined sigsetmask]: + Remove. All uses replaced by pthread_sigmask. + (signal): Remove. Its remaining uses (with SIG_DFL and SIG_IGN) + no longer need to be replaced, and its typical old uses + are now done via emacs_sigaction_init and sigaction. + (sys_sigblock, sys_sigunblock, sys_sigsetmask): Remove decls. + (sys_sigdel): Remove; unused. + (NSIG): Remove a FIXME; the code's fine. Remove an unnecessary ifdef. + (SIGNAL_THREAD_CHECK): Remove. All uses replaced by calls to + forwarded_signal. This is cleaner, because the caller can now + see that the check might immediately return from the caller. + 2012-09-02 Paul Eggert <eggert@cs.ucla.edu> * emacs.c, eval.c: Use bool for boolean. === modified file 'src/alloc.c' --- src/alloc.c 2012-09-02 16:56:31 +0000 +++ src/alloc.c 2012-09-03 00:06:25 +0000 @@ -26,8 +26,6 @@ #include <limits.h> /* For CHAR_BIT. */ #include <setjmp.h> -#include <signal.h> - #ifdef HAVE_PTHREAD #include <pthread.h> #endif @@ -42,7 +40,6 @@ #include "keyboard.h" #include "frame.h" #include "blockinput.h" -#include "syssignal.h" #include "termhooks.h" /* For struct terminal. */ #include <setjmp.h> #include <verify.h> === modified file 'src/atimer.c' --- src/atimer.c 2012-08-23 08:27:08 +0000 +++ src/atimer.c 2012-09-03 07:24:03 +0000 @@ -17,7 +17,6 @@ along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */ #include <config.h> -#include <signal.h> #include <stdio.h> #include <setjmp.h> #include "lisp.h" @@ -51,8 +50,24 @@ /* Block/unblock SIGALRM. */ -#define BLOCK_ATIMERS sigblock (sigmask (SIGALRM)) -#define UNBLOCK_ATIMERS sigunblock (sigmask (SIGALRM)) +static void +sigmask_atimers (int how) +{ + sigset_t blocked; + sigemptyset (&blocked); + sigaddset (&blocked, SIGALRM); + pthread_sigmask (how, &blocked, 0); +} +static void +block_atimers (void) +{ + sigmask_atimers (SIG_BLOCK); +} +static void +unblock_atimers (void) +{ + sigmask_atimers (SIG_UNBLOCK); +} /* Function prototypes. */ @@ -111,7 +126,7 @@ t->fn = fn; t->client_data = client_data; - BLOCK_ATIMERS; + block_atimers (); /* Compute the timer's expiration time. */ switch (type) @@ -132,7 +147,7 @@ /* Insert the timer in the list of active atimers. */ schedule_atimer (t); - UNBLOCK_ATIMERS; + unblock_atimers (); /* Arrange for a SIGALRM at the time the next atimer is ripe. */ set_alarm (); @@ -148,7 +163,7 @@ { int i; - BLOCK_ATIMERS; + block_atimers (); for (i = 0; i < 2; ++i) { @@ -175,7 +190,7 @@ } } - UNBLOCK_ATIMERS; + unblock_atimers (); } @@ -206,7 +221,7 @@ void stop_other_atimers (struct atimer *t) { - BLOCK_ATIMERS; + block_atimers (); if (t) { @@ -231,7 +246,7 @@ stopped_atimers = append_atimer_lists (atimers, stopped_atimers); atimers = t; - UNBLOCK_ATIMERS; + unblock_atimers (); } @@ -246,7 +261,7 @@ struct atimer *t = atimers; struct atimer *next; - BLOCK_ATIMERS; + block_atimers (); atimers = stopped_atimers; stopped_atimers = NULL; @@ -257,7 +272,7 @@ t = next; } - UNBLOCK_ATIMERS; + unblock_atimers (); } } @@ -378,7 +393,8 @@ alarm_signal_handler (int signo) { #ifndef SYNC_INPUT - SIGNAL_THREAD_CHECK (signo); + if (forwarded_signal (signo)) + return; #endif pending_atimers = 1; @@ -397,9 +413,9 @@ { if (pending_atimers) { - BLOCK_ATIMERS; + block_atimers (); run_timers (); - UNBLOCK_ATIMERS; + unblock_atimers (); } } @@ -412,7 +428,9 @@ { if (on) { - signal (SIGALRM, alarm_signal_handler); + struct sigaction action; + emacs_sigaction_init (&action, alarm_signal_handler); + sigaction (SIGALRM, &action, 0); set_alarm (); } else @@ -423,8 +441,10 @@ void init_atimer (void) { + struct sigaction action; free_atimers = stopped_atimers = atimers = NULL; pending_atimers = 0; /* pending_signals is initialized in init_keyboard.*/ - signal (SIGALRM, alarm_signal_handler); + emacs_sigaction_init (&action, alarm_signal_handler); + sigaction (SIGALRM, &action, 0); } === modified file 'src/callproc.c' --- src/callproc.c 2012-08-25 03:11:12 +0000 +++ src/callproc.c 2012-09-03 06:50:19 +0000 @@ -19,7 +19,6 @@ #include <config.h> -#include <signal.h> #include <errno.h> #include <stdio.h> #include <setjmp.h> @@ -506,9 +505,6 @@ if (fd_output >= 0) fd1 = fd_output; -#if 0 /* Some systems don't have sigblock. */ - mask = sigblock (sigmask (SIGCHLD)); -#endif /* Record that we're about to create a synchronous process. */ synch_process_alive = 1; === modified file 'src/conf_post.h' --- src/conf_post.h 2012-08-20 16:48:10 +0000 +++ src/conf_post.h 2012-09-01 21:37:07 +0000 @@ -40,11 +40,6 @@ #endif #endif -#ifdef SIGNAL_H_AHB -#undef SIGNAL_H_AHB -#include <signal.h> -#endif - /* This silences a few compilation warnings on FreeBSD. */ #ifdef BSD_SYSTEM_AHB #undef BSD_SYSTEM_AHB === modified file 'src/data.c' --- src/data.c 2012-08-27 17:23:48 +0000 +++ src/data.c 2012-09-03 07:24:03 +0000 @@ -19,7 +19,6 @@ #include <config.h> -#include <signal.h> #include <stdio.h> #include <setjmp.h> @@ -3213,15 +3212,16 @@ static void arith_error (int signo) { - sigsetmask (SIGEMPTYMASK); - - SIGNAL_THREAD_CHECK (signo); + pthread_sigmask (SIG_SETMASK, &empty_mask, 0); + if (forwarded_signal (signo)) + return; xsignal0 (Qarith_error); } void init_data (void) { + struct sigaction action; /* Don't do this if just dumping out. We don't want to call `signal' in this case so that we don't have trouble with dumping @@ -3230,5 +3230,6 @@ if (!initialized) return; #endif /* CANNOT_DUMP */ - signal (SIGFPE, arith_error); + emacs_sigaction_init (&action, arith_error); + sigaction (SIGFPE, &action, 0); } === modified file 'src/dispnew.c' --- src/dispnew.c 2012-09-01 06:38:52 +0000 +++ src/dispnew.c 2012-09-03 07:24:03 +0000 @@ -21,7 +21,6 @@ #define DISPEXTERN_INLINE EXTERN_INLINE -#include <signal.h> #include <stdio.h> #include <setjmp.h> #include <unistd.h> @@ -5561,8 +5560,11 @@ struct tty_display_info *tty; - signal (SIGWINCH, window_change_signal); - SIGNAL_THREAD_CHECK (signalnum); + struct sigaction action; + emacs_sigaction_init (&action, window_change_signal); + sigaction (SIGWINCH, &action, 0); + if (forwarded_signal (signalnum)) + return; /* The frame size change obviously applies to a single termcap-controlled terminal, but we can't decide which. @@ -6173,7 +6175,11 @@ #ifndef CANNOT_DUMP if (initialized) #endif /* CANNOT_DUMP */ - signal (SIGWINCH, window_change_signal); + { + struct sigaction action; + emacs_sigaction_init (&action, window_change_signal); + sigaction (SIGWINCH, &action, 0); + } #endif /* SIGWINCH */ /* If running as a daemon, no need to initialize any frames/terminal. */ === modified file 'src/emacs.c' --- src/emacs.c 2012-09-02 17:10:35 +0000 +++ src/emacs.c 2012-09-03 07:24:03 +0000 @@ -20,7 +20,6 @@ #include <config.h> -#include <signal.h> #include <errno.h> #include <stdio.h> @@ -297,7 +296,10 @@ void fatal_error_signal (int sig) { - SIGNAL_THREAD_CHECK (sig); + sigset_t unblocked; + + if (forwarded_signal (sig)) + return; fatal_error_code = sig; signal (sig, SIG_DFL); @@ -319,7 +321,9 @@ going to send is probably blocked, so we have to unblock it if we want to really receive it. */ #ifndef MSDOS - sigunblock (sigmask (fatal_error_code)); + sigemptyset (&unblocked); + sigaddset (&unblocked, fatal_error_code); + pthread_sigmask (SIG_UNBLOCK, &unblocked, 0); #endif kill (getpid (), fatal_error_code); @@ -331,8 +335,11 @@ void memory_warning_signal (int sig) { - signal (sig, memory_warning_signal); - SIGNAL_THREAD_CHECK (sig); + struct sigaction action; + emacs_sigaction_init (&action, memory_warning_signal); + sigaction (sig, &action, 0); + if (forwarded_signal (sig)) + return; malloc_warning ("Operating system warns that virtual memory is running low.\n"); @@ -687,6 +694,7 @@ char dname_arg2[80]; #endif char *ch_to_dir; + struct sigaction fatal_error_action; #if GC_MARK_STACK stack_base = &dummy; @@ -1111,6 +1119,7 @@ } init_signals (); + emacs_sigaction_init (&fatal_error_action, fatal_error_signal); /* Don't catch SIGHUP if dumping. */ if (1 @@ -1119,13 +1128,17 @@ #endif ) { - sigblock (sigmask (SIGHUP)); /* In --batch mode, don't catch SIGHUP if already ignored. That makes nohup work. */ - if (! noninteractive - || signal (SIGHUP, SIG_IGN) != SIG_IGN) - signal (SIGHUP, fatal_error_signal); - sigunblock (sigmask (SIGHUP)); + bool catch_SIGHUP = !noninteractive; + if (!catch_SIGHUP) + { + struct sigaction old_action; + sigaction (SIGHUP, 0, &old_action); + catch_SIGHUP = old_action.sa_handler != SIG_IGN; + } + if (catch_SIGHUP) + sigaction (SIGHUP, &fatal_error_action, 0); } if ( @@ -1139,9 +1152,9 @@ /* Don't catch these signals in batch mode if dumping. On some machines, this sets static data that would make signal fail to work right when the dumped Emacs is run. */ - signal (SIGQUIT, fatal_error_signal); - signal (SIGILL, fatal_error_signal); - signal (SIGTRAP, fatal_error_signal); + sigaction (SIGQUIT, &fatal_error_action, 0); + sigaction (SIGILL, &fatal_error_action, 0); + sigaction (SIGTRAP, &fatal_error_action, 0); #ifdef SIGUSR1 add_user_signal (SIGUSR1, "sigusr1"); #endif @@ -1149,68 +1162,73 @@ add_user_signal (SIGUSR2, "sigusr2"); #endif #ifdef SIGABRT - signal (SIGABRT, fatal_error_signal); + sigaction (SIGABRT, &fatal_error_action, 0); #endif #ifdef SIGHWE - signal (SIGHWE, fatal_error_signal); + sigaction (SIGHWE, &fatal_error_action, 0); #endif #ifdef SIGPRE - signal (SIGPRE, fatal_error_signal); + sigaction (SIGPRE, &fatal_error_action, 0); #endif #ifdef SIGORE - signal (SIGORE, fatal_error_signal); + sigaction (SIGORE, &fatal_error_action, 0); #endif #ifdef SIGUME - signal (SIGUME, fatal_error_signal); + sigaction (SIGUME, &fatal_error_action, 0); #endif #ifdef SIGDLK - signal (SIGDLK, fatal_error_signal); + sigaction (SIGDLK, &fatal_error_action, 0); #endif #ifdef SIGCPULIM - signal (SIGCPULIM, fatal_error_signal); + sigaction (SIGCPULIM, &fatal_error_action, 0); #endif #ifdef SIGIOT /* This is missing on some systems - OS/2, for example. */ - signal (SIGIOT, fatal_error_signal); + sigaction (SIGIOT, &fatal_error_action, 0); #endif #ifdef SIGEMT - signal (SIGEMT, fatal_error_signal); + sigaction (SIGEMT, &fatal_error_action, 0); #endif - signal (SIGFPE, fatal_error_signal); + sigaction (SIGFPE, &fatal_error_action, 0); #ifdef SIGBUS - signal (SIGBUS, fatal_error_signal); + sigaction (SIGBUS, &fatal_error_action, 0); #endif - signal (SIGSEGV, fatal_error_signal); + sigaction (SIGSEGV, &fatal_error_action, 0); #ifdef SIGSYS - signal (SIGSYS, fatal_error_signal); + sigaction (SIGSYS, &fatal_error_action, 0); #endif /* May need special treatment on MS-Windows. See http://lists.gnu.org/archive/html/emacs-devel/2010-09/msg01062.html Please update the doc of kill-emacs, kill-emacs-hook, and NEWS if you change this. */ - if (noninteractive) signal (SIGINT, fatal_error_signal); - signal (SIGTERM, fatal_error_signal); + if (noninteractive) + sigaction (SIGINT, &fatal_error_action, 0); + sigaction (SIGTERM, &fatal_error_action, 0); #ifdef SIGXCPU - signal (SIGXCPU, fatal_error_signal); + sigaction (SIGXCPU, &fatal_error_action, 0); #endif #ifdef SIGXFSZ - signal (SIGXFSZ, fatal_error_signal); + sigaction (SIGXFSZ, &fatal_error_action, 0); #endif /* SIGXFSZ */ #ifdef SIGDANGER /* This just means available memory is getting low. */ - signal (SIGDANGER, memory_warning_signal); + { + struct sigaction action; + emacs_sigaction_init (&action, memory_warning_signal); + sigaction (SIGDANGER, &action, 0); + } #endif #ifdef AIX /* 20 is SIGCHLD, 21 is SIGTTIN, 22 is SIGTTOU. */ - signal (SIGXCPU, fatal_error_signal); - signal (SIGIOINT, fatal_error_signal); - signal (SIGGRANT, fatal_error_signal); - signal (SIGRETRACT, fatal_error_signal); - signal (SIGSOUND, fatal_error_signal); - signal (SIGMSG, fatal_error_signal); + sigaction (SIGXCPU, &fatal_error_action, 0); + sigaction (SIGIOINT, &fatal_error_action, 0); + sigaction (SIGGRANT, &fatal_error_action, 0); + sigaction (SIGRETRACT, &fatal_error_action, 0); + sigaction (SIGSOUND, &fatal_error_action, 0); + sigaction (SIGMSG, &fatal_error_action, 0); #endif /* AIX */ } === modified file 'src/emacsgtkfixed.c' --- src/emacsgtkfixed.c 2012-04-23 07:34:29 +0000 +++ src/emacsgtkfixed.c 2012-09-01 21:37:07 +0000 @@ -21,7 +21,6 @@ #include <config.h> #include "emacsgtkfixed.h" -#include <signal.h> #include <stdio.h> #include <setjmp.h> #include "lisp.h" === modified file 'src/floatfns.c' --- src/floatfns.c 2012-07-17 02:56:00 +0000 +++ src/floatfns.c 2012-09-03 07:24:03 +0000 @@ -48,7 +48,6 @@ */ #include <config.h> -#include <signal.h> #include <setjmp.h> #include "lisp.h" #include "syssignal.h" @@ -955,13 +954,18 @@ fatal_error_signal (signo); #ifdef BSD_SYSTEM - sigsetmask (SIGEMPTYMASK); + pthread_sigmask (SIG_SETMASK, &empty_mask, 0); #else /* Must reestablish handler each time it is called. */ - signal (SIGILL, float_error); + { + struct sigaction action; + emacs_sigaction_init (&action, float_error); + sigaction (SIGILL, &action, 0); + } #endif /* BSD_SYSTEM */ - SIGNAL_THREAD_CHECK (signo); + if (forwarded_signal (signo)) + return; in_float = 0; xsignal1 (Qarith_error, float_error_arg); @@ -1007,7 +1011,9 @@ init_floatfns (void) { #ifdef FLOAT_CATCH_SIGILL - signal (SIGILL, float_error); + struct sigaction action; + emacs_sigaction_init (&action, float_error); + sigaction (SIGILL, &action, 0); #endif in_float = 0; } === modified file 'src/gtkutil.c' --- src/gtkutil.c 2012-09-02 16:56:31 +0000 +++ src/gtkutil.c 2012-09-03 06:50:19 +0000 @@ -21,7 +21,6 @@ #ifdef USE_GTK #include <float.h> -#include <signal.h> #include <stdio.h> #include <setjmp.h> @@ -1979,7 +1978,10 @@ /* I really don't know why this is needed, but without this the GLIBC add on library linuxthreads hangs when the Gnome file chooser backend creates threads. */ - sigblock (sigmask (__SIGRTMIN)); + sigset_t blocked; + sigemptyset (&blocked); + sigaddset (&blocked, __SIGRTMIN); + pthread_sigmask (SIG_BLOCK, &blocked, 0); #endif /* HAVE_PTHREAD */ #ifdef HAVE_GTK_FILE_SELECTION_NEW @@ -2001,7 +2003,7 @@ filesel_done = xg_dialog_run (f, w); #if defined (HAVE_PTHREAD) && defined (__SIGRTMIN) - sigunblock (sigmask (__SIGRTMIN)); + pthread_sigmask (SIG_UNBLOCK, &blocked, 0); #endif if (filesel_done == GTK_RESPONSE_OK) @@ -2056,7 +2058,10 @@ Lisp_Object font = Qnil; #if defined (HAVE_PTHREAD) && defined (__SIGRTMIN) - sigblock (sigmask (__SIGRTMIN)); + sigset_t blocked; + sigemptyset (&blocked); + sigaddset (&blocked, __SIGRTMIN); + pthread_sigmask (SIG_BLOCK, &blocked, 0); #endif /* HAVE_PTHREAD */ w = gtk_font_chooser_dialog_new @@ -2085,7 +2090,7 @@ done = xg_dialog_run (f, w); #if defined (HAVE_PTHREAD) && defined (__SIGRTMIN) - sigunblock (sigmask (__SIGRTMIN)); + pthread_sigmask (SIG_UNBLOCK, &blocked, 0); #endif if (done == GTK_RESPONSE_OK) === modified file 'src/keyboard.c' --- src/keyboard.c 2012-09-02 16:56:31 +0000 +++ src/keyboard.c 2012-09-03 07:24:03 +0000 @@ -21,7 +21,6 @@ #define KEYBOARD_INLINE EXTERN_INLINE -#include <signal.h> #include <stdio.h> #include <setjmp.h> #include "lisp.h" @@ -3681,7 +3680,7 @@ if (immediate_quit && NILP (Vinhibit_quit)) { immediate_quit = 0; - sigfree (); + pthread_sigmask (SIG_SETMASK, &empty_mask, 0); QUIT; } } @@ -3833,7 +3832,11 @@ unhold_keyboard_input (); #ifdef SIGIO if (!noninteractive) - signal (SIGIO, input_available_signal); + { + struct sigaction action; + emacs_sigaction_init (&action, input_available_signal); + sigaction (SIGIO, &action, 0); + } #endif /* SIGIO */ start_polling (); } @@ -6781,10 +6784,12 @@ #ifdef SIGIO if (interrupt_input) { - SIGMASKTYPE mask; - mask = sigblock (sigmask (SIGIO)); + sigset_t blocked, procmask; + sigemptyset (&blocked); + sigaddset (&blocked, SIGIO); + pthread_sigmask (SIG_BLOCK, &blocked, &procmask); read_avail_input (expected); - sigsetmask (mask); + pthread_sigmask (SIG_SETMASK, &procmask, 0); } else #ifdef POLL_FOR_INPUT @@ -6793,10 +6798,12 @@ it's always set. */ if (!interrupt_input && poll_suppress_count == 0) { - SIGMASKTYPE mask; - mask = sigblock (sigmask (SIGALRM)); + sigset_t blocked, procmask; + sigemptyset (&blocked); + sigaddset (&blocked, SIGALRM); + pthread_sigmask (SIG_BLOCK, &blocked, &procmask); read_avail_input (expected); - sigsetmask (mask); + pthread_sigmask (SIG_SETMASK, &procmask, 0); } else #endif @@ -6832,10 +6839,12 @@ #ifdef SIGIO if (interrupt_input) { - SIGMASKTYPE mask; - mask = sigblock (sigmask (SIGIO)); + sigset_t blocked, procmask; + sigemptyset (&blocked); + sigaddset (&blocked, SIGIO); + pthread_sigmask (SIG_BLOCK, &blocked, &procmask); kbd_buffer_store_event (&event); - sigsetmask (mask); + pthread_sigmask (SIG_SETMASK, &procmask, 0); } else #endif @@ -7240,7 +7249,8 @@ { /* Must preserve main program's value of errno. */ int old_errno = errno; - SIGNAL_THREAD_CHECK (signo); + if (forwarded_signal (signo)) + return; #ifdef SYNC_INPUT interrupt_input_pending = 1; @@ -7296,6 +7306,7 @@ void add_user_signal (int sig, const char *name) { + struct sigaction action; struct user_signal_info *p; for (p = user_signals; p; p = p->next) @@ -7310,7 +7321,8 @@ p->next = user_signals; user_signals = p; - signal (sig, handle_user_signal); + emacs_sigaction_init (&action, handle_user_signal); + sigaction (sig, &action, 0); } static void @@ -7320,7 +7332,8 @@ struct user_signal_info *p; const char *special_event_name = NULL; - SIGNAL_THREAD_CHECK (sig); + if (forwarded_signal (sig)) + return; if (SYMBOLP (Vdebug_on_event)) special_event_name = SSDATA (SYMBOL_NAME (Vdebug_on_event)); @@ -7381,7 +7394,7 @@ for (p = user_signals; p; p = p->next) if (p->npending > 0) { - SIGMASKTYPE mask; + sigset_t blocked, procmask; if (nstored == 0) { @@ -7391,7 +7404,10 @@ } nstored += p->npending; - mask = sigblock (sigmask (p->sig)); + sigemptyset (&blocked); + sigaddset (&blocked, p->sig); + pthread_sigmask (SIG_BLOCK, &blocked, &procmask); + do { buf.code = p->sig; @@ -7399,7 +7415,8 @@ p->npending--; } while (p->npending > 0); - sigsetmask (mask); + + pthread_sigmask (SIG_SETMASK, &procmask, 0); } return nstored; @@ -10783,7 +10800,8 @@ int old_errno = errno; struct terminal *terminal; - SIGNAL_THREAD_CHECK (signalnum); + if (forwarded_signal (signalnum)) + return; /* See if we have an active terminal on our controlling tty. */ terminal = get_named_tty ("/dev/tty"); @@ -10840,7 +10858,10 @@ /* 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. */ - sigblock (sigmask (SIGINT)); + sigset_t blocked; + sigemptyset (&blocked); + sigaddset (&blocked, SIGINT); + pthread_sigmask (SIG_BLOCK, &blocked, 0); fflush (stdout); reset_all_sys_modes (); @@ -10911,7 +10932,7 @@ #endif /* not MSDOS */ fflush (stdout); init_all_sys_modes (); - sigfree (); + pthread_sigmask (SIG_SETMASK, &empty_mask, 0); } else { @@ -10924,7 +10945,7 @@ struct gcpro gcpro1, gcpro2, gcpro3, gcpro4; immediate_quit = 0; - sigfree (); + pthread_sigmask (SIG_SETMASK, &empty_mask, 0); saved = gl_state; GCPRO4 (saved.object, saved.global_code, saved.current_syntax_table, saved.old_prop); @@ -10969,7 +10990,7 @@ if (!from_signal && EQ (Vquit_flag, Qkill_emacs)) Fkill_emacs (Qnil); - sigfree (); + pthread_sigmask (SIG_SETMASK, &empty_mask, 0); /* Prevent another signal from doing this before we finish. */ clear_waiting_for_input (); input_pending = 0; @@ -11404,17 +11425,23 @@ SIGINT. There is special code in interrupt_signal to exit Emacs on SIGINT when there are no termcap frames on the controlling terminal. */ - signal (SIGINT, interrupt_signal); + struct sigaction action; + emacs_sigaction_init (&action, interrupt_signal); + sigaction (SIGINT, &action, 0); #ifndef DOS_NT /* For systems with SysV TERMIO, C-g is set up for both SIGINT and SIGQUIT and we can't tell which one it will give us. */ - signal (SIGQUIT, interrupt_signal); + sigaction (SIGQUIT, &action, 0); #endif /* not DOS_NT */ } /* Note SIGIO has been undef'd if FIONREAD is missing. */ #ifdef SIGIO if (!noninteractive) - signal (SIGIO, input_available_signal); + { + 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 @@ -11426,7 +11453,7 @@ interrupt_input = 0; #endif - sigfree (); + pthread_sigmask (SIG_SETMASK, &empty_mask, 0); dribble = 0; if (keyboard_init_hook) === modified file 'src/nsfns.m' --- src/nsfns.m 2012-08-17 23:38:43 +0000 +++ src/nsfns.m 2012-09-01 21:37:07 +0000 @@ -30,7 +30,6 @@ interpretation of even the system includes. */ #include <config.h> -#include <signal.h> #include <math.h> #include <setjmp.h> #include <c-strcase.h> === modified file 'src/process.c' --- src/process.c 2012-09-02 17:10:35 +0000 +++ src/process.c 2012-09-03 07:24:03 +0000 @@ -23,7 +23,6 @@ #define PROCESS_INLINE EXTERN_INLINE -#include <signal.h> #include <stdio.h> #include <errno.h> #include <setjmp.h> @@ -1603,8 +1602,7 @@ #if !defined (WINDOWSNT) && defined (FD_CLOEXEC) int wait_child_setup[2]; #endif - sigset_t procmask; - sigset_t blocked; + sigset_t blocked, procmask; struct sigaction sigint_action; struct sigaction sigquit_action; struct sigaction sigpipe_action; @@ -1756,12 +1754,6 @@ int xforkin = forkin; int xforkout = forkout; -#if 0 /* This was probably a mistake--it duplicates code later on, - but fails to handle all the cases. */ - /* Make sure SIGCHLD is not blocked in the child. */ - sigsetmask (SIGEMPTYMASK); -#endif - /* Make the pty be the controlling terminal of the process. */ #ifdef HAVE_PTYS /* First, disconnect its current controlling terminal. */ @@ -5429,8 +5421,12 @@ static void send_process_trap (int ignore) { - SIGNAL_THREAD_CHECK (SIGPIPE); - sigunblock (sigmask (SIGPIPE)); + sigset_t unblocked; + if (forwarded_signal (SIGPIPE)) + return; + sigemptyset (&unblocked); + sigaddset (&unblocked, SIGPIPE); + pthread_sigmask (SIG_UNBLOCK, &unblocked, 0); longjmp (send_process_frame, 1); } @@ -5524,7 +5520,7 @@ struct Lisp_Process *p = XPROCESS (proc); ssize_t rv; struct coding_system *coding; - void (*volatile old_sigpipe) (int); + struct sigaction old_sigpipe_action; if (p->raw_status_new) update_status (p); @@ -5663,7 +5659,9 @@ /* Send this batch, using one or more write calls. */ ptrdiff_t written = 0; int outfd = p->outfd; - old_sigpipe = (void (*) (int)) signal (SIGPIPE, send_process_trap); + struct sigaction action; + emacs_sigaction_init (&action, send_process_trap); + sigaction (SIGPIPE, &action, &old_sigpipe_action); #ifdef DATAGRAM_SOCKETS if (DATAGRAM_CHAN_P (outfd)) { @@ -5674,7 +5672,7 @@ written = rv; else if (errno == EMSGSIZE) { - signal (SIGPIPE, old_sigpipe); + sigaction (SIGPIPE, &old_sigpipe_action, 0); report_file_error ("sending datagram", Fcons (proc, Qnil)); } @@ -5699,7 +5697,7 @@ } #endif } - signal (SIGPIPE, old_sigpipe); + sigaction (SIGPIPE, &old_sigpipe_action, 0); if (rv < 0) { @@ -5759,7 +5757,7 @@ } else { - signal (SIGPIPE, old_sigpipe); + sigaction (SIGPIPE, &old_sigpipe_action, 0); proc = process_sent_to; p = XPROCESS (proc); p->raw_status_new = 0; @@ -6404,7 +6402,8 @@ Lisp_Object proc; struct Lisp_Process *p; - SIGNAL_THREAD_CHECK (signo); + if (forwarded_signal (signo)) + return; while (1) { @@ -7387,7 +7386,11 @@ #ifndef CANNOT_DUMP if (! noninteractive || initialized) #endif - signal (SIGCHLD, sigchld_handler); + { + struct sigaction action; + emacs_sigaction_init (&action, sigchld_handler); + sigaction (SIGCHLD, &action, 0); + } #endif FD_ZERO (&input_wait_mask); === modified file 'src/sound.c' --- src/sound.c 2012-07-29 08:18:29 +0000 +++ src/sound.c 2012-09-03 06:50:19 +0000 @@ -48,7 +48,6 @@ #include "lisp.h" #include "dispextern.h" #include "atimer.h" -#include <signal.h> #include "syssignal.h" /* END: Common Includes */ @@ -316,7 +315,12 @@ turn_on_atimers (1); #ifdef SIGIO - sigunblock (sigmask (SIGIO)); + { + sigset_t unblocked; + sigemptyset (&unblocked); + sigaddset (&unblocked, SIGIO); + pthread_sigmask (SIG_UNBLOCK, &unblocked, 0); + } #endif if (saved_errno != 0) error ("%s: %s", msg, strerror (saved_errno)); @@ -728,6 +732,9 @@ vox_configure (struct sound_device *sd) { int val; +#ifdef SIGIO + sigset_t blocked; +#endif eassert (sd->fd >= 0); @@ -736,7 +743,9 @@ troubles. */ turn_on_atimers (0); #ifdef SIGIO - sigblock (sigmask (SIGIO)); + sigemptyset (&blocked); + sigaddset (&blocked, SIGIO); + pthread_sigmask (SIG_BLOCK, &blocked, 0); #endif val = sd->format; @@ -770,7 +779,7 @@ turn_on_atimers (1); #ifdef SIGIO - sigunblock (sigmask (SIGIO)); + pthread_sigmask (SIG_UNBLOCK, &blocked, 0); #endif } @@ -786,7 +795,10 @@ be interrupted by a signal. Block the ones we know to cause troubles. */ #ifdef SIGIO - sigblock (sigmask (SIGIO)); + sigset_t blocked; + sigemptyset (&blocked); + sigaddset (&blocked, SIGIO); + pthread_sigmask (SIG_BLOCK, &blocked, 0); #endif turn_on_atimers (0); @@ -795,7 +807,7 @@ turn_on_atimers (1); #ifdef SIGIO - sigunblock (sigmask (SIGIO)); + pthread_sigmask (SIG_UNBLOCK, &blocked, 0); #endif /* Close the device. */ === modified file 'src/sysdep.c' --- src/sysdep.c 2012-09-01 01:13:50 +0000 +++ src/sysdep.c 2012-09-03 07:38:38 +0000 @@ -19,9 +19,9 @@ #include <config.h> +#define SYSSIGNAL_INLINE EXTERN_INLINE #define SYSTIME_INLINE EXTERN_INLINE -#include <signal.h> #include <stdio.h> #include <setjmp.h> #ifdef HAVE_PWD_H @@ -302,27 +302,34 @@ 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. */ - sigsetmask (sigmask (SIGCHLD)); + sigset_t sigchild_mask; + sigemptyset (&sigchild_mask); + sigaddset (&sigchild_mask, SIGCHLD); + pthread_sigmask (SIG_SETMASK, &sigchild_mask, 0); + if (0 > kill (pid, 0)) { - sigsetmask (SIGEMPTYMASK); + pthread_sigmask (SIG_SETMASK, &empty_mask, 0); kill (getpid (), SIGCHLD); break; } if (wait_debugging) sleep (1); else - sigpause (SIGEMPTYMASK); + sigsuspend (&empty_mask); #else /* not BSD_SYSTEM, and not HPUX version >= 6 */ #ifdef WINDOWSNT wait (0); break; #else /* not WINDOWSNT */ - sigblock (sigmask (SIGCHLD)); + sigset_t blocked; + sigemptyset (&blocked); + sigaddset (&blocked, SIGCHLD); + pthread_sigmask (SIG_BLOCK, &blocked, 0); errno = 0; if (kill (pid, 0) == -1 && errno == ESRCH) { - sigunblock (sigmask (SIGCHLD)); + pthread_sigmask (SIG_UNBLOCK, &blocked, 0); break; } @@ -456,11 +463,11 @@ #endif /* not MSDOS */ \f -/* Record a signal code and the handler for it. */ +/* Record a signal code and the action for it. */ struct save_signal { int code; - void (*handler) (int); + struct sigaction action; }; static void save_signal_handlers (struct save_signal *); @@ -618,8 +625,9 @@ { while (saved_handlers->code) { - saved_handlers->handler - = (void (*) (int)) signal (saved_handlers->code, SIG_IGN); + struct sigaction action; + emacs_sigaction_init (&action, SIG_IGN); + sigaction (saved_handlers->code, &action, &saved_handlers->action); saved_handlers++; } } @@ -629,7 +637,7 @@ { while (saved_handlers->code) { - signal (saved_handlers->code, saved_handlers->handler); + sigaction (saved_handlers->code, &saved_handlers->action, 0); saved_handlers++; } } @@ -686,13 +694,17 @@ void request_sigio (void) { + sigset_t unblocked; + if (noninteractive) return; + sigemptyset (&unblocked); #ifdef SIGWINCH - sigunblock (sigmask (SIGWINCH)); + sigaddset (&unblocked, SIGWINCH); #endif - sigunblock (sigmask (SIGIO)); + sigaddset (&unblocked, SIGIO); + pthread_sigmask (SIG_UNBLOCK, &unblocked, 0); interrupts_deferred = 0; } @@ -700,6 +712,8 @@ void unrequest_sigio (void) { + sigset_t blocked; + if (noninteractive) return; @@ -708,10 +722,12 @@ return; #endif + sigemptyset (&blocked); #ifdef SIGWINCH - sigblock (sigmask (SIGWINCH)); + sigaddset (&blocked, SIGWINCH); #endif - sigblock (sigmask (SIGIO)); + sigaddset (&blocked, SIGIO); + pthread_sigmask (SIG_BLOCK, &blocked, 0); interrupts_deferred = 1; } @@ -1470,20 +1486,16 @@ } } \f -/* POSIX signals support - DJB */ -/* Anyone with POSIX signals should have ANSI C declarations */ - sigset_t empty_mask; -#ifndef WINDOWSNT - -signal_handler_t -sys_signal (int signal_number, signal_handler_t action) +/* Store into *ACTION a signal action suitable for Emacs, with handler + HANDLER. */ +void +emacs_sigaction_init (struct sigaction *action, signal_handler_t handler) { - struct sigaction new_action, old_action; - sigemptyset (&new_action.sa_mask); - new_action.sa_handler = action; - new_action.sa_flags = 0; + sigemptyset (&action->sa_mask); + 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. @@ -1500,57 +1512,31 @@ # if defined (BROKEN_SA_RESTART) || defined (SYNC_INPUT) if (noninteractive) # endif - new_action.sa_flags = SA_RESTART; -#endif - sigaction (signal_number, &new_action, &old_action); - return (old_action.sa_handler); -} - -#endif /* WINDOWSNT */ - -#ifndef __GNUC__ -/* If we're compiling with GCC, we don't need this function, since it - can be written as a macro. */ -sigset_t -sys_sigmask (int sig) -{ - sigset_t mask; - sigemptyset (&mask); - sigaddset (&mask, sig); - return mask; -} -#endif - -/* I'd like to have these guys return pointers to the mask storage in here, - but there'd be trouble if the code was saving multiple masks. I'll be - safe and pass the structure. It normally won't be more than 2 bytes - anyhow. - DJB */ - -sigset_t -sys_sigblock (sigset_t new_mask) -{ - sigset_t old_mask; - pthread_sigmask (SIG_BLOCK, &new_mask, &old_mask); - return (old_mask); -} - -sigset_t -sys_sigunblock (sigset_t new_mask) -{ - sigset_t old_mask; - pthread_sigmask (SIG_UNBLOCK, &new_mask, &old_mask); - return (old_mask); -} - -sigset_t -sys_sigsetmask (sigset_t new_mask) -{ - sigset_t old_mask; - pthread_sigmask (SIG_SETMASK, &new_mask, &old_mask); - return (old_mask); -} - -\f + action->sa_flags = SA_RESTART; +#endif +} + +#ifdef FORWARD_SIGNAL_TO_MAIN_THREAD +/* POSIX says any thread can receive the signal. On GNU/Linux that is + not true, but for other systems (FreeBSD at least) it is. So + direct the signal to the correct thread and block it from this thread. */ +bool +forwarded_signal (int signo) +{ + if (pthread_equal (pthread_self (), main_thread)) + return 0; + else + { + sigset_t blocked; + sigemptyset (&blocked); + sigaddset (&blocked, signo); + pthread_sigmask (SIG_BLOCK, &blocked, 0); + pthread_kill (main_thread, signo); + return 1; + } +} +#endif + #if !defined HAVE_STRSIGNAL && !HAVE_DECL_SYS_SIGLIST static char *my_sys_siglist[NSIG]; # ifdef sys_siglist === modified file 'src/syssignal.h' --- src/syssignal.h 2012-07-13 01:19:06 +0000 +++ src/syssignal.h 2012-09-03 07:38:38 +0000 @@ -17,6 +17,9 @@ You should have received a copy of the GNU General Public License along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */ +#include <signal.h> +#include <stdbool.h> + extern void init_signals (void); #ifdef HAVE_PTHREAD @@ -26,63 +29,21 @@ #define FORWARD_SIGNAL_TO_MAIN_THREAD #endif -/* Don't #include <signal.h>. That header should always be #included - before "config.h", because some configuration files (like s/hpux.h) - indicate that SIGIO doesn't work by #undef-ing SIGIO. If this file - #includes <signal.h>, then that will re-#define SIGIO and confuse - things. */ -/* XXX This is not correct anymore, there is a BROKEN_SIGIO macro. */ - -#define SIGMASKTYPE sigset_t - -#define SIGEMPTYMASK (empty_mask) +INLINE_HEADER_BEGIN +#ifndef SYSSIGNAL_INLINE +# define SYSSIGNAL_INLINE INLINE +#endif + extern sigset_t empty_mask; -/* POSIX pretty much destroys any possibility of writing sigmask as a - macro in standard C. We always define our own version because the - predefined macro in Glibc 2.1 is only provided for compatibility for old - programs that use int as signal mask type. */ -#undef sigmask -#ifdef __GNUC__ -#define sigmask(SIG) \ - ({ \ - sigset_t _mask; \ - sigemptyset (&_mask); \ - sigaddset (&_mask, SIG); \ - _mask; \ - }) -#else /* ! defined (__GNUC__) */ -extern sigset_t sys_sigmask (); -#define sigmask(SIG) (sys_sigmask (SIG)) -#endif /* ! defined (__GNUC__) */ - -#undef sigpause -#define sigpause(MASK) sigsuspend (&(MASK)) - -#define sigblock(SIG) sys_sigblock (SIG) -#define sigunblock(SIG) sys_sigunblock (SIG) -#ifndef sigsetmask -#define sigsetmask(SIG) sys_sigsetmask (SIG) -#endif -#undef signal -#define signal(SIG,ACT) sys_signal(SIG,ACT) - -/* Whether this is what all systems want or not, this is what - appears to be assumed in the source, for example data.c:arith_error. */ typedef void (*signal_handler_t) (int); -signal_handler_t sys_signal (int signal_number, signal_handler_t action); -sigset_t sys_sigblock (sigset_t new_mask); -sigset_t sys_sigunblock (sigset_t new_mask); -sigset_t sys_sigsetmask (sigset_t new_mask); +extern void emacs_sigaction_init (struct sigaction *, signal_handler_t); + #if ! (defined TIOCNOTTY || defined USG5 || defined CYGWIN) _Noreturn void croak (char *); #endif -#define sys_sigdel(MASK,SIG) sigdelset (&MASK,SIG) - -#define sigfree() sigsetmask (SIGEMPTYMASK) - #if defined (SIGIO) && defined (BROKEN_SIGIO) # undef SIGIO #endif @@ -97,12 +58,8 @@ #undef SIGPTY #endif - -/* FIXME? Emacs only defines NSIG_MINIMUM on some platforms? */ #if NSIG < NSIG_MINIMUM -# ifdef NSIG -# undef NSIG -# endif +# undef NSIG # define NSIG NSIG_MINIMUM #endif @@ -133,24 +90,9 @@ #ifdef FORWARD_SIGNAL_TO_MAIN_THREAD extern pthread_t main_thread; -#define SIGNAL_THREAD_CHECK(signo) \ - do { \ - if (!pthread_equal (pthread_self (), main_thread)) \ - { \ - /* POSIX says any thread can receive the signal. On GNU/Linux \ - that is not true, but for other systems (FreeBSD at least) \ - it is. So direct the signal to the correct thread and block \ - it from this thread. */ \ - sigset_t new_mask; \ - \ - sigemptyset (&new_mask); \ - sigaddset (&new_mask, signo); \ - pthread_sigmask (SIG_BLOCK, &new_mask, 0); \ - pthread_kill (main_thread, signo); \ - return; \ - } \ - } while (0) +extern bool forwarded_signal (int); +#else +SYSSIGNAL_INLINE bool forwarded_signal (int signo) { return 0; } +#endif -#else /* not FORWARD_SIGNAL_TO_MAIN_THREAD */ -#define SIGNAL_THREAD_CHECK(signo) -#endif /* not FORWARD_SIGNAL_TO_MAIN_THREAD */ +INLINE_HEADER_END === modified file 'src/term.c' --- src/term.c 2012-08-31 10:53:19 +0000 +++ src/term.c 2012-09-03 06:50:19 +0000 @@ -25,7 +25,6 @@ #include <sys/file.h> #include <sys/time.h> #include <unistd.h> -#include <signal.h> #include <setjmp.h> #include "lisp.h" @@ -2932,7 +2931,10 @@ no_controlling_tty = 1; #else #ifdef TIOCNOTTY /* Try BSD ioctls. */ - sigblock (sigmask (SIGTTOU)); + sigset_t blocked; + sigemptyset (&blocked); + sigaddset (&blocked, SIGTTOU); + pthread_sigmask (SIG_BLOCK, &blocked, 0); fd = emacs_open (DEV_TTY, O_RDWR, 0); if (fd != -1 && ioctl (fd, TIOCNOTTY, 0) != -1) { @@ -2940,7 +2942,7 @@ } if (fd != -1) emacs_close (fd); - sigunblock (sigmask (SIGTTOU)); + pthread_sigmask (SIG_UNBLOCK, &blocked, 0); #else /* Unknown system. */ croak (); @@ -2971,6 +2973,7 @@ struct tty_display_info *tty = NULL; struct terminal *terminal = NULL; int ctty = 0; /* 1 if asked to open controlling tty. */ + sigset_t blocked; if (!terminal_type) maybe_fatal (must_succeed, 0, @@ -3074,9 +3077,11 @@ /* On some systems, tgetent tries to access the controlling terminal. */ - sigblock (sigmask (SIGTTOU)); + sigemptyset (&blocked); + sigaddset (&blocked, SIGTTOU); + pthread_sigmask (SIG_BLOCK, &blocked, 0); status = tgetent (tty->termcap_term_buffer, terminal_type); - sigunblock (sigmask (SIGTTOU)); + pthread_sigmask (SIG_UNBLOCK, &blocked, 0); if (status < 0) { === modified file 'src/widget.c' --- src/widget.c 2012-09-02 16:56:31 +0000 +++ src/widget.c 2012-09-03 00:06:25 +0000 @@ -50,9 +50,6 @@ #include <X11/ShellP.h> #include "../lwlib/lwlib.h" -#include <signal.h> -#include "syssignal.h" - #include "character.h" #include "font.h" === modified file 'src/xmenu.c' --- src/xmenu.c 2012-08-17 21:52:15 +0000 +++ src/xmenu.c 2012-09-01 21:37:07 +0000 @@ -32,11 +32,6 @@ #include <config.h> -#if 0 /* Why was this included? And without syssignal.h? */ -/* On 4.3 this loses if it comes after xterm.h. */ -#include <signal.h> -#endif - #include <stdio.h> #include <setjmp.h> === modified file 'src/xterm.c' --- src/xterm.c 2012-09-02 17:10:35 +0000 +++ src/xterm.c 2012-09-03 07:24:03 +0000 @@ -21,7 +21,6 @@ /* Xt features made by Fred Pierresteguy. */ #include <config.h> -#include <signal.h> #include <stdio.h> #include <setjmp.h> @@ -29,9 +28,6 @@ #include "lisp.h" #include "blockinput.h" - -/* Need syssignal.h for various externs and definitions that may be required - by some configurations for calls to signal later in this source file. */ #include "syssignal.h" /* This may include sys/types.h, and that somehow loses @@ -7766,7 +7762,9 @@ #ifdef USG /* USG systems forget handlers when they are used; must reestablish each time */ - signal (signalnum, x_connection_signal); + struct sigaction action; + emacs_sigaction_init (&action, x_connection_signal); + sigaction (signalnum, &action, 0); #endif /* USG */ } @@ -7788,6 +7786,7 @@ struct x_display_info *dpyinfo = x_display_info_for_display (dpy); Lisp_Object frame, tail; ptrdiff_t idx = SPECPDL_INDEX (); + sigset_t unblocked; error_msg = alloca (strlen (error_message) + 1); strcpy (error_msg, error_message); @@ -7876,10 +7875,12 @@ } /* Ordinary stack unwind doesn't deal with these. */ + sigemptyset (&unblocked); #ifdef SIGIO - sigunblock (sigmask (SIGIO)); + sigaddset (&unblocked, SIGIO); #endif - sigunblock (sigmask (SIGALRM)); + sigaddset (&unblocked, SIGALRM); + pthread_sigmask (SIG_UNBLOCK, &unblocked, 0); TOTALLY_UNBLOCK_INPUT; unbind_to (idx, Qnil); @@ -10759,6 +10760,8 @@ void x_initialize (void) { + struct sigaction action; + baud_rate = 19200; x_noop_count = 0; @@ -10805,7 +10808,8 @@ XSetErrorHandler (x_error_handler); XSetIOErrorHandler (x_io_error_quitter); - signal (SIGPIPE, x_connection_signal); + emacs_sigaction_init (&action, x_connection_signal); + sigaction (SIGPIPE, &action, 0); } ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-03 8:16 ` Paul Eggert @ 2012-09-03 8:49 ` Andreas Schwab 2012-09-03 9:02 ` Paul Eggert 2012-09-03 15:32 ` Eli Zaretskii 1 sibling, 1 reply; 23+ messages in thread From: Andreas Schwab @ 2012-09-03 8:49 UTC (permalink / raw) To: Paul Eggert; +Cc: lekktu, 12327 Paul Eggert <eggert@cs.ucla.edu> writes: > /* Must reestablish handler each time it is called. */ That's not true for POSIX signals. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-03 8:49 ` Andreas Schwab @ 2012-09-03 9:02 ` Paul Eggert 2012-09-03 9:17 ` Andreas Schwab 0 siblings, 1 reply; 23+ messages in thread From: Paul Eggert @ 2012-09-03 9:02 UTC (permalink / raw) To: Andreas Schwab; +Cc: lekktu, 12327 On 09/03/2012 01:49 AM, Andreas Schwab wrote: > That's not true for POSIX signals. Yes, thanks, that's right. Further cleanup is needed, both there and elsewhere. I've avoided these further cleanups in order to keep the size of the patch down. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-03 9:02 ` Paul Eggert @ 2012-09-03 9:17 ` Andreas Schwab 2012-09-04 8:12 ` Paul Eggert 0 siblings, 1 reply; 23+ messages in thread From: Andreas Schwab @ 2012-09-03 9:17 UTC (permalink / raw) To: Paul Eggert; +Cc: lekktu, 12327 Paul Eggert <eggert@cs.ucla.edu> writes: > On 09/03/2012 01:49 AM, Andreas Schwab wrote: >> That's not true for POSIX signals. > > Yes, thanks, that's right. Further cleanup is needed, > both there and elsewhere. I've avoided these further > cleanups in order to keep the size of the patch down. But that's really part of the signal->sigaction move. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-03 9:17 ` Andreas Schwab @ 2012-09-04 8:12 ` Paul Eggert 2012-09-06 11:59 ` Andy Moreton 0 siblings, 1 reply; 23+ messages in thread From: Paul Eggert @ 2012-09-04 8:12 UTC (permalink / raw) To: Andreas Schwab; +Cc: lekktu, 12327 [-- Attachment #1: Type: text/plain, Size: 352 bytes --] On 09/03/2012 02:17 AM, Andreas Schwab wrote: > But that's really part of the signal->sigaction move. OK, I looked into that, and several other places are also affected by the move. Attached is a patch to fix the issue that you found, and the other related issues that I saw. This assumes the patch I already sent in <http://bugs.gnu.org/12327#20>. [-- Attachment #2: syssignao.txt --] [-- Type: text/plain, Size: 22134 bytes --] === 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 <eggert@cs.ucla.edu> + 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 <errno.h> #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); } \f -#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; } \f -#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 @@ \f -/* 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 */ \f -/* 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 */ -} - -\f /************************************************************************ 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); } ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-04 8:12 ` Paul Eggert @ 2012-09-06 11:59 ` Andy Moreton 2012-09-06 14:41 ` martin rudalics 0 siblings, 1 reply; 23+ messages in thread From: Andy Moreton @ 2012-09-06 11:59 UTC (permalink / raw) To: 12327 On Tue 04 Sep 2012, Paul Eggert wrote: > On 09/03/2012 02:17 AM, Andreas Schwab wrote: >> But that's really part of the signal->sigaction move. > > OK, I looked into that, and several other places are also > affected by the move. Attached is a patch to fix the issue > that you found, and the other related issues that I saw. This > assumes the patch I already sent in <http://bugs.gnu.org/12327#20>. This patch set (or something similar) was committed in r109893, and causes a crash in the Windows build (r109892 is ok). Crash is in r109893 built on Windows XP using MingGW gcc 4.6.2. To reproduce it, run "M-x rgrep" using Cygwin grep. The grep output is all displayed in the *grep* buffer, and then emacs aborts. Attaching gdb gives the output below. (gdb) thread apply all bt full Thread 6 (Thread 2072.0x9fc): #0 0x7c90e514 in ntdll!LdrAccessResource () from C:\WINDOWS\system32\ntdll.dll No symbol table info available. #1 0x7c90d9da in ntdll!ZwReadFile () from C:\WINDOWS\system32\ntdll.dll No symbol table info available. #2 0x7c801879 in ReadFile () from C:\WINDOWS\system32\kernel32.dll No symbol table info available. #3 0x00000628 in ?? () No symbol table info available. #4 0x00000000 in ?? () No symbol table info available. Thread 5 (Thread 2072.0xdfc): #0 0x7c90e514 in ntdll!LdrAccessResource () from C:\WINDOWS\system32\ntdll.dll No symbol table info available. #1 0x7c90da4a in ntdll!ZwRemoveIoCompletion () from C:\WINDOWS\system32\ntdll.dll No symbol table info available. #2 0x7c80a7e6 in KERNEL32!GetQueuedCompletionStatus () from C:\WINDOWS\system32\kernel32.dll No symbol table info available. #3 0x77e7715c in RPCRT4!I_RpcBindingCopy () from C:\WINDOWS\system32\rpcrt4.dll No symbol table info available. #4 0x77e772a0 in RPCRT4!I_RpcBindingCopy () from C:\WINDOWS\system32\rpcrt4.dll No symbol table info available. #5 0x77e77328 in RPCRT4!I_RpcBindingCopy () from C:\WINDOWS\system32\rpcrt4.dll No symbol table info available. #6 0x77e76ad1 in RPCRT4!I_RpcBindingCopy () from C:\WINDOWS\system32\rpcrt4.dll No symbol table info available. #7 0x77e76c97 in RPCRT4!I_RpcBindingCopy () from C:\WINDOWS\system32\rpcrt4.dll No symbol table info available. #8 0x7c80b729 in KERNEL32!GetModuleFileNameA () from C:\WINDOWS\system32\kernel32.dll No symbol table info available. #9 0x00000000 in ?? () No symbol table info available. Thread 4 (Thread 2072.0x358): #0 0x7c90e514 in ntdll!LdrAccessResource () from C:\WINDOWS\system32\ntdll.dll No symbol table info available. #1 0x7c90da4a in ntdll!ZwRemoveIoCompletion () from C:\WINDOWS\system32\ntdll.dll No symbol table info available. #2 0x7c80a7e6 in KERNEL32!GetQueuedCompletionStatus () from C:\WINDOWS\system32\kernel32.dll No symbol table info available. #3 0x77e7715c in RPCRT4!I_RpcBindingCopy () from C:\WINDOWS\system32\rpcrt4.dll No symbol table info available. #4 0x77e772a0 in RPCRT4!I_RpcBindingCopy () from C:\WINDOWS\system32\rpcrt4.dll No symbol table info available. #5 0x77e77328 in RPCRT4!I_RpcBindingCopy () from C:\WINDOWS\system32\rpcrt4.dll No symbol table info available. #6 0x77e76ad1 in RPCRT4!I_RpcBindingCopy () from C:\WINDOWS\system32\rpcrt4.dll No symbol table info available. #7 0x77e76c97 in RPCRT4!I_RpcBindingCopy () from C:\WINDOWS\system32\rpcrt4.dll No symbol table info available. #8 0x7c80b729 in KERNEL32!GetModuleFileNameA () from C:\WINDOWS\system32\kernel32.dll No symbol table info available. #9 0x00000000 in ?? () No symbol table info available. Thread 3 (Thread 2072.0x1dc): #0 0x7c90e514 in ntdll!LdrAccessResource () from C:\WINDOWS\system32\ntdll.dll No symbol table info available. #1 0x7c90df5a in ntdll!ZwWaitForSingleObject () from C:\WINDOWS\system32\ntdll.dll No symbol table info available. #2 0x7c8025db in WaitForSingleObjectEx () from C:\WINDOWS\system32\kernel32.dll No symbol table info available. #3 0x00000620 in ?? () No symbol table info available. #4 0x00000000 in ?? () No symbol table info available. Thread 2 (Thread 2072.0x410): #0 0x7c90e514 in ntdll!LdrAccessResource () from C:\WINDOWS\system32\ntdll.dll No symbol table info available. #1 0x77f16bf2 in GdiDrawStream () from C:\WINDOWS\system32\gdi32.dll No symbol table info available. #2 0x00fa2b1b in ?? () from C:\WINDOWS\system32\uxtheme.dll No symbol table info available. #3 0x00fa289b in ?? () from C:\WINDOWS\system32\uxtheme.dll No symbol table info available. #4 0x00fa2753 in ?? () from C:\WINDOWS\system32\uxtheme.dll No symbol table info available. #5 0x00fa2c9f in UxTheme!DrawThemeParentBackground () from C:\WINDOWS\system32\uxtheme.dll No symbol table info available. #6 0x00fa5f42 in UxTheme!SetThemeAppProperties () from C:\WINDOWS\system32\uxtheme.dll No symbol table info available. #7 0x00fa6485 in UxTheme!SetThemeAppProperties () from C:\WINDOWS\system32\uxtheme.dll No symbol table info available. #8 0x00fbc4b9 in UxTheme!GetThemeTextMetrics () from C:\WINDOWS\system32\uxtheme.dll No symbol table info available. #9 0x00fa1ac7 in ?? () from C:\WINDOWS\system32\uxtheme.dll No symbol table info available. #10 0x00fa1b3d in ?? () from C:\WINDOWS\system32\uxtheme.dll No symbol table info available. #11 0x7e4294ed in USER32!GetPropW () from C:\WINDOWS\system32\user32.dll No symbol table info available. #12 0x0114b954 in w32_wnd_proc (hwnd=0xa036c, msg=0x85, wParam=0x870409db, lParam=0x0) at w32fns.c:3832 f = 0xa45740 dpyinfo = 0x16626c0 wmsg = { msg = { hwnd = 0x12, message = 0x7240f7c8, wParam = 0xfc505d, lParam = 0xfbe152, time = 0xfbe535, pt = { x = 0x7240f838, y = 0x6 } }, dwModifiers = 0x6, rect = { left = 0x11, top = 0x11, right = 0x7240f87c, bottom = 0x11 } } windows_translate = 0xfa39d6 key = 0xa45740 #13 0x7e418734 in USER32!GetDC () from C:\WINDOWS\system32\user32.dll No symbol table info available. #14 0x000a036c in ?? () No symbol table info available. #15 0x00000085 in ?? () No symbol table info available. #16 0x870409db in ?? () No symbol table info available. #17 0x00000000 in ?? () No symbol table info available. Thread 1 (Thread 2072.0x564): #0 0x7c90120f in ntdll!DbgUiConnectToDbg () from C:\WINDOWS\system32\ntdll.dll No symbol table info available. #1 0x0115317e in emacs_abort () at w32fns.c:7215 button = 0x6 #2 0x010539f2 in sys_wait (status=0x82f264) at w32proc.c:462 active = 0x0 retval = 0x0 nh = 0x0 pid = 0xc38 cp = 0x165f430 cps = {0x165f430, 0x82f1d8, 0x82f490, 0x82fcdc, 0x1, 0x8, 0x353c81a, 0x0, 0x82f23c, 0x132c256, 0x60, 0x0, 0x82f99c, 0x82f3e8, 0x10e02e4, 0x5, 0x7c90df4a, 0x7c809590, 0x7ffdf000, 0x82f280, 0x7c8095d2, 0x82f22c, 0x7c8095c0, 0x0, 0x8, 0x0, 0x714, 0x710, 0x68c, 0x644, 0x5fc, 0x600} wait_hnd = {0x0, 0x82f168, 0x1028d66, 0x3555ad2, 0x353c832, 0x82f1d8, 0x1036669, 0x13f203d, 0x3, 0x132c24e, 0x353c81a, 0x163a5e8, 0x82f15c, 0x82fcdc, 0x3555ad0, 0x353c81a, 0x82fcf0, 0x82f1d8, 0x10380e2, 0x3555ad2, 0x353c832, 0x0, 0x82f12c, 0x10320ff, 0x82ffe0, 0x0, 0x3555ad2, 0x353c832, 0x0, 0x0, 0x3, 0x35554aa} #3 0x010508a0 in record_child_status_change () at process.c:6422 proc = 0x534cee5 p = 0x534cee0 pid = 0xc38 w = 0x0 tail = 0x536717e #4 0x01050ee0 in handle_child_signal (sig=0x12) at process.c:6529 No locals. #5 0x01143cfe in handle_on_main_thread (sig=0x12, handler=0x1050ed1 <handle_child_signal>) at sysdep.c:1584 old_errno = 0x16 on_main_thread = 0x1 #6 0x01050eff in deliver_child_signal (sig=0x12) at process.c:6536 No locals. #7 0x01054d67 in sys_select (nfds=0xa, rfds=0x82f6e4, wfds=0x0, efds=0x0, timeout=0x82f6d4, ignored=0x0) at w32proc.c:1325 orfds = { bits = {0x309, 0x0} } timeout_ms = 0xfa start_time = 0x5e2323e i = 0xa nh = 0x5 nc = 0x2 nr = 0x1 active = 0x5 cp = 0x165f430 cps = {0x165f430, 0x165f3d8, 0x82f5bc, 0xee6b280, 0x0, 0xee6b280, 0x30, 0x0 <repeats 17 times>, 0xa, 0x200, 0x82f628, 0x1014eea, 0x0, 0x0, 0x0, 0x353c81a} wait_hnd = {0x714, 0x710, 0x68c, 0x644, 0x5fc, 0x600, 0x624, 0x82f5bc, 0x1, 0x82f5e8, 0x82f598, 0x12c0b6b, 0x82f4e8, 0x0, 0x850000, 0x0, 0x1645970, 0x86a4d0, 0x850000, 0x1028d66, 0x15ad2, 0x8, 0x82f3d4, 0x850000, 0x82f4c8, 0xe11f, 0x82f4b8, 0x105917e, 0x50488ca1, 0x36d61600, 0x82f4d8, 0x1103595, 0x50488ca1, 0x36d61600, 0x29e, 0x1, 0x82f99c, 0x3c8c90c, 0x82f4e8, 0x5048, 0x0, 0xe09c0, 0x8ca1, 0x5048, 0x7c80a0e9, 0x0, 0x82f518, 0x100dd13, 0x0, 0xee6b280, 0xee6b280, 0xee6b280, 0x50488ca1, 0xe11f, 0x82f528, 0x105917e, 0x50488ca1, 0x36d61600, 0x82f548, 0x1103595, 0x50488ca1, 0x36d61600, 0x29e, 0xce70001, 0xfbe96c7a, 0x1cd8c24, 0x82f558, 0x5048, 0x0, 0xe09c0, 0x8ca1, 0x5048, 0x0, 0x0, 0x82f588, 0x100dd13, 0x0, 0xee6b280, 0xee6b280, 0x1662720, 0x16626c0, 0x0, 0x82f588, 0x1142a8d, 0x50488ca 1, 0x36d61600, 0x0, 0x0, 0xa, 0x1, 0x0, 0x3811de0, 0x1, 0x82f5e8, 0x82f608, 0x10150ed} fdindex = {0xffffffff, 0x0, 0x3, 0x8, 0x9, 0x353c81a, 0x82f378, 0x1028c3b, 0x163a414, 0x353c81a, 0x0, 0x0, 0x163a414, 0x82f380, 0x82f378, 0x1028a1c, 0x3546de2, 0x457c7c0, 0x353c81a, 0x38e4226, 0x391e1ca, 0x0, 0x82f418, 0x10362be, 0x3546de2, 0x353c81a, 0x392284a, 0x39229ca, 0x4bbf405, 0x54184be, 0x82f438, 0x36f1e8a, 0x353c802, 0x82f4a4, 0x26aaec7a, 0x77c49210, 0x26aaec7a, 0x2fda46, 0x989680, 0x0, 0x200, 0x1, 0x77c4921d, 0xce7e2a0, 0xfbe96c7a, 0x1cd8c24, 0x86a508, 0x7c90cfea, 0x7c80a0e9, 0x710, 0x82f438, 0x7c90cfea, 0x7c80a0e9, 0x710, 0x82f448, 0x12e5c9c, 0x16627bc, 0x353c81a, 0x4405800, 0x353c81a, 0x36f1e8a, 0x82f514, 0x26aaec7a, 0x77c49210} #8 0x0104ca1f in wait_reading_process_output (time_limit=0x23, nsecs=0x0, read_kbd=0xffffffff, do_display=0x1, wait_for_cell=0x353c81a, wait_proc=0x0, just_wait_proc=0x0) at process.c:4682 timeout_reduced_for_timers = 0x1 channel = 0xa nfds = 0x1 Available = { bits = {0x200, 0x0} } Writeok = { bits = {0x0, 0x0} } check_write = 0x0 check_delay = 0x0 no_avail = 0x0 xerrno = 0x16 proc = 0x534cee5 timeout = { tv_sec = 0x0, tv_nsec = 0x0 } end_time = { tv_sec = 0x50488cc4, tv_nsec = 0x26fb3f80 } wait_channel = 0xffffffff got_some_input = 0x1 count = 0x2 #9 0x010faab4 in sit_for (timeout=0x8c, reading=0x1, do_display=0x1) at dispnew.c:5972 sec = 0x23 nsec = 0x0 #10 0x01009478 in read_char (commandflag=0x1, nmaps=0x5, maps=0x82f990, prev_event=0x353c81a, used_mouse_menu=0x82fa78, end_time=0x0) at keyboard.c:2742 tem0 = 0x82f938 timeout = 0x23 delay_level = 0x5 buffer_size = 0x3f c = 0x353c81a jmpcount = 0x2 local_getcjmp = {0x82f938, 0x3c8c900, 0x82f99c, 0x3c8c90c, 0x82f7bc, 0x1008d63, 0x82ffe0, 0x0, 0x82f8e8, 0x378e1ca, 0x3565f02, 0x3c8c900, 0x82f8c8, 0x1027b11, 0x379e540, 0x379e540} save_jump = {0x0 <repeats 16 times>} key_already_recorded = 0x0 tem = 0x3cf79ee save = 0xdb15 previous_echo_area_message = 0x353c81a also_record = 0x353c81a reread = 0x0 gcpro1 = { next = 0x12e3542, var = 0x51e2276, nvars = 0x354377a } gcpro2 = { next = 0x82f830, var = 0x354377a, nvars = 0x82f828 } polling_stopped_here = 0x0 orig_kboard = 0x353f500 #11 0x0101c9ae in read_key_sequence (keybuf=0x82fc00, bufsize=0x1e, prompt=0x353c81a, dont_downcase_last=0x0, can_return_switch_frame=0x1, fix_current_buffer=0x1) at keyboard.c:9356 interrupted_kboard = 0x353f500 interrupted_frame = 0x3811de0 key = 0x4405805 used_mouse_menu = 0x0 echo_local_start = 0x0 last_real_key_start = 0x0 keys_local_start = 0x0 local_first_binding = 0x0 from_string = 0x353c81a count = 0x2 t = 0x0 echo_start = 0x0 keys_start = 0x0 nmaps = 0x5 nmaps_allocated = 0x5 defs = 0x82f960 submaps = 0x82f990 orig_local_map = 0x4247c76 orig_keymap = 0x353c81a localized_local_map = 0x0 first_binding = 0x0 first_unbound = 0x1f mock_input = 0x0 fkey = { parent = 0x38f7456, map = 0x38f7456, start = 0x0, end = 0x0 } keytran = { parent = 0x3539ec6, map = 0x3539ec6, start = 0x0, end = 0x0 } indec = { parent = 0x38f745e, map = 0x38f745e, start = 0x0, end = 0x0 } shift_translated = 0x0 delayed_switch_frame = 0x353c81a original_uppercase = 0x36c50 original_uppercase_position = 0xffffffff dummyflag = 0x0 starting_buffer = 0x4405800 fake_prefixed_keys = 0x353c81a gcpro1 = { next = 0x4405800, var = 0x44c0668, nvars = 0x82fac8 } #12 0x01005e1b in command_loop_1 () at keyboard.c:1499 cmd = 0x3565f02 keybuf = {0x200001e0, 0x353c81a, 0x82fb00, 0x0, 0x163a630, 0x353c81a, 0x3745dfa, 0x35554aa, 0x353c81a, 0x0, 0x0, 0x35554a8, 0x0, 0x0, 0x82fc68, 0x1037cfb, 0x35554aa, 0x353c81a, 0x44f6b7e, 0x353c81a, 0xc, 0x353c81a, 0x2, 0x7ffd6000, 0x0, 0x0, 0x82fc98, 0x1033c91, 0x2, 0x44f6b7e} i = 0x1 prev_modiff = 0x1dd prev_buffer = 0x4405800 already_adjusted = 0x0 #13 0x01032285 in internal_condition_case (bfun=0x1005929 <command_loop_1>, handlers=0x3548dca, hfun=0x10050fe <cmd_error>) at eval.c:1319 val = 0x44f6b7e c = { tag = 0x353c81a, val = 0x353c81a, next = 0x82fda4, gcpro = 0x0, jmp = {0x82fd68, 0x7ffd6000, 0x0, 0x0, 0x82fcbc, 0x1032232, 0x82ffe0, 0x0, 0x2, 0x0, 0x82fdd0, 0x2, 0x2, 0x0, 0x1, 0x1}, backlist = 0x0, handlerlist = 0x0, lisp_eval_depth = 0x0, pdlcount = 0x2, poll_suppress_count = 0x0, interrupt_input_blocked = 0x0, byte_stack = 0x0 } h = { handler = 0x3548dca, var = 0x353c81a, chosen_clause = 0x2020e99, tag = 0x82fcf0, next = 0x0 } #14 0x01005556 in command_loop_2 (ignore=0x353c81a) at keyboard.c:1194 val = 0x7ffd6000 #15 0x01031c96 in internal_catch (tag=0x3546612, func=0x1005532 <command_loop_2>, arg=0x353c81a) at eval.c:1076 c = { tag = 0x3546612, val = 0x353c81a, next = 0x0, gcpro = 0x0, jmp = {0x82fe18, 0x7ffd6000, 0x0, 0x0, 0x82fd8c, 0x1031c87, 0x82ffe0, 0x0, 0x353c81a, 0x3542e00, 0x740053, 0x6e0061, 0x7ffd6000, 0x82fe18, 0x1027b01, 0x165c4fc}, backlist = 0x0, handlerlist = 0x0, lisp_eval_depth = 0x0, pdlcount = 0x2, poll_suppress_count = 0x0, interrupt_input_blocked = 0x0, byte_stack = 0x0 } #16 0x01005512 in command_loop () at keyboard.c:1173 No locals. #17 0x01004abb in recursive_edit_1 () at keyboard.c:794 count = 0x1 val = 0x77c4a190 #18 0x01004de8 in Frecursive_edit () at keyboard.c:858 count = 0x0 buffer = 0x353c81a #19 0x01002abc in main (argc=0x2, argv=0xa425b0) at emacs.c:1643 dummy = 0x82ffe0 stack_bottom_variable = 0x0 do_initial_setlocale = 0x1 skip_args = 0x0 no_loadup = 0x0 junk = 0x0 dname_arg = 0x0 ch_to_dir = 0x0 (gdb) quit ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-06 11:59 ` Andy Moreton @ 2012-09-06 14:41 ` martin rudalics 2012-09-06 16:46 ` Eli Zaretskii 0 siblings, 1 reply; 23+ messages in thread From: martin rudalics @ 2012-09-06 14:41 UTC (permalink / raw) To: Andy Moreton; +Cc: 12327 > Crash is in r109893 built on Windows XP using MingGW gcc 4.6.2. > > To reproduce it, run "M-x rgrep" using Cygwin grep. The grep > output is all displayed in the *grep* buffer, and then emacs aborts. With r109905 doing M-x shell and then exit gets me Breakpoint 1, emacs_abort () at w32fns.c:7201 7201 button = MessageBox (NULL, (gdb) bt #0 emacs_abort () at w32fns.c:7201 #1 0x010345e2 in sys_wait (status=0x82f284) at w32proc.c:462 #2 0x01031ff0 in record_child_status_change () at process.c:6422 #3 0x01032315 in handle_child_signal (sig=18) at process.c:6529 #4 0x010dd5fa in handle_on_main_thread (sig=18, handler=0x1032306 <handle_child_signal>) at sysdep.c:1584 #5 0x01032334 in deliver_child_signal (sig=18) at process.c:6536 #6 0x010358ec in sys_select (nfds=4, rfds=0x82f704, wfds=0x0, efds=0x0, timeout=0x82f6f4, ignored=0x0) at w32proc.c:1325 #7 0x0102f943 in wait_reading_process_output (time_limit=900, nsecs=0, read_kbd=-1, do_display=1, wait_for_cell=52672538, wait_proc=0x0, just_wait_proc=0) at process.c:4682 #8 0x010a4b7e in sit_for (timeout=3600, reading=true, do_display=1) at dispnew.c:5972 #9 0x010071dc in read_char (commandflag=1, nmaps=3, maps=0x82f9a0, prev_event=52672538, used_mouse_menu=0x82fa78, end_time=0x0) at keyboard.c:2742 #10 0x01011081 in read_key_sequence (keybuf=0x82fc00, bufsize=30, prompt=52672538, dont_downcase_last=0, can_return_switch_frame=1, fix_current_buffer=1) at keyboard.c:9356 #11 0x01004fad in command_loop_1 () at keyboard.c:1499 #12 0x0101e737 in internal_condition_case (bfun=0x1004c3e <command_loop_1>, handlers=52723146, hfun=0x10045e8 <cmd_error>) at eval.c:1319 #13 0x0100496f in command_loop_2 (ignore=52672538) at keyboard.c:1194 #14 0x0101e26b in internal_catch (tag=52712978, func=0x100494b <command_loop_2>, arg=52672538) at eval.c:1076 #15 0x0100492b in command_loop () at keyboard.c:1173 #16 0x01004225 in recursive_edit_1 () at keyboard.c:794 #17 0x01004374 in Frecursive_edit () at keyboard.c:858 #18 0x010028e6 in main (argc=1, argv=0xa32808) at emacs.c:1643 martin, on GNU Emacs 24.2.50.1 (i386-mingw-nt5.1.2600) of 2012-09-06 on MACHNO ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-06 14:41 ` martin rudalics @ 2012-09-06 16:46 ` Eli Zaretskii 2012-09-06 16:54 ` Andy Moreton 2012-09-06 17:37 ` martin rudalics 0 siblings, 2 replies; 23+ messages in thread From: Eli Zaretskii @ 2012-09-06 16:46 UTC (permalink / raw) To: martin rudalics; +Cc: andrewjmoreton, 12327 > Date: Thu, 06 Sep 2012 16:41:27 +0200 > From: martin rudalics <rudalics@gmx.at> > Cc: 12327@debbugs.gnu.org > > > Crash is in r109893 built on Windows XP using MingGW gcc 4.6.2. > > > > To reproduce it, run "M-x rgrep" using Cygwin grep. The grep > > output is all displayed in the *grep* buffer, and then emacs aborts. > > With r109905 doing M-x shell and then exit gets me > > Breakpoint 1, emacs_abort () at w32fns.c:7201 > 7201 button = MessageBox (NULL, > (gdb) bt > #0 emacs_abort () at w32fns.c:7201 > #1 0x010345e2 in sys_wait (status=0x82f284) at w32proc.c:462 > #2 0x01031ff0 in record_child_status_change () at process.c:6422 > #3 0x01032315 in handle_child_signal (sig=18) at process.c:6529 > #4 0x010dd5fa in handle_on_main_thread (sig=18, handler=0x1032306 <handle_child_signal>) at sysdep.c:1584 > #5 0x01032334 in deliver_child_signal (sig=18) at process.c:6536 > #6 0x010358ec in sys_select (nfds=4, rfds=0x82f704, wfds=0x0, efds=0x0, timeout=0x82f6f4, ignored=0x0) at w32proc.c:1325 I think I fixed this in revision 109907 on the trunk, please test. At least "M-x shell" followed by "exit RET" no longer winds up in emacs_abort for me. I cannot test Andrew's recipe, as I don't have Cygwin installed. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-06 16:46 ` Eli Zaretskii @ 2012-09-06 16:54 ` Andy Moreton 2012-09-06 17:20 ` Eli Zaretskii 2012-09-06 17:37 ` martin rudalics 1 sibling, 1 reply; 23+ messages in thread From: Andy Moreton @ 2012-09-06 16:54 UTC (permalink / raw) To: 12327 On Thu 06 Sep 2012, Eli Zaretskii wrote: >> Date: Thu, 06 Sep 2012 16:41:27 +0200 >> From: martin rudalics <rudalics@gmx.at> >> Cc: 12327@debbugs.gnu.org >> >> > Crash is in r109893 built on Windows XP using MingGW gcc 4.6.2. >> > >> > To reproduce it, run "M-x rgrep" using Cygwin grep. The grep >> > output is all displayed in the *grep* buffer, and then emacs aborts. >> >> With r109905 doing M-x shell and then exit gets me >> >> Breakpoint 1, emacs_abort () at w32fns.c:7201 >> 7201 button = MessageBox (NULL, >> (gdb) bt >> #0 emacs_abort () at w32fns.c:7201 >> #1 0x010345e2 in sys_wait (status=0x82f284) at w32proc.c:462 >> #2 0x01031ff0 in record_child_status_change () at process.c:6422 >> #3 0x01032315 in handle_child_signal (sig=18) at process.c:6529 >> #4 0x010dd5fa in handle_on_main_thread (sig=18, handler=0x1032306 <handle_child_signal>) at sysdep.c:1584 >> #5 0x01032334 in deliver_child_signal (sig=18) at process.c:6536 >> #6 0x010358ec in sys_select (nfds=4, rfds=0x82f704, wfds=0x0, efds=0x0, >> timeout=0x82f6f4, ignored=0x0) at w32proc.c:1325 > > I think I fixed this in revision 109907 on the trunk, please test. At > least "M-x shell" followed by "exit RET" no longer winds up in > emacs_abort for me. I cannot test Andrew's recipe, as I don't have > Cygwin installed. Thanks Eli, it all seems to happy again. AndyM ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-06 16:54 ` Andy Moreton @ 2012-09-06 17:20 ` Eli Zaretskii 0 siblings, 0 replies; 23+ messages in thread From: Eli Zaretskii @ 2012-09-06 17:20 UTC (permalink / raw) To: Andy Moreton; +Cc: 12327-done > From: Andy Moreton <andrewjmoreton@gmail.com> > Date: Thu, 06 Sep 2012 17:54:29 +0100 > > > I think I fixed this in revision 109907 on the trunk, please test. At > > least "M-x shell" followed by "exit RET" no longer winds up in > > emacs_abort for me. I cannot test Andrew's recipe, as I don't have > > Cygwin installed. > > Thanks Eli, it all seems to happy again. Thanks for testing, I'm closing the bug. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-06 16:46 ` Eli Zaretskii 2012-09-06 16:54 ` Andy Moreton @ 2012-09-06 17:37 ` martin rudalics 1 sibling, 0 replies; 23+ messages in thread From: martin rudalics @ 2012-09-06 17:37 UTC (permalink / raw) To: Eli Zaretskii; +Cc: andrewjmoreton, 12327 > I think I fixed this in revision 109907 on the trunk, please test. At > least "M-x shell" followed by "exit RET" no longer winds up in > emacs_abort for me. I cannot test Andrew's recipe, as I don't have > Cygwin installed. Works now. Thanks, martin ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-03 8:16 ` Paul Eggert 2012-09-03 8:49 ` Andreas Schwab @ 2012-09-03 15:32 ` Eli Zaretskii 1 sibling, 0 replies; 23+ messages in thread From: Eli Zaretskii @ 2012-09-03 15:32 UTC (permalink / raw) To: Paul Eggert; +Cc: lekktu, 12327 > Date: Mon, 03 Sep 2012 01:16:08 -0700 > From: Paul Eggert <eggert@cs.ucla.edu> > CC: 12327@debbugs.gnu.org, lekktu@gmail.com > > > I could go with replacing 'signal' etc. with their modern Posix > > replacements > > OK, thanks, I reworked the patch to implement that suggestion. Thanks. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-01 22:38 bug#12327: Signal-handler cleanup for Emacs Paul Eggert 2012-09-02 17:51 ` Eli Zaretskii @ 2012-09-07 1:35 ` Paul Eggert 2012-09-07 6:02 ` Eli Zaretskii 1 sibling, 1 reply; 23+ messages in thread From: Paul Eggert @ 2012-09-07 1:35 UTC (permalink / raw) To: 12327; +Cc: Juanma Barranquero I installed the reworked patch <http://bugs.gnu.org/12327#20> into the trunk as bzr 109909. I also installed the first chunk of the later cleanup <http://bugs.gnu.org/12327#35> as trunk bzr 109893. This had a typo that prevented it working on Windows hosts -- sorry about that, and thanks to Andy and Martin for reporting it and Eli for fixing it. Other parts of the later cleanup still need installing, to fix the problem Andreas mentioned, plus some other race conditions I've found during analysis and testing on GNU/Linux. Since Bug#12327 is now marked as done and is getting a bit hard-to-follow anyway, I'll submit these separately, as a different bug#. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-07 1:35 ` Paul Eggert @ 2012-09-07 6:02 ` Eli Zaretskii 2012-09-07 7:26 ` Eli Zaretskii 0 siblings, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2012-09-07 6:02 UTC (permalink / raw) To: Paul Eggert; +Cc: lekktu, 12327 > Date: Thu, 06 Sep 2012 18:35:59 -0700 > From: Paul Eggert <eggert@cs.ucla.edu> > CC: Eli Zaretskii <eliz@gnu.org>, Juanma Barranquero <lekktu@gmail.com> > > I installed the reworked patch <http://bugs.gnu.org/12327#20> > into the trunk as bzr 109909. This doesn't compile on Windows: gcc -I. -c -gdwarf-2 -g3 -DEMACSDEBUG -fno-crossjumping -Id:/usr/include/libxml2 -DGLYPH_DEBUG=1 -Demacs=1 -I../lib -I../nt/inc -DHAVE_NTGUI=1 -DUSE_CRT_DLL=1 -o oo/i386/editfns.o editfns.c alloc.c: In function `die': alloc.c:6716: error: `SIGABRT' undeclared (first use in this function) alloc.c:6716: error: (Each undeclared identifier is reported only once alloc.c:6716: error: for each function it appears in.) makefile:322: recipe for target `oo/i386/alloc.o' failed make[1]: *** [oo/i386/alloc.o] Error 1 make[1]: *** Waiting for unfinished jobs.... make[1]: Leaving directory `D:/gnu/bzr/emacs/trunk/src' makefile:432: recipe for target `all-other-dirs-gmake' failed make: *** [all-other-dirs-gmake] Error 2 ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-07 6:02 ` Eli Zaretskii @ 2012-09-07 7:26 ` Eli Zaretskii 2012-09-07 8:27 ` Eli Zaretskii 0 siblings, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2012-09-07 7:26 UTC (permalink / raw) To: eggert; +Cc: lekktu, 12327 > Date: Fri, 07 Sep 2012 09:02:04 +0300 > From: Eli Zaretskii <eliz@gnu.org> > Cc: lekktu@gmail.com, 12327@debbugs.gnu.org > > This doesn't compile on Windows: > > gcc -I. -c -gdwarf-2 -g3 -DEMACSDEBUG -fno-crossjumping -Id:/usr/include/libxml2 -DGLYPH_DEBUG=1 -Demacs=1 -I../lib -I../nt/inc -DHAVE_NTGUI=1 -DUSE_CRT_DLL=1 -o oo/i386/editfns.o editfns.c > alloc.c: In function `die': > alloc.c:6716: error: `SIGABRT' undeclared (first use in this function) > alloc.c:6716: error: (Each undeclared identifier is reported only once > alloc.c:6716: error: for each function it appears in.) > makefile:322: recipe for target `oo/i386/alloc.o' failed > make[1]: *** [oo/i386/alloc.o] Error 1 > make[1]: *** Waiting for unfinished jobs.... > make[1]: Leaving directory `D:/gnu/bzr/emacs/trunk/src' > makefile:432: recipe for target `all-other-dirs-gmake' failed > make: *** [all-other-dirs-gmake] Error 2 Actually, this isn't Windows-specific, it happens in any build that specifies --enable-checking. Fixed by revision 109918 on the trunk. The following link failure _is_ Windows-specific, stand by for a fix. gcc -o oo/i386/temacs.bin -gdwarf-2 -g3 -Wl,-stack,0x00800000 -Wl,-heap,0x00100000 -Wl,-image-base,0x01000000 -Wl,-subsystem,console -Wl,-entry,__start -Wl,-Map,oo/i386/temacs.map oo/i386/firstfile.o oo/i386/emacs.res oo/i386/temacs0.a oo/i386/temacs1.a oo/i386/temacs2.a oo/i386/lastfile.a ../lib/oo/i386/libgnu.a -lwinmm -ladvapi32 -lgdi32 -lcomdlg32 -luser32 -lmpr -lshell32 -lwinspool -lole32 -lcomctl32 -lusp10 oo/i386/temacs0.a(emacs.o)(.text+0x1066): In function `main': D:\gnu\bzr\emacs\trunk\src/emacs.c:1129: undefined reference to `sigaction' oo/i386/temacs0.a(emacs.o)(.text+0x1091):D:\gnu\bzr\emacs\trunk\src/emacs.c:1133: undefined reference to `sigaction' oo/i386/temacs0.a(emacs.o)(.text+0x10ff):D:\gnu\bzr\emacs\trunk\src/emacs.c:1157: undefined reference to `sigaction' oo/i386/temacs0.a(emacs.o)(.text+0x111a):D:\gnu\bzr\emacs\trunk\src/emacs.c:1184: undefined reference to `sigaction' oo/i386/temacs0.a(emacs.o)(.text+0x1135):D:\gnu\bzr\emacs\trunk\src/emacs.c:1188: undefined reference to `sigaction' oo/i386/temacs0.a(emacs.o)(.text+0x1159):D:\gnu\bzr\emacs\trunk\src/emacs.c:1198: more undefined references to `sigaction' follow collect2: ld returned 1 exit status makefile:501: recipe for target `oo/i386/temacs.exe' failed make[1]: *** [oo/i386/temacs.exe] Error 1 make[1]: Leaving directory `D:/gnu/bzr/emacs/trunk/src' makefile:432: recipe for target `all-other-dirs-gmake' failed make: *** [all-other-dirs-gmake] Error 2 ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-07 7:26 ` Eli Zaretskii @ 2012-09-07 8:27 ` Eli Zaretskii 2012-09-07 8:59 ` Paul Eggert 0 siblings, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2012-09-07 8:27 UTC (permalink / raw) To: eggert; +Cc: lekktu, 12327 > Date: Fri, 07 Sep 2012 10:26:17 +0300 > From: Eli Zaretskii <eliz@gnu.org> > Cc: lekktu@gmail.com, 12327@debbugs.gnu.org > > The following link failure _is_ Windows-specific, stand by for a fix. > > gcc -o oo/i386/temacs.bin -gdwarf-2 -g3 -Wl,-stack,0x00800000 -Wl,-heap,0x00100000 -Wl,-image-base,0x01000000 -Wl,-subsystem,console -Wl,-entry,__start -Wl,-Map,oo/i386/temacs.map oo/i386/firstfile.o oo/i386/emacs.res oo/i386/temacs0.a oo/i386/temacs1.a oo/i386/temacs2.a oo/i386/lastfile.a ../lib/oo/i386/libgnu.a -lwinmm -ladvapi32 -lgdi32 -lcomdlg32 -luser32 -lmpr -lshell32 -lwinspool -lole32 -lcomctl32 -lusp10 > oo/i386/temacs0.a(emacs.o)(.text+0x1066): In function `main': > D:\gnu\bzr\emacs\trunk\src/emacs.c:1129: undefined reference to `sigaction' > oo/i386/temacs0.a(emacs.o)(.text+0x1091):D:\gnu\bzr\emacs\trunk\src/emacs.c:1133: undefined reference to `sigaction' > oo/i386/temacs0.a(emacs.o)(.text+0x10ff):D:\gnu\bzr\emacs\trunk\src/emacs.c:1157: undefined reference to `sigaction' > oo/i386/temacs0.a(emacs.o)(.text+0x111a):D:\gnu\bzr\emacs\trunk\src/emacs.c:1184: undefined reference to `sigaction' > oo/i386/temacs0.a(emacs.o)(.text+0x1135):D:\gnu\bzr\emacs\trunk\src/emacs.c:1188: undefined reference to `sigaction' > oo/i386/temacs0.a(emacs.o)(.text+0x1159):D:\gnu\bzr\emacs\trunk\src/emacs.c:1198: more undefined references to `sigaction' follow > collect2: ld returned 1 exit status > makefile:501: recipe for target `oo/i386/temacs.exe' failed > make[1]: *** [oo/i386/temacs.exe] Error 1 > make[1]: Leaving directory `D:/gnu/bzr/emacs/trunk/src' > makefile:432: recipe for target `all-other-dirs-gmake' failed > make: *** [all-other-dirs-gmake] Error 2 Should be fixed with trunk revision 109919. (Why are there still calls to 'signal', as opposed to 'sigaction', in some parts of Emacs?) ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-07 8:27 ` Eli Zaretskii @ 2012-09-07 8:59 ` Paul Eggert 2012-09-07 10:21 ` Eli Zaretskii 0 siblings, 1 reply; 23+ messages in thread From: Paul Eggert @ 2012-09-07 8:59 UTC (permalink / raw) To: Eli Zaretskii; +Cc: lekktu, 12327 On 09/07/2012 01:27 AM, Eli Zaretskii wrote: > Should be fixed with trunk revision 109919. Thanks, I looked for other problems of that ilk and fixed them in trunk bzr 109921. > (Why are there still calls to 'signal', as opposed to 'sigaction', in > some parts of Emacs?) Thanks for catching that. Three of them were wrong -- a glitch when merging patches. Fixed in trunk bzr 109922. The other calls are setting signal handlers to SIG_DFL or SIG_IGN. For these, signal and sigaction do the same thing, so I left the calls alone, to minimize changes. If it's better, for regularity purposes, to replace these 'signal' calls with 'sigaction' calls, that's an easy and simple change that I can do. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#12327: Signal-handler cleanup for Emacs 2012-09-07 8:59 ` Paul Eggert @ 2012-09-07 10:21 ` Eli Zaretskii 0 siblings, 0 replies; 23+ messages in thread From: Eli Zaretskii @ 2012-09-07 10:21 UTC (permalink / raw) To: Paul Eggert; +Cc: lekktu, 12327 > Date: Fri, 07 Sep 2012 01:59:20 -0700 > From: Paul Eggert <eggert@cs.ucla.edu> > CC: lekktu@gmail.com, 12327@debbugs.gnu.org > > > (Why are there still calls to 'signal', as opposed to 'sigaction', in > > some parts of Emacs?) > > Thanks for catching that. Three of them were wrong -- a glitch when > merging patches. Fixed in trunk bzr 109922. > > The other calls are setting signal handlers to SIG_DFL or SIG_IGN. > For these, signal and sigaction do the same thing, so I left the > calls alone, to minimize changes. If it's better, for regularity > purposes, to replace these 'signal' calls with 'sigaction' calls, > that's an easy and simple change that I can do. I think it's better to use a single family of functions, it makes the code slightly easier to comprehend. But I'm okay with leaving those calls to 'signal', if no one else cares. It just means that 'sys_signal' in the Windows sources will have to be rewritten to call 'sigaction' internally (I left it alone in revision 109919 because I thought it was on its way out). ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2012-09-07 10:21 UTC | newest] Thread overview: 23+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-09-01 22:38 bug#12327: Signal-handler cleanup for Emacs Paul Eggert 2012-09-02 17:51 ` Eli Zaretskii 2012-09-02 18:37 ` Eli Zaretskii 2012-09-02 19:01 ` Paul Eggert 2012-09-02 21:20 ` Eli Zaretskii 2012-09-03 8:16 ` Paul Eggert 2012-09-03 8:49 ` Andreas Schwab 2012-09-03 9:02 ` Paul Eggert 2012-09-03 9:17 ` Andreas Schwab 2012-09-04 8:12 ` Paul Eggert 2012-09-06 11:59 ` Andy Moreton 2012-09-06 14:41 ` martin rudalics 2012-09-06 16:46 ` Eli Zaretskii 2012-09-06 16:54 ` Andy Moreton 2012-09-06 17:20 ` Eli Zaretskii 2012-09-06 17:37 ` martin rudalics 2012-09-03 15:32 ` Eli Zaretskii 2012-09-07 1:35 ` Paul Eggert 2012-09-07 6:02 ` Eli Zaretskii 2012-09-07 7:26 ` Eli Zaretskii 2012-09-07 8:27 ` Eli Zaretskii 2012-09-07 8:59 ` Paul Eggert 2012-09-07 10:21 ` Eli Zaretskii
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.