* bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin) @ 2010-10-15 23:36 Ken Brown 2010-10-16 7:32 ` Eli Zaretskii 0 siblings, 1 reply; 13+ messages in thread From: Ken Brown @ 2010-10-15 23:36 UTC (permalink / raw) To: 7225 I think I can fix a longstanding bug in the Cygwin build of emacs, in which C-c C-c doesn't work to interrupt a process in shell mode. Since the problem doesn't seem to be in the bug database, I'm reporting it first for documentation purposes, and then I'll give the proposed fix. To reproduce: emacs -Q M-x shell cat C-c C-c (The cat process doesn't get killed.) The following patch seems to fix it: === modified file 'src/s/cygwin.h' --- src/s/cygwin.h 2010-01-13 08:35:10 +0000 +++ src/s/cygwin.h 2010-10-15 22:20:39 +0000 @@ -132,6 +132,9 @@ returns ENOSYS. A workaround is to set G_SLICE=always-malloc. */ #define G_SLICE_ALWAYS_MALLOC +/* Send signals to subprocesses by "typing" special chars at them. */ +#define SIGNALS_VIA_CHARACTERS + /* the end */ /* arch-tag: 5ae7ba00-83b0-4ab3-806a-3e845779191b I'd like to apply this to the emacs-23 branch. Does anyone see a problem with it or anything further I should test? Ken ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin) 2010-10-15 23:36 bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin) Ken Brown @ 2010-10-16 7:32 ` Eli Zaretskii 2010-10-16 12:44 ` Ken Brown 0 siblings, 1 reply; 13+ messages in thread From: Eli Zaretskii @ 2010-10-16 7:32 UTC (permalink / raw) To: Ken Brown; +Cc: 7225 > Date: Fri, 15 Oct 2010 19:36:36 -0400 > From: Ken Brown <kbrown@cornell.edu> > Cc: > > I think I can fix a longstanding bug in the Cygwin build of emacs, in > which C-c C-c doesn't work to interrupt a process in shell mode. Since > the problem doesn't seem to be in the bug database, I'm reporting it > first for documentation purposes, and then I'll give the proposed fix. > To reproduce: > > emacs -Q > M-x shell > cat > C-c C-c > (The cat process doesn't get killed.) Is the reason for this known? It looks like the `kill' syscall isn't doing its job, but if that's so, there should be a good reason for that. > The following patch seems to fix it: > > === modified file 'src/s/cygwin.h' > --- src/s/cygwin.h 2010-01-13 08:35:10 +0000 > +++ src/s/cygwin.h 2010-10-15 22:20:39 +0000 > @@ -132,6 +132,9 @@ > returns ENOSYS. A workaround is to set G_SLICE=always-malloc. */ > #define G_SLICE_ALWAYS_MALLOC > > +/* Send signals to subprocesses by "typing" special chars at them. */ > +#define SIGNALS_VIA_CHARACTERS > + > /* the end */ > > /* arch-tag: 5ae7ba00-83b0-4ab3-806a-3e845779191b > > I'd like to apply this to the emacs-23 branch. Does anyone see a > problem with it or anything further I should test? Did you test it with a subprogram that puts its stdin into a mode where (speaking in termios terms) the ISIG flag in c_lflag passed to tcsetattr is unset? In any case, I wouldn't recommend to take this workaround unless we understand why this doesn't work without it. ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin) 2010-10-16 7:32 ` Eli Zaretskii @ 2010-10-16 12:44 ` Ken Brown 2010-10-16 13:26 ` Eli Zaretskii 0 siblings, 1 reply; 13+ messages in thread From: Ken Brown @ 2010-10-16 12:44 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 7225@debbugs.gnu.org On 10/16/2010 3:32 AM, Eli Zaretskii wrote: >> Date: Fri, 15 Oct 2010 19:36:36 -0400 >> From: Ken Brown<kbrown@cornell.edu> >> Cc: >> >> I think I can fix a longstanding bug in the Cygwin build of emacs, in >> which C-c C-c doesn't work to interrupt a process in shell mode. Since >> the problem doesn't seem to be in the bug database, I'm reporting it >> first for documentation purposes, and then I'll give the proposed fix. >> To reproduce: >> >> emacs -Q >> M-x shell >> cat >> C-c C-c >> (The cat process doesn't get killed.) > > Is the reason for this known? It looks like the `kill' syscall isn't > doing its job, but if that's so, there should be a good reason for > that. The problem is the group id in the call to 'kill'. Cygwin doesn't define TIOCGPGRP, and the group id as set in line 6233 of process.c (emacs-23 branch) is the wrong group id. >> The following patch seems to fix it: >> >> === modified file 'src/s/cygwin.h' >> --- src/s/cygwin.h 2010-01-13 08:35:10 +0000 >> +++ src/s/cygwin.h 2010-10-15 22:20:39 +0000 >> @@ -132,6 +132,9 @@ >> returns ENOSYS. A workaround is to set G_SLICE=always-malloc. */ >> #define G_SLICE_ALWAYS_MALLOC >> >> +/* Send signals to subprocesses by "typing" special chars at them. */ >> +#define SIGNALS_VIA_CHARACTERS >> + >> /* the end */ >> >> /* arch-tag: 5ae7ba00-83b0-4ab3-806a-3e845779191b >> >> I'd like to apply this to the emacs-23 branch. Does anyone see a >> problem with it or anything further I should test? > > Did you test it with a subprogram that puts its stdin into a mode > where (speaking in termios terms) the ISIG flag in c_lflag passed to > tcsetattr is unset? I don't know how to do this. Can you give me a short test program to try? > In any case, I wouldn't recommend to take this workaround unless we > understand why this doesn't work without it. I'm not sure why you call it a workaround. The comments preceding (and in) the definition of process_send_signal make it clear that SIGNALS_VIA_CHARACTERS is the preferred method. It's used by GNU/Linux (see src/s/gnu-linux.h) and several other systems. ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin) 2010-10-16 12:44 ` Ken Brown @ 2010-10-16 13:26 ` Eli Zaretskii 2010-10-16 18:22 ` Ken Brown 2010-10-17 21:40 ` Ken Brown 0 siblings, 2 replies; 13+ messages in thread From: Eli Zaretskii @ 2010-10-16 13:26 UTC (permalink / raw) To: Ken Brown; +Cc: 7225 > Date: Sat, 16 Oct 2010 08:44:28 -0400 > From: Ken Brown <kbrown@cornell.edu> > CC: "7225@debbugs.gnu.org" <7225@debbugs.gnu.org> > > >> C-c C-c > >> (The cat process doesn't get killed.) > > > > Is the reason for this known? It looks like the `kill' syscall isn't > > doing its job, but if that's so, there should be a good reason for > > that. > > The problem is the group id in the call to 'kill'. Cygwin doesn't > define TIOCGPGRP, and the group id as set in line 6233 of process.c > (emacs-23 branch) is the wrong group id. Does Cygwin have another way of getting the process group ID? If so, can it be used in process_send_signal instead of emacs_get_tty_pgrp? > > Did you test it with a subprogram that puts its stdin into a mode > > where (speaking in termios terms) the ISIG flag in c_lflag passed to > > tcsetattr is unset? > > I don't know how to do this. Can you give me a short test program to try? Sorry, no, not off-hand. > > In any case, I wouldn't recommend to take this workaround unless we > > understand why this doesn't work without it. > > I'm not sure why you call it a workaround. The comments preceding (and > in) the definition of process_send_signal make it clear that > SIGNALS_VIA_CHARACTERS is the preferred method. It's used by GNU/Linux > (see src/s/gnu-linux.h) and several other systems. SIGNALS_VIA_CHARACTERS is the preferred method for a few signals that are raised by keyboard characters. But unless you find a way to send any signal to a subprocess on Cygwin, you will still have a bug, although not with SIGINT. So I suggest to look into tweaking process_send_signal so that it works with Cygwin. It looks like it makes all kinds of assumptions regarding systems that don't have TIOCGPGRP, and they are probably wrong for Cyugwin. Or maybe you need something special in EMACS_KILLPG. When you have fixed process_send_signal, you can then introduce SIGNALS_VIA_CHARACTERS for Cygwin. However, this is just a suggestion. If you don't feel like digging into these issues, go ahead and commit the change. ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin) 2010-10-16 13:26 ` Eli Zaretskii @ 2010-10-16 18:22 ` Ken Brown 2010-10-17 21:40 ` Ken Brown 1 sibling, 0 replies; 13+ messages in thread From: Ken Brown @ 2010-10-16 18:22 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 7225@debbugs.gnu.org On 10/16/2010 9:26 AM, Eli Zaretskii wrote: >> Date: Sat, 16 Oct 2010 08:44:28 -0400 >> From: Ken Brown<kbrown@cornell.edu> >> CC: "7225@debbugs.gnu.org"<7225@debbugs.gnu.org> >> >>>> C-c C-c >>>> (The cat process doesn't get killed.) >>> >>> Is the reason for this known? It looks like the `kill' syscall isn't >>> doing its job, but if that's so, there should be a good reason for >>> that. >> >> The problem is the group id in the call to 'kill'. Cygwin doesn't >> define TIOCGPGRP, and the group id as set in line 6233 of process.c >> (emacs-23 branch) is the wrong group id. > > Does Cygwin have another way of getting the process group ID? If so, > can it be used in process_send_signal instead of emacs_get_tty_pgrp? > >>> Did you test it with a subprogram that puts its stdin into a mode >>> where (speaking in termios terms) the ISIG flag in c_lflag passed to >>> tcsetattr is unset? >> >> I don't know how to do this. Can you give me a short test program to try? > > Sorry, no, not off-hand. > >>> In any case, I wouldn't recommend to take this workaround unless we >>> understand why this doesn't work without it. >> >> I'm not sure why you call it a workaround. The comments preceding (and >> in) the definition of process_send_signal make it clear that >> SIGNALS_VIA_CHARACTERS is the preferred method. It's used by GNU/Linux >> (see src/s/gnu-linux.h) and several other systems. > > SIGNALS_VIA_CHARACTERS is the preferred method for a few signals that > are raised by keyboard characters. But unless you find a way to send > any signal to a subprocess on Cygwin, you will still have a bug, > although not with SIGINT. OK, now I understand. > So I suggest to look into tweaking process_send_signal so that it > works with Cygwin. It looks like it makes all kinds of assumptions > regarding systems that don't have TIOCGPGRP, and they are probably > wrong for Cyugwin. Or maybe you need something special in > EMACS_KILLPG. > > When you have fixed process_send_signal, you can then introduce > SIGNALS_VIA_CHARACTERS for Cygwin. > > However, this is just a suggestion. If you don't feel like digging > into these issues, go ahead and commit the change. Thanks for the suggestion. I would like to dig into this, but I don't really know enough. I'll see if I can get some help on the Cygwin list. Ken ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin) 2010-10-16 13:26 ` Eli Zaretskii 2010-10-16 18:22 ` Ken Brown @ 2010-10-17 21:40 ` Ken Brown 2010-10-17 22:08 ` Eli Zaretskii 1 sibling, 1 reply; 13+ messages in thread From: Ken Brown @ 2010-10-17 21:40 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 7225@debbugs.gnu.org On 10/16/2010 9:26 AM, Eli Zaretskii wrote: >> Date: Sat, 16 Oct 2010 08:44:28 -0400 >> From: Ken Brown<kbrown@cornell.edu> >> CC: "7225@debbugs.gnu.org"<7225@debbugs.gnu.org> >> >>>> C-c C-c >>>> (The cat process doesn't get killed.) >>> >>> Is the reason for this known? It looks like the `kill' syscall isn't >>> doing its job, but if that's so, there should be a good reason for >>> that. >> >> The problem is the group id in the call to 'kill'. Cygwin doesn't >> define TIOCGPGRP, and the group id as set in line 6233 of process.c >> (emacs-23 branch) is the wrong group id. > > Does Cygwin have another way of getting the process group ID? If so, > can it be used in process_send_signal instead of emacs_get_tty_pgrp? [...] > So I suggest to look into tweaking process_send_signal so that it > works with Cygwin. It looks like it makes all kinds of assumptions > regarding systems that don't have TIOCGPGRP, and they are probably > wrong for Cyugwin. Or maybe you need something special in > EMACS_KILLPG. > > When you have fixed process_send_signal, you can then introduce > SIGNALS_VIA_CHARACTERS for Cygwin. I haven't yet found an alternative to TIOCGPGRP that works for Cygwin, and it may well be that there isn't one. But I think there's a separate issue that is not specific to Cygwin. It seems to me that line 6233 of process.c is simply wrong, and not just for Cygwin. When we reach that line, we want the process group ID of the foreground process group of the terminal associated with p (which is a shell in the most common use case). We don't have TIOCGPGRP, so we don't know how to do this. We therefore give up and set gid = p->pid. Is there any situation in which this is the right thing to do? It means that (in the common use case) we'll send the signal to the shell instead of to the process running in the shell. Wouldn't it be better to just return at that point and issue a warning message saying that we can't send the signal? What this would mean for Cygwin, once I make the change I proposed (and assuming I don't find a better solution), is that the only signals we'll be able to send to the foreground process of a shell are SIGINT, SIGQUIT, and SIGTSTP, and we'll see a failure message if we try to send a different signal. That's better than sending a signal to the wrong process. ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin) 2010-10-17 21:40 ` Ken Brown @ 2010-10-17 22:08 ` Eli Zaretskii 2010-10-17 23:11 ` Ken Brown 2010-10-18 5:49 ` Jan Djärv 0 siblings, 2 replies; 13+ messages in thread From: Eli Zaretskii @ 2010-10-17 22:08 UTC (permalink / raw) To: Ken Brown; +Cc: 7225 > Date: Sun, 17 Oct 2010 17:40:39 -0400 > From: Ken Brown <kbrown@cornell.edu> > CC: "7225@debbugs.gnu.org" <7225@debbugs.gnu.org> > > I haven't yet found an alternative to TIOCGPGRP that works for Cygwin, > and it may well be that there isn't one. But I think there's a separate > issue that is not specific to Cygwin. It seems to me that line 6233 of > process.c is simply wrong, and not just for Cygwin. > > When we reach that line, we want the process group ID of the foreground > process group of the terminal associated with p (which is a shell in the > most common use case). We don't have TIOCGPGRP, so we don't know how to > do this. We therefore give up and set gid = p->pid. Is there any > situation in which this is the right thing to do? It means that (in the > common use case) we'll send the signal to the shell instead of to the > process running in the shell. Wouldn't it be better to just return at > that point and issue a warning message saying that we can't send the signal? The problem that code is trying to solve is how to send a signal to the whole process group starting at the shell (or whatever process is the group leader). Failure to do so could mean that the immediate subprocess of Emacs will get the signal, but its children will not. If the signal kills the subprocess, its children may remain behind as orphans. > What this would mean for Cygwin, once I make the change I proposed (and > assuming I don't find a better solution), is that the only signals we'll > be able to send to the foreground process of a shell are SIGINT, > SIGQUIT, and SIGTSTP, and we'll see a failure message if we try to send > a different signal. That's better than sending a signal to the wrong > process. It's not the wrong process. The problem is that its children might not get the signal. So the question is: how to send a signal to the whole process group with Cygwin? Does Cygwin support the "minus pgid" method of calling `kill'? Or maybe there's some other method? ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin) 2010-10-17 22:08 ` Eli Zaretskii @ 2010-10-17 23:11 ` Ken Brown 2010-10-17 23:39 ` Ken Brown 2010-10-18 6:59 ` Eli Zaretskii 2010-10-18 5:49 ` Jan Djärv 1 sibling, 2 replies; 13+ messages in thread From: Ken Brown @ 2010-10-17 23:11 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 7225@debbugs.gnu.org On 10/17/2010 6:08 PM, Eli Zaretskii wrote: >> Date: Sun, 17 Oct 2010 17:40:39 -0400 >> From: Ken Brown<kbrown@cornell.edu> >> CC: "7225@debbugs.gnu.org"<7225@debbugs.gnu.org> >> >> I haven't yet found an alternative to TIOCGPGRP that works for Cygwin, >> and it may well be that there isn't one. But I think there's a separate >> issue that is not specific to Cygwin. It seems to me that line 6233 of >> process.c is simply wrong, and not just for Cygwin. >> >> When we reach that line, we want the process group ID of the foreground >> process group of the terminal associated with p (which is a shell in the >> most common use case). We don't have TIOCGPGRP, so we don't know how to >> do this. We therefore give up and set gid = p->pid. Is there any >> situation in which this is the right thing to do? It means that (in the >> common use case) we'll send the signal to the shell instead of to the >> process running in the shell. Wouldn't it be better to just return at >> that point and issue a warning message saying that we can't send the signal? > > The problem that code is trying to solve is how to send a signal to > the whole process group starting at the shell (or whatever process is > the group leader). Failure to do so could mean that the immediate > subprocess of Emacs will get the signal, but its children will not. > If the signal kills the subprocess, its children may remain behind as > orphans. Am I misunderstanding the comment preceding the definition of emacs_get_tty_pgrp? Here's what it says: /* Return the foreground process group for the tty/pty that the process P uses. */ That's not the same as the process group of the shell, at least in Cygwin. See below. You seem to be assuming that the process group of the shell will include all of the shell's children. Is that what happens in GNU/Linux? >> What this would mean for Cygwin, once I make the change I proposed (and >> assuming I don't find a better solution), is that the only signals we'll >> be able to send to the foreground process of a shell are SIGINT, >> SIGQUIT, and SIGTSTP, and we'll see a failure message if we try to send >> a different signal. That's better than sending a signal to the wrong >> process. > > It's not the wrong process. The problem is that its children might > not get the signal. It is the wrong process on Cygwin. Here's an example. I start a shell and then run 'cat'. In another shell I run 'ps'. The relevant part of the output is: PID PPID PGID WINPID TTY UID STIME COMMAND 508 920 508 3552 1 1009 18:50:15 /usr/bin/sh I 916 508 916 1832 1 1009 18:50:18 /usr/bin/cat 508 is the PID as well as the PGID of the shell. But cat is in a different process group, even though it's a child of the shell (as we see from the PPID). On the other hand, it has the same controlling tty as the shell, so we'd be in business if we could get the PGID of the foreground process of the controlling tty of the shell. That's what I thought emacs_get_tty_pgrp was doing (according to its comment). When I type C-c C-c, emacs sends a signal to PGID 508, which is not the right thing to do. > So the question is: how to send a signal to the whole process group > with Cygwin? Does Cygwin support the "minus pgid" method of calling > `kill'? Yes. ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin) 2010-10-17 23:11 ` Ken Brown @ 2010-10-17 23:39 ` Ken Brown 2010-10-18 6:59 ` Eli Zaretskii 1 sibling, 0 replies; 13+ messages in thread From: Ken Brown @ 2010-10-17 23:39 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 7225@debbugs.gnu.org On 10/17/2010 7:11 PM, Ken Brown wrote: > On 10/17/2010 6:08 PM, Eli Zaretskii wrote: >>> Date: Sun, 17 Oct 2010 17:40:39 -0400 >>> From: Ken Brown<kbrown@cornell.edu> >>> CC: "7225@debbugs.gnu.org"<7225@debbugs.gnu.org> >>> >>> I haven't yet found an alternative to TIOCGPGRP that works for Cygwin, >>> and it may well be that there isn't one. But I think there's a separate >>> issue that is not specific to Cygwin. It seems to me that line 6233 of >>> process.c is simply wrong, and not just for Cygwin. >>> >>> When we reach that line, we want the process group ID of the foreground >>> process group of the terminal associated with p (which is a shell in the >>> most common use case). We don't have TIOCGPGRP, so we don't know how to >>> do this. We therefore give up and set gid = p->pid. Is there any >>> situation in which this is the right thing to do? It means that (in the >>> common use case) we'll send the signal to the shell instead of to the >>> process running in the shell. Wouldn't it be better to just return at >>> that point and issue a warning message saying that we can't send the signal? >> >> The problem that code is trying to solve is how to send a signal to >> the whole process group starting at the shell (or whatever process is >> the group leader). Failure to do so could mean that the immediate >> subprocess of Emacs will get the signal, but its children will not. >> If the signal kills the subprocess, its children may remain behind as >> orphans. > > Am I misunderstanding the comment preceding the definition of > emacs_get_tty_pgrp? Here's what it says: > > /* Return the foreground process group for the tty/pty that > the process P uses. */ > > That's not the same as the process group of the shell, at least in > Cygwin. See below. You seem to be assuming that the process group of > the shell will include all of the shell's children. Is that what > happens in GNU/Linux? > >>> What this would mean for Cygwin, once I make the change I proposed (and >>> assuming I don't find a better solution), is that the only signals we'll >>> be able to send to the foreground process of a shell are SIGINT, >>> SIGQUIT, and SIGTSTP, and we'll see a failure message if we try to send >>> a different signal. That's better than sending a signal to the wrong >>> process. >> >> It's not the wrong process. The problem is that its children might >> not get the signal. > > It is the wrong process on Cygwin. Here's an example. I start a shell > and then run 'cat'. In another shell I run 'ps'. The relevant part of > the output is: > > PID PPID PGID WINPID TTY UID STIME COMMAND > 508 920 508 3552 1 1009 18:50:15 /usr/bin/sh > I 916 508 916 1832 1 1009 18:50:18 /usr/bin/cat I've just checked that the same thing happens in GNU/Linux (or at least in the GNU/Linux system I have access to). Start a shell, start 'cat', and the PGID of cat will not be the same as that of the shell. ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin) 2010-10-17 23:11 ` Ken Brown 2010-10-17 23:39 ` Ken Brown @ 2010-10-18 6:59 ` Eli Zaretskii 2010-10-18 11:58 ` Ken Brown 1 sibling, 1 reply; 13+ messages in thread From: Eli Zaretskii @ 2010-10-18 6:59 UTC (permalink / raw) To: Ken Brown; +Cc: 7225 > Date: Sun, 17 Oct 2010 19:11:15 -0400 > From: Ken Brown <kbrown@cornell.edu> > CC: "7225@debbugs.gnu.org" <7225@debbugs.gnu.org> > > > The problem that code is trying to solve is how to send a signal to > > the whole process group starting at the shell (or whatever process is > > the group leader). Failure to do so could mean that the immediate > > subprocess of Emacs will get the signal, but its children will not. > > If the signal kills the subprocess, its children may remain behind as > > orphans. > > Am I misunderstanding the comment preceding the definition of > emacs_get_tty_pgrp? No, you understand it correctly. > Here's what it says: > > /* Return the foreground process group for the tty/pty that > the process P uses. */ > > That's not the same as the process group of the shell, at least in > Cygwin. Right, these are in general two different groups. > You seem to be assuming that the process group of the shell will > include all of the shell's children. No, I didn't assume that, sorry for possibly confusing wording. There's a misunderstanding here: you were talking about the case of interrupting a command run by a shell, whereas I was talking about the case of sending a signal to a process that is not the shell, or perhaps about signaling the shell itself. For the case you ware talking about, sending the signal to the shell is indeed the wrong thing, because the shell makes any foreground command it runs into a separate process group, and it's that process group that we want to send the signal to. But for that case, you already found the right solution: use SIGNALS_VIA_CHARACTERS. This does exactly the right thing, like the INTR character typed on the tty. What's left is the case of signals other that SIGINT/SIGQUIT/SIGTSTP. In that case, in the absence of TIOCGPGRP, the code in process.c supports sending the signal only to p->pid's process group (if p->pid is not a group leader, only it itself will be signaled). I think this is better than punting, because if there _is_ a process group of which p->pid is the leader, doing what the code does now will not leave orphans, processes other than p->pid that belonged to p->pid's group. It is true that this group might not be the foreground process group, but with signals other than SIGINT/SIGQUIT/SIGTSTP, this matters much less, I think. Does this make sense? If it does, then I think you should add SIGNALS_VIA_CHARACTERS, and leave the rest unaltered, because that's the best Emacs can do if TIOCGPGRP is not supported. > > It's not the wrong process. The problem is that its children might > > not get the signal. > > It is the wrong process on Cygwin. Here's an example. I start a shell > and then run 'cat'. In another shell I run 'ps'. The relevant part of > the output is: > > PID PPID PGID WINPID TTY UID STIME COMMAND > 508 920 508 3552 1 1009 18:50:15 /usr/bin/sh > I 916 508 916 1832 1 1009 18:50:18 /usr/bin/cat > > 508 is the PID as well as the PGID of the shell. But cat is in a > different process group, even though it's a child of the shell (as we > see from the PPID). On the other hand, it has the same controlling tty > as the shell, so we'd be in business if we could get the PGID of the > foreground process of the controlling tty of the shell. That's what I > thought emacs_get_tty_pgrp was doing (according to its comment). When I > type C-c C-c, emacs sends a signal to PGID 508, which is not the right > thing to do. For the case of a foreground command run by the shell, when we want to interrupt that command, yes. But that case is solved by using SIGNALS_VIA_CHARACTERS. ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin) 2010-10-18 6:59 ` Eli Zaretskii @ 2010-10-18 11:58 ` Ken Brown 2010-10-18 17:37 ` Ken Brown 0 siblings, 1 reply; 13+ messages in thread From: Ken Brown @ 2010-10-18 11:58 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 7225@debbugs.gnu.org On 10/18/2010 2:59 AM, Eli Zaretskii wrote: >> Date: Sun, 17 Oct 2010 19:11:15 -0400 >> From: Ken Brown<kbrown@cornell.edu> >> CC: "7225@debbugs.gnu.org"<7225@debbugs.gnu.org> >> >>> The problem that code is trying to solve is how to send a signal to >>> the whole process group starting at the shell (or whatever process is >>> the group leader). Failure to do so could mean that the immediate >>> subprocess of Emacs will get the signal, but its children will not. >>> If the signal kills the subprocess, its children may remain behind as >>> orphans. >> >> Am I misunderstanding the comment preceding the definition of >> emacs_get_tty_pgrp? > > No, you understand it correctly. > >> Here's what it says: >> >> /* Return the foreground process group for the tty/pty that >> the process P uses. */ >> >> That's not the same as the process group of the shell, at least in >> Cygwin. > > Right, these are in general two different groups. > >> You seem to be assuming that the process group of the shell will >> include all of the shell's children. > > No, I didn't assume that, sorry for possibly confusing wording. > > There's a misunderstanding here: you were talking about the case of > interrupting a command run by a shell, whereas I was talking about the > case of sending a signal to a process that is not the shell, or > perhaps about signaling the shell itself. > > For the case you ware talking about, sending the signal to the shell > is indeed the wrong thing, because the shell makes any foreground > command it runs into a separate process group, and it's that process > group that we want to send the signal to. But for that case, you > already found the right solution: use SIGNALS_VIA_CHARACTERS. This > does exactly the right thing, like the INTR character typed on the > tty. > > What's left is the case of signals other that SIGINT/SIGQUIT/SIGTSTP. > In that case, in the absence of TIOCGPGRP, the code in process.c > supports sending the signal only to p->pid's process group (if p->pid > is not a group leader, only it itself will be signaled). I think this > is better than punting, because if there _is_ a process group of which > p->pid is the leader, doing what the code does now will not leave > orphans, processes other than p->pid that belonged to p->pid's group. > It is true that this group might not be the foreground process group, > but with signals other than SIGINT/SIGQUIT/SIGTSTP, this matters much > less, I think. OK, I'm glad we understand each other now. But I still think line 6233 is wrong. That code gets executed only when current_group is non-nil. (As far as I know, the only use case for this is in shell mode, when we're trying to signal the foreground process; but I haven't done an exhaustive search to see if there are other cases.) According to the documentation preceding process_send_signal, we should not be sending a signal to p->pid in that case. So I think punting is better than what's done now. When current_group is nil, on the other hand, we do want to send a signal to p->pid and its process group, and this is correctly taken care of in line 6104. As a practical matter, I don't think the problem is serious, but I do think it's a bug. Anyway, I'll go ahead with my patch, since we both agree that it's the right fix for the original bug I reported. ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin) 2010-10-18 11:58 ` Ken Brown @ 2010-10-18 17:37 ` Ken Brown 0 siblings, 0 replies; 13+ messages in thread From: Ken Brown @ 2010-10-18 17:37 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 7225-done Patch applied, bug closed. BTW, in response to my request for help on the Cygwin list, one of the project leaders said he would implement TIOCGPGRP. So that will take care of everything we've discussed, as far as Cygwin is concerned. ^ permalink raw reply [flat|nested] 13+ messages in thread
* bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin) 2010-10-17 22:08 ` Eli Zaretskii 2010-10-17 23:11 ` Ken Brown @ 2010-10-18 5:49 ` Jan Djärv 1 sibling, 0 replies; 13+ messages in thread From: Jan Djärv @ 2010-10-18 5:49 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 7225 Eli Zaretskii skrev 2010-10-18 00.08: > So the question is: how to send a signal to the whole process group > with Cygwin? Does Cygwin support the "minus pgid" method of calling > `kill'? Or maybe there's some other method? > Cygwin has killpg AFAIK. Jan D. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2010-10-18 17:37 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-10-15 23:36 bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin) Ken Brown 2010-10-16 7:32 ` Eli Zaretskii 2010-10-16 12:44 ` Ken Brown 2010-10-16 13:26 ` Eli Zaretskii 2010-10-16 18:22 ` Ken Brown 2010-10-17 21:40 ` Ken Brown 2010-10-17 22:08 ` Eli Zaretskii 2010-10-17 23:11 ` Ken Brown 2010-10-17 23:39 ` Ken Brown 2010-10-18 6:59 ` Eli Zaretskii 2010-10-18 11:58 ` Ken Brown 2010-10-18 17:37 ` Ken Brown 2010-10-18 5:49 ` Jan Djärv
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).