* bug#12697: 24.2; Emacs crashes when using it as the commit editor for git @ 2012-10-21 20:17 Matthew Leach 2012-10-22 8:26 ` Andreas Schwab ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Matthew Leach @ 2012-10-21 20:17 UTC (permalink / raw) To: 12697 When I use Emacs as the editor for commit messages in git, by way of setting the EDITOR environment variable, i.e. "export EDITOR='emacs -nw '" in my .bashrc, if I press C-g when Emacs opens this causes Emacs to crash and not shut down correctly as the console is not reset properly and text from the buffer remains on the screen when I am returned to a terminal prompt. I am using git version 1.7.12.4 In GNU Emacs 24.2.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.4.4) of 2012-08-28 on shaun Windowing system distributor `The X.Org Foundation', version 11.0.11300000 Configured using: `configure '--prefix=/usr' '--sysconfdir=/etc' '--libexecdir=/usr/lib' '--localstatedir=/var' '--with-x-toolkit=gtk3' '--with-xft' 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector --param=ssp-buffer-size=4 -D_FORTIFY_SOURCE=2' 'LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro'' Important settings: value of $LC_ALL: nil value of $LC_COLLATE: nil value of $LC_CTYPE: nil value of $LC_MESSAGES: nil value of $LC_MONETARY: nil value of $LC_NUMERIC: nil value of $LC_TIME: nil value of $LANG: en_GB.UTF-8 value of $XMODIFIERS: nil locale-coding-system: utf-8-unix default enable-multibyte-characters: t Major mode: Fundamental Minor modes in effect: show-paren-mode: t tooltip-mode: t mouse-wheel-mode: t tool-bar-mode: t menu-bar-mode: t file-name-shadow-mode: t global-font-lock-mode: t blink-cursor-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t line-number-mode: t transient-mark-mode: t Recent input: <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <help-echo> <menu-bar> <help-menu> <send-emacs-bug-report> Recent messages: For information about GNU Emacs and the GNU system, type C-h C-a. Load-path shadows: None found. Features: (shadow sort gnus-util mail-extr emacsbug message idna format-spec rfc822 mml easymenu mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail regexp-opt rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils edmacro kmacro tango-dark-theme paren ido time-date tooltip ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd tool-bar dnd fontset image fringe lisp-mode register page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock syntax facemenu font-core frame cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer loaddefs button faces cus-face files text-properties overlay sha1 md5 base64 format env code-pages mule custom widget hashtable-print-readable backquote make-network-process dbusbind dynamic-setting system-font-setting font-render-setting move-toolbar gtk x-toolkit x multi-tty emacs) ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#12697: 24.2; Emacs crashes when using it as the commit editor for git 2012-10-21 20:17 bug#12697: 24.2; Emacs crashes when using it as the commit editor for git Matthew Leach @ 2012-10-22 8:26 ` Andreas Schwab 2012-10-22 17:15 ` Eli Zaretskii 2012-10-27 5:31 ` bug#12697: " Paul Eggert 2012-10-31 17:28 ` Paul Eggert 2 siblings, 1 reply; 10+ messages in thread From: Andreas Schwab @ 2012-10-22 8:26 UTC (permalink / raw) To: Matthew Leach; +Cc: 12697 Matthew Leach <matthew@mattleach.net> writes: > When I use Emacs as the editor for commit messages in git, by way of > setting the EDITOR environment variable, i.e. "export EDITOR='emacs -nw > '" in my .bashrc, if I press C-g when Emacs opens this causes Emacs to C-g in emacs -nw is equivalent to SIGINT on the controlling terminal. This causes all processes in the same process group to receive SIGINT, which includes the parent git process, now that emacs no longer puts itself and the terminal into its own process group. 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] 10+ messages in thread
* bug#12697: 24.2; Emacs crashes when using it as the commit editor for git 2012-10-22 8:26 ` Andreas Schwab @ 2012-10-22 17:15 ` Eli Zaretskii 2012-10-22 17:39 ` Andreas Schwab 0 siblings, 1 reply; 10+ messages in thread From: Eli Zaretskii @ 2012-10-22 17:15 UTC (permalink / raw) To: Andreas Schwab; +Cc: matthew, 12697 > From: Andreas Schwab <schwab@linux-m68k.org> > Date: Mon, 22 Oct 2012 10:26:35 +0200 > Cc: 12697@debbugs.gnu.org > > C-g in emacs -nw is equivalent to SIGINT on the controlling terminal. > This causes all processes in the same process group to receive SIGINT, > which includes the parent git process Shouldn't git block SIGINT when it invokes $EDITOR? > now that emacs no longer puts itself and the terminal into its own > process group. Did we have good reason to stop doing that? ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#12697: 24.2; Emacs crashes when using it as the commit editor for git 2012-10-22 17:15 ` Eli Zaretskii @ 2012-10-22 17:39 ` Andreas Schwab 2012-10-22 20:22 ` Eli Zaretskii 0 siblings, 1 reply; 10+ messages in thread From: Andreas Schwab @ 2012-10-22 17:39 UTC (permalink / raw) To: Eli Zaretskii; +Cc: matthew, 12697 Eli Zaretskii <eliz@gnu.org> writes: >> C-g in emacs -nw is equivalent to SIGINT on the controlling terminal. >> This causes all processes in the same process group to receive SIGINT, >> which includes the parent git process > > Shouldn't git block SIGINT when it invokes $EDITOR? Maybe. Perhaps the OP should report it to the git list. >> now that emacs no longer puts itself and the terminal into its own >> process group. > > Did we have good reason to stop doing that? IMHO no. All the BSD_PGRPS stuff was removed on the premise that it wasn't used any more, but disabling BSD_PGRPS in s/gnu-linux.h was already questionable. 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] 10+ messages in thread
* bug#12697: 24.2; Emacs crashes when using it as the commit editor for git 2012-10-22 17:39 ` Andreas Schwab @ 2012-10-22 20:22 ` Eli Zaretskii 2012-10-22 21:34 ` Andreas Schwab 0 siblings, 1 reply; 10+ messages in thread From: Eli Zaretskii @ 2012-10-22 20:22 UTC (permalink / raw) To: Andreas Schwab; +Cc: matthew, 12697 > From: Andreas Schwab <schwab@linux-m68k.org> > Cc: matthew@mattleach.net, 12697@debbugs.gnu.org > Date: Mon, 22 Oct 2012 19:39:55 +0200 > > >> now that emacs no longer puts itself and the terminal into its own > >> process group. > > > > Did we have good reason to stop doing that? > > IMHO no. All the BSD_PGRPS stuff was removed on the premise that it > wasn't used any more, but disabling BSD_PGRPS in s/gnu-linux.h was > already questionable. Come think about it, why not stop depending on SIGINT for quitting? We already do that in GUI session (AFAIU), so why should TTY frames be different? We could just detect C-g in the SIGIO handler, no? If this makes sense, then this whole issue would go away. Of course, we will then lose the "emergency exit" feature, but since core dumps are reportedly disabled on many systems anyway, that doesn't sound like too much of a loss, and GUI sessions already cannot use it anyway. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#12697: 24.2; Emacs crashes when using it as the commit editor for git 2012-10-22 20:22 ` Eli Zaretskii @ 2012-10-22 21:34 ` Andreas Schwab 0 siblings, 0 replies; 10+ messages in thread From: Andreas Schwab @ 2012-10-22 21:34 UTC (permalink / raw) To: Eli Zaretskii; +Cc: matthew, 12697 Eli Zaretskii <eliz@gnu.org> writes: > We could just detect C-g in the SIGIO handler, no? That works already (just disable ISIG in init_sys_modes to try and see). 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] 10+ messages in thread
* bug#12697: Emacs crashes when using it as the commit editor for git 2012-10-21 20:17 bug#12697: 24.2; Emacs crashes when using it as the commit editor for git Matthew Leach 2012-10-22 8:26 ` Andreas Schwab @ 2012-10-27 5:31 ` Paul Eggert 2012-10-27 7:52 ` Eli Zaretskii 2012-10-31 17:28 ` Paul Eggert 2 siblings, 1 reply; 10+ messages in thread From: Paul Eggert @ 2012-10-27 5:31 UTC (permalink / raw) To: 12697; +Cc: Matthew Leach, Andreas Schwab [-- Attachment #1: Type: text/plain, Size: 391 bytes --] I tried Andreas's suggestion of disabling ISIG, but this didn't work on Solaris, and it doesn't look like it'd work on any host where USABLE_SIGIO is false. Instead, how about the attached patch? It reenables the BSD_PGRPS idea, although reimplemented, as the old code can't easily be resurrected as-is. I haven't thought about how this affects the Windows port, so I'll CC: this to Eli. [-- Attachment #2: git-commit.txt --] [-- Type: text/plain, Size: 8107 bytes --] === modified file 'src/ChangeLog' --- src/ChangeLog 2012-10-26 09:46:46 +0000 +++ src/ChangeLog 2012-10-27 05:22:54 +0000 @@ -1,3 +1,24 @@ +2012-10-27 Paul Eggert <eggert@cs.ucla.edu> + + 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-26 Eli Zaretskii <eliz@gnu.org> * w32fns.c (w32_wnd_proc) <WM_MOUSEMOVE>: Don't enable tracking of === modified file 'src/callproc.c' --- src/callproc.c 2012-10-19 19:25:18 +0000 +++ src/callproc.c 2012-10-27 05:22:54 +0000 @@ -64,13 +64,6 @@ #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 @@ { if (fd[0] >= 0) emacs_close (fd[0]); + #ifdef HAVE_SETSID setsid (); +#else + setpgid (0, 0); #endif -#if defined (USG) - setpgrp (); -#else - setpgrp (pid, pid); -#endif /* USG */ /* Emacs ignores SIGPIPE, but the child should not. */ signal (SIGPIPE, SIG_DFL); @@ -1295,13 +1286,9 @@ 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); === modified file 'src/conf_post.h' --- src/conf_post.h 2012-10-19 19:25:18 +0000 +++ src/conf_post.h 2012-10-27 05:07:17 +0000 @@ -112,6 +112,14 @@ #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 === modified file 'src/dispnew.c' --- src/dispnew.c 2012-10-17 19:02:44 +0000 +++ src/dispnew.c 2012-10-27 05:07:17 +0000 @@ -6283,6 +6283,8 @@ 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. */ === modified file 'src/emacs.c' --- src/emacs.c 2012-10-25 04:35:39 +0000 +++ src/emacs.c 2012-10-27 05:07:17 +0000 @@ -1091,19 +1091,14 @@ #endif /* DOS_NT */ } - 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); + if (! noninteractive) + { + extern void malloc_enable_thread (void); - malloc_enable_thread (); - } + malloc_enable_thread (); + } #endif - } init_signals (dumping); === modified file 'src/lisp.h' --- src/lisp.h 2012-10-21 07:23:34 +0000 +++ src/lisp.h 2012-10-27 05:07:17 +0000 @@ -3474,6 +3474,7 @@ 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); === modified file 'src/process.c' --- src/process.c 2012-10-19 19:25:18 +0000 +++ src/process.c 2012-10-27 05:07:17 +0000 @@ -1759,12 +1759,10 @@ #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 @@ /* 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 */ === modified file 'src/sysdep.c' --- src/sysdep.c 2012-10-25 04:35:39 +0000 +++ src/sysdep.c 2012-10-27 05:07:17 +0000 @@ -683,6 +683,70 @@ } \f +/* 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. */ +static void +tcsetpgrp_without_stopping (int fd, pid_t pgid) +{ + signal_handler_t handler; + block_input (); + handler = signal (SIGTTOU, SIG_IGN); + tcsetpgrp (fd, pgid); + signal (SIGTTOU, handler); + unblock_input (); +} + +/* 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); +} +\f /* Getting and setting emacs_tty structures. */ /* Set *TC to the parameters associated with the terminal FD. @@ -799,6 +863,8 @@ 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 +1297,7 @@ dos_ttcooked (); #endif + widen_foreground_group (fileno (tty_out->input)); } \f #ifdef HAVE_PTYS ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#12697: Emacs crashes when using it as the commit editor for git 2012-10-27 5:31 ` bug#12697: " Paul Eggert @ 2012-10-27 7:52 ` Eli Zaretskii 2012-10-28 18:57 ` Paul Eggert 0 siblings, 1 reply; 10+ messages in thread From: Eli Zaretskii @ 2012-10-27 7:52 UTC (permalink / raw) To: Paul Eggert; +Cc: matthew, 12697, schwab > Date: Fri, 26 Oct 2012 22:31:21 -0700 > From: Paul Eggert <eggert@cs.ucla.edu> > CC: Andreas Schwab <schwab@linux-m68k.org>, Eli Zaretskii <eliz@gnu.org>, > Matthew Leach <matthew@mattleach.net> > > Instead, how about the attached patch? It reenables > the BSD_PGRPS idea, although reimplemented, as the old > code can't easily be resurrected as-is. I haven't > thought about how this affects the Windows port The termios-related stuff (the call to tcsetgrp) in sysdep.c should be #ifedf'ed away for Windows (for DOS_NT, actually). Also, the references to SIGTTOU should be conditioned on that signal being defined. Other than that, this should be OK, I think, since, if setpgid is not available, it is redirected to setpgrp, which already has a no-op implementation for Windows. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#12697: Emacs crashes when using it as the commit editor for git 2012-10-27 7:52 ` Eli Zaretskii @ 2012-10-28 18:57 ` Paul Eggert 0 siblings, 0 replies; 10+ messages in thread From: Paul Eggert @ 2012-10-28 18:57 UTC (permalink / raw) To: Eli Zaretskii; +Cc: matthew, 12697, schwab [-- Attachment #1: Type: text/plain, Size: 1412 bytes --] On 10/27/2012 12:52 AM, Eli Zaretskii wrote: > The termios-related stuff (the call to tcsetgrp) in sysdep.c should be > #ifedf'ed away for Windows (for DOS_NT, actually). Also, the > references to SIGTTOU should be conditioned on that signal being > defined. Thanks, that problem occurs with older POSIXish systems too. Here's a simple fix for it. I'm attaching the resulting combined patch, relative to trunk bzr 110706. The bug is a relatively serious one, so I'm inclined to install the combined patch soon. === modified file 'src/sysdep.c' --- src/sysdep.c 2012-10-27 05:07:17 +0000 +++ src/sysdep.c 2012-10-28 18:39:51 +0000 @@ -716,16 +716,21 @@ /* 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. */ + 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 [-- Attachment #2: git-commit.txt --] [-- Type: text/plain, Size: 8224 bytes --] === modified file 'src/ChangeLog' --- src/ChangeLog 2012-10-28 17:42:52 +0000 +++ src/ChangeLog 2012-10-28 18:33:41 +0000 @@ -1,3 +1,24 @@ +2012-10-28 Paul Eggert <eggert@cs.ucla.edu> + + 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-28 Eli Zaretskii <eliz@gnu.org> * w32proc.c (TIMER_TICKS_PER_SEC): New macro. === modified file 'src/callproc.c' --- src/callproc.c 2012-10-19 19:25:18 +0000 +++ src/callproc.c 2012-10-27 05:22:54 +0000 @@ -64,13 +64,6 @@ #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 @@ { if (fd[0] >= 0) emacs_close (fd[0]); + #ifdef HAVE_SETSID setsid (); +#else + setpgid (0, 0); #endif -#if defined (USG) - setpgrp (); -#else - setpgrp (pid, pid); -#endif /* USG */ /* Emacs ignores SIGPIPE, but the child should not. */ signal (SIGPIPE, SIG_DFL); @@ -1295,13 +1286,9 @@ 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); === modified file 'src/conf_post.h' --- src/conf_post.h 2012-10-19 19:25:18 +0000 +++ src/conf_post.h 2012-10-27 05:07:17 +0000 @@ -112,6 +112,14 @@ #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 === modified file 'src/dispnew.c' --- src/dispnew.c 2012-10-17 19:02:44 +0000 +++ src/dispnew.c 2012-10-27 05:07:17 +0000 @@ -6283,6 +6283,8 @@ 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. */ === modified file 'src/emacs.c' --- src/emacs.c 2012-10-25 04:35:39 +0000 +++ src/emacs.c 2012-10-27 05:07:17 +0000 @@ -1091,19 +1091,14 @@ #endif /* DOS_NT */ } - 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); + if (! noninteractive) + { + extern void malloc_enable_thread (void); - malloc_enable_thread (); - } + malloc_enable_thread (); + } #endif - } init_signals (dumping); === modified file 'src/lisp.h' --- src/lisp.h 2012-10-21 07:23:34 +0000 +++ src/lisp.h 2012-10-27 05:07:17 +0000 @@ -3474,6 +3474,7 @@ 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); === modified file 'src/process.c' --- src/process.c 2012-10-19 19:25:18 +0000 +++ src/process.c 2012-10-27 05:07:17 +0000 @@ -1759,12 +1759,10 @@ #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 @@ /* 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 */ === modified file 'src/sysdep.c' --- src/sysdep.c 2012-10-25 04:35:39 +0000 +++ src/sysdep.c 2012-10-28 18:50:00 +0000 @@ -683,6 +683,75 @@ } \f +/* 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); +} +\f /* Getting and setting emacs_tty structures. */ /* Set *TC to the parameters associated with the terminal FD. @@ -799,6 +868,8 @@ 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 @@ dos_ttcooked (); #endif + widen_foreground_group (fileno (tty_out->input)); } \f #ifdef HAVE_PTYS ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#12697: Emacs crashes when using it as the commit editor for git 2012-10-21 20:17 bug#12697: 24.2; Emacs crashes when using it as the commit editor for git Matthew Leach 2012-10-22 8:26 ` Andreas Schwab 2012-10-27 5:31 ` bug#12697: " Paul Eggert @ 2012-10-31 17:28 ` Paul Eggert 2 siblings, 0 replies; 10+ messages in thread From: Paul Eggert @ 2012-10-31 17:28 UTC (permalink / raw) To: 12697-done I installed the patch into the trunk as bzr 110750 and am marking this as done. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-10-31 17:28 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-10-21 20:17 bug#12697: 24.2; Emacs crashes when using it as the commit editor for git Matthew Leach 2012-10-22 8:26 ` Andreas Schwab 2012-10-22 17:15 ` Eli Zaretskii 2012-10-22 17:39 ` Andreas Schwab 2012-10-22 20:22 ` Eli Zaretskii 2012-10-22 21:34 ` Andreas Schwab 2012-10-27 5:31 ` bug#12697: " Paul Eggert 2012-10-27 7:52 ` Eli Zaretskii 2012-10-28 18:57 ` Paul Eggert 2012-10-31 17:28 ` Paul Eggert
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).