bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Ken Brown
Subject: bug#7225: 23.2.50; [PATCH] C-c C-c doesn't work in shell mode (Cygwin)
Date: Mon, 18 Oct 2010 07:58:35 -0400
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.9) Gecko/20100915 Thunderbird/3.1.4

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.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]