From: Paul Eggert Date: Mon, 24 Jun 2013 00:31:31 +0000 (-0700) Subject: A more-conservative workaround for Cygwin SIGCHLD issues. X-Git-Tag: emacs-24.3.90~173^2^2~42^2~45^2~387^2~1992^2~66 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=fa55d2aaa23d5916b87a6980c9606466e07df124;p=emacs.git A more-conservative workaround for Cygwin SIGCHLD issues. * callproc.c (Fcall_process): * process.c (create_process): Make sure SIGCHLD is caught before we fork, since Emacs startup no arranges to catch SIGCHLD. * process.c (lib_child_handler): Initialize to null, not to dummy_handler. (catch_child_signal): Allow self to be called lazily. Do nothing if it's already been called. Assume caller has blocked SIGCHLD (all callers do now). * emacs.c (main): Do not catch SIGCHLD here; defer it until just before it's really needed. * nsterm.m (ns_term_init): No need to re-catch SIGCHLD here, since it hasn't been caught yet. Fixes: debbugs:14569 --- diff --git a/src/ChangeLog b/src/ChangeLog index f9451711f32..6357491725d 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,20 @@ +2013-06-23 Paul Eggert + + A more-conservative workaround for Cygwin SIGCHLD issues (Bug#14569). + * callproc.c (Fcall_process): + * process.c (create_process): + Make sure SIGCHLD is caught before we fork, + since Emacs startup no arranges to catch SIGCHLD. + * process.c (lib_child_handler): Initialize to null, not to + dummy_handler. + (catch_child_signal): Allow self to be called lazily. + Do nothing if it's already been called. + Assume caller has blocked SIGCHLD (all callers do now). + * emacs.c (main): Do not catch SIGCHLD here; defer it until + just before it's really needed. + * nsterm.m (ns_term_init): No need to re-catch SIGCHLD here, + since it hasn't been caught yet. + 2013-06-23 Lars Magne Ingebrigtsen * image.c (compute_image_size): New function to implement diff --git a/src/callproc.c b/src/callproc.c index f0aa8222342..7db984fa71c 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -613,6 +613,7 @@ usage: (call-process PROGRAM &optional INFILE DESTINATION DISPLAY &rest ARGS) * block_input (); block_child_signal (); + catch_child_signal (); #ifdef WINDOWSNT pid = child_setup (filefd, fd1, fd_error, new_argv, 0, current_dir); diff --git a/src/emacs.c b/src/emacs.c index c5b32c7c0e7..13f6d117ebc 100644 --- a/src/emacs.c +++ b/src/emacs.c @@ -1257,13 +1257,6 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem tzset (); #endif /* MSDOS */ - /* Do this after initializing the memory allocator, since it uses - glib and glib uses malloc. And do it before anything else that - invokes glib, to avoid potential races among glib subthreads in - Cygwin glib. gfilenotify invokes glib, so this can't be delayed - further. */ - catch_child_signal (); - #ifdef HAVE_GFILENOTIFY globals_of_gfilenotify (); #endif diff --git a/src/nsterm.m b/src/nsterm.m index 93f693fe55e..c88e5034d39 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -4360,12 +4360,6 @@ ns_term_init (Lisp_Object display_name) [NSApp run]; ns_do_open_file = YES; - -#ifdef NS_IMPL_GNUSTEP - /* GNUstep steals SIGCHLD for use in NSTask, but we don't use NSTask. - We must re-catch it so subprocess works. */ - catch_child_signal (); -#endif return dpyinfo; } diff --git a/src/process.c b/src/process.c index 6df1bf7eff7..3f062b6db16 100644 --- a/src/process.c +++ b/src/process.c @@ -1590,7 +1590,6 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) #ifndef WINDOWSNT int wait_child_setup[2]; #endif - sigset_t blocked; int forkin, forkout; bool pty_flag = 0; Lisp_Object lisp_pty_name = Qnil; @@ -1685,12 +1684,8 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) encoded_current_dir = ENCODE_FILE (current_dir); block_input (); - - /* Block SIGCHLD until we have a chance to store the new fork's - pid in its process structure. */ - sigemptyset (&blocked); - sigaddset (&blocked, SIGCHLD); - pthread_sigmask (SIG_BLOCK, &blocked, 0); + block_child_signal (); + catch_child_signal (); #ifndef WINDOWSNT /* vfork, and prevent local vars from being clobbered by the vfork. */ @@ -1822,8 +1817,8 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) /* Emacs ignores SIGPIPE, but the child should not. */ signal (SIGPIPE, SIG_DFL); - /* Stop blocking signals in the child. */ - pthread_sigmask (SIG_SETMASK, &empty_mask, 0); + /* Stop blocking SIGCHLD in the child. */ + unblock_child_signal (); if (pty_flag) child_setup_tty (xforkout); @@ -1843,8 +1838,8 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) if (pid >= 0) XPROCESS (process)->alive = 1; - /* Stop blocking signals in the parent. */ - pthread_sigmask (SIG_SETMASK, &empty_mask, 0); + /* Stop blocking in the parent. */ + unblock_child_signal (); unblock_input (); if (pid < 0) @@ -6125,9 +6120,10 @@ process has been transmitted to the serial port. */) /* LIB_CHILD_HANDLER is a SIGCHLD handler that Emacs calls while doing its own SIGCHLD handling. On POSIXish systems, glib needs this to - keep track of its own children. The default handler does nothing. */ + keep track of its own children. GNUstep is similar. */ + static void dummy_handler (int sig) {} -static signal_handler_t volatile lib_child_handler = dummy_handler; +static signal_handler_t volatile lib_child_handler; /* Handle a SIGCHLD signal by looking for known child processes of Emacs whose status have changed. For each one found, record its @@ -7060,7 +7056,10 @@ integer or floating point values. return system_process_attributes (pid); } -/* Arrange to catch SIGCHLD if needed. */ +/* Arrange to catch SIGCHLD if this hasn't already been arranged. + Invoke this after init_process_emacs, and after glib and/or GNUstep + futz with the SIGCHLD handler, but before Emacs forks any children. + This function's caller should block SIGCHLD. */ void catch_child_signal (void) @@ -7072,25 +7071,38 @@ catch_child_signal (void) return; #endif +#ifndef NS_IMPL_GNUSTEP + if (lib_child_handler) + return; +#endif + #if defined HAVE_GLIB && !defined WINDOWSNT /* Tickle glib's child-handling code. Ask glib to wait for Emacs itself; this should always fail, but is enough to initialize glib's private SIGCHLD handler, allowing the code below to copy it into LIB_CHILD_HANDLER. - Do this early in Emacs initialization, before glib creates - threads, to avoid race condition bugs in Cygwin glib. */ - g_source_unref (g_child_watch_source_new (getpid ())); + Do this here, rather than early in Emacs initialization where it + might make more sense, to try to avoid bugs in Cygwin glib (Bug#14569). */ + { + GSource *source = g_child_watch_source_new (getpid ()); + g_source_unref (source); + } #endif emacs_sigaction_init (&action, deliver_child_signal); - block_child_signal (); sigaction (SIGCHLD, &action, &old_action); eassert (! (old_action.sa_flags & SA_SIGINFO)); - if (old_action.sa_handler != SIG_DFL && old_action.sa_handler != SIG_IGN - && old_action.sa_handler != deliver_child_signal) - lib_child_handler = old_action.sa_handler; - unblock_child_signal (); + +#ifdef NS_IMPL_GNUSTEP + if (old_action.sa_handler == deliver_child_signal) + return; +#endif + + lib_child_handler + = (old_action.sa_handler == SIG_DFL || old_action.sa_handler == SIG_IGN + ? dummy_handler + : old_action.sa_handler); }