From: Paul Eggert Date: Wed, 31 Oct 2012 17:27:29 +0000 (-0700) Subject: Fix crash when using Emacs as commit editor for git. X-Git-Tag: emacs-24.2.90~200 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=322aea6ddf7ec7fd71410d98ec1de69f219aff3e;p=emacs.git Fix crash when using Emacs as commit editor for git. * callproc.c (setpgrp): Remove macro, as we now use setpgid and it is configured in conf_post.h. (Fcall_process): Don't invoke both setsid and setpgid; the former is enough, if it exists. * callproc.c (Fcall_process, child_setup): * process.c (create_process): Use setpgid. * conf_post.h (setpgid) [!HAVE_SETPGID]: New macro, which substitutes for the real thing. * dispnew.c (init_display): Initialize the foreground group if we are running a tty display. * emacs.c (main): Do not worry about setpgrp; init_display does it now. * lisp.h (init_foreground_group): New decl. * sysdep.c (inherited_pgroup): New static var. (init_foreground_group, tcsetpgrp_without_stopping) (narrow_foreground_group, widen_foreground_group): New functions. (init_sys_modes): Narrow foreground group. (reset_sys_modes): Widen foreground group. Fixes: debbugs:12697 --- diff --git a/src/ChangeLog b/src/ChangeLog index 4edb3a7007b..4dc18b6909b 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,24 @@ +2012-10-31 Paul Eggert + + Fix crash when using Emacs as commit editor for git (Bug#12697). + * callproc.c (setpgrp): Remove macro, as we now use setpgid + and it is configured in conf_post.h. + (Fcall_process): Don't invoke both setsid and setpgid; the former + is enough, if it exists. + * callproc.c (Fcall_process, child_setup): + * process.c (create_process): Use setpgid. + * conf_post.h (setpgid) [!HAVE_SETPGID]: New macro, which substitutes + for the real thing. + * dispnew.c (init_display): Initialize the foreground group + if we are running a tty display. + * emacs.c (main): Do not worry about setpgrp; init_display does it now. + * lisp.h (init_foreground_group): New decl. + * sysdep.c (inherited_pgroup): New static var. + (init_foreground_group, tcsetpgrp_without_stopping) + (narrow_foreground_group, widen_foreground_group): New functions. + (init_sys_modes): Narrow foreground group. + (reset_sys_modes): Widen foreground group. + 2012-10-31 Michael Albinus * dbusbind.c: Fix cut'n'waste error. Use HAVE_DBUS_VALIDATE_INTERFACE. diff --git a/src/callproc.c b/src/callproc.c index b33882e54c2..c236f22fc86 100644 --- a/src/callproc.c +++ b/src/callproc.c @@ -64,13 +64,6 @@ along with GNU Emacs. If not, see . */ #include "nsterm.h" #endif -#ifdef HAVE_SETPGID -#if !defined (USG) -#undef setpgrp -#define setpgrp setpgid -#endif -#endif - /* Pattern used by call-process-region to make temp files. */ static Lisp_Object Vtemp_file_name_pattern; @@ -618,14 +611,12 @@ usage: (call-process PROGRAM &optional INFILE BUFFER DISPLAY &rest ARGS) */) { if (fd[0] >= 0) emacs_close (fd[0]); + #ifdef HAVE_SETSID setsid (); -#endif -#if defined (USG) - setpgrp (); #else - setpgrp (pid, pid); -#endif /* USG */ + setpgid (0, 0); +#endif /* Emacs ignores SIGPIPE, but the child should not. */ signal (SIGPIPE, SIG_DFL); @@ -1295,13 +1286,9 @@ child_setup (int in, int out, int err, char **new_argv, bool set_pgrp, if (err != in && err != out) emacs_close (err); -#if defined (USG) -#ifndef SETPGRP_RELEASES_CTTY - setpgrp (); /* No arguments but equivalent in this case */ +#if defined HAVE_SETPGID || ! (defined USG && defined SETPGRP_RELEASES_CTTY) + setpgid (pid, pid); #endif -#else /* not USG */ - setpgrp (pid, pid); -#endif /* not USG */ /* setpgrp_of_tty is incorrect here; it uses input_fd. */ tcsetpgrp (0, pid); diff --git a/src/conf_post.h b/src/conf_post.h index aa008107ba6..6056821d4a7 100644 --- a/src/conf_post.h +++ b/src/conf_post.h @@ -112,6 +112,14 @@ You lose; /* Emacs for DOS must be compiled with DJGPP */ #endif /* End of gnulib-related stuff. */ +#ifndef HAVE_SETPGID +# ifdef USG +# define setpgid(pid, pgid) setpgrp () +# else +# define setpgid(pid, pgid) setpgrp (pid, pgid) +# endif +#endif + /* Define one of these for easier conditionals. */ #ifdef HAVE_X_WINDOWS /* We need a little extra space, see ../../lisp/loadup.el and the diff --git a/src/dispnew.c b/src/dispnew.c index fa24408aa43..9f0e22fcdcb 100644 --- a/src/dispnew.c +++ b/src/dispnew.c @@ -6283,6 +6283,8 @@ init_display (void) struct terminal *t; struct frame *f = XFRAME (selected_frame); + init_foreground_group (); + /* Open a display on the controlling tty. */ t = init_tty (0, terminal_type, 1); /* Errors are fatal. */ diff --git a/src/emacs.c b/src/emacs.c index 7f3228641ae..98e3f11f0cb 100644 --- a/src/emacs.c +++ b/src/emacs.c @@ -1091,19 +1091,14 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem #endif /* DOS_NT */ } +#if defined (HAVE_PTHREAD) && !defined (SYSTEM_MALLOC) && !defined (DOUG_LEA_MALLOC) if (! noninteractive) { -#if defined (USG5) && defined (INTERRUPT_INPUT) - setpgrp (); -#endif -#if defined (HAVE_PTHREAD) && !defined (SYSTEM_MALLOC) && !defined (DOUG_LEA_MALLOC) - { - extern void malloc_enable_thread (void); + extern void malloc_enable_thread (void); - malloc_enable_thread (); - } -#endif + malloc_enable_thread (); } +#endif init_signals (dumping); diff --git a/src/lisp.h b/src/lisp.h index 4cf8fef0de3..3ec188b67c7 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -3474,6 +3474,7 @@ struct terminal; extern char *get_current_dir_name (void); #endif extern void stuff_char (char c); +extern void init_foreground_group (void); extern void init_sigio (int); extern void sys_subshell (void); extern void sys_suspend (void); diff --git a/src/process.c b/src/process.c index 307e82819d6..77e99ead01f 100644 --- a/src/process.c +++ b/src/process.c @@ -1759,12 +1759,10 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) #endif } #else /* not HAVE_SETSID */ -#ifdef USG - /* It's very important to call setpgrp here and no time + /* It's very important to call setpgid here and no time afterwards. Otherwise, we lose our controlling tty which is set when we open the pty. */ - setpgrp (); -#endif /* USG */ + setpgid (0, 0); #endif /* not HAVE_SETSID */ #if defined (LDISC1) if (pty_flag && xforkin >= 0) @@ -1802,11 +1800,7 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir) /* In order to get a controlling terminal on some versions of BSD, it is necessary to put the process in pgrp 0 before it opens the terminal. */ -#ifdef HAVE_SETPGID setpgid (0, 0); -#else - setpgrp (0, 0); -#endif #endif } #endif /* TIOCNOTTY */ diff --git a/src/sysdep.c b/src/sysdep.c index c7174e91612..63eac5d9e09 100644 --- a/src/sysdep.c +++ b/src/sysdep.c @@ -682,6 +682,75 @@ ignore_sigio (void) #endif } + +/* Saving and restoring the process group of Emacs's terminal. */ + +/* The process group of which Emacs was a member when it initially + started. + + If Emacs was in its own process group (i.e. inherited_pgroup == + getpid ()), then we know we're running under a shell with job + control (Emacs would never be run as part of a pipeline). + Everything is fine. + + If Emacs was not in its own process group, then we know we're + running under a shell (or a caller) that doesn't know how to + separate itself from Emacs (like sh). Emacs must be in its own + process group in order to receive SIGIO correctly. In this + situation, we put ourselves in our own pgroup, forcibly set the + tty's pgroup to our pgroup, and make sure to restore and reinstate + the tty's pgroup just like any other terminal setting. If + inherited_group was not the tty's pgroup, then we'll get a + SIGTTmumble when we try to change the tty's pgroup, and a CONT if + it goes foreground in the future, which is what should happen. */ + +static pid_t inherited_pgroup; + +void +init_foreground_group (void) +{ + pid_t pgrp = EMACS_GETPGRP (0); + inherited_pgroup = getpid () == pgrp ? 0 : pgrp; +} + +/* Safely set a controlling terminal FD's process group to PGID. + If we are not in the foreground already, POSIX requires tcsetpgrp + to deliver a SIGTTOU signal, which would stop us. This is an + annoyance, so temporarily ignore the signal. + + In practice, platforms lacking SIGTTOU also lack tcsetpgrp, so + skip all this unless SIGTTOU is defined. */ +static void +tcsetpgrp_without_stopping (int fd, pid_t pgid) +{ +#ifdef SIGTTOU + signal_handler_t handler; + block_input (); + handler = signal (SIGTTOU, SIG_IGN); + tcsetpgrp (fd, pgid); + signal (SIGTTOU, handler); + unblock_input (); +#endif +} + +/* Split off the foreground process group to Emacs alone. When we are + in the foreground, but not started in our own process group, + redirect the tty device handle FD to point to our own process + group. FD must be the file descriptor of the controlling tty. */ +static void +narrow_foreground_group (int fd) +{ + if (inherited_pgroup && setpgid (0, 0) == 0) + tcsetpgrp_without_stopping (fd, getpid ()); +} + +/* Set the tty to our original foreground group. */ +static void +widen_foreground_group (int fd) +{ + if (inherited_pgroup && setpgid (0, inherited_pgroup) == 0) + tcsetpgrp_without_stopping (fd, inherited_pgroup); +} /* Getting and setting emacs_tty structures. */ @@ -799,6 +868,8 @@ init_sys_modes (struct tty_display_info *tty_out) if (!tty_out->output) return; /* The tty is suspended. */ + narrow_foreground_group (fileno (tty_out->input)); + if (! tty_out->old_tty) tty_out->old_tty = xmalloc (sizeof *tty_out->old_tty); @@ -1231,6 +1302,7 @@ reset_sys_modes (struct tty_display_info *tty_out) dos_ttcooked (); #endif + widen_foreground_group (fileno (tty_out->input)); } #ifdef HAVE_PTYS