bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin)

bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin)

[Ken Brown]

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

bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin)

[Eli Zaretskii]

> 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.

bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin)

[Ken Brown]

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.

bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin)

[Eli Zaretskii]

> 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.

bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin)

[Ken Brown]

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

bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin)

[Ken Brown]

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.

bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin)

[Eli Zaretskii]

> 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?

bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin)

[Ken Brown]

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.

bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin)

[Ken Brown]

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.

bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin)

[Jan Djärv]

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.

bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin)

[Eli Zaretskii]

> 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.

bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin)

[Ken Brown]

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.

bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin)

[Ken Brown]

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.