public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* [PATCH] Ctrl-C and non-Cygwin programs
@ 2012-03-29  2:56 Ein Terakawa
  2012-03-29  3:24 ` Ryan Johnson
  2012-04-01 22:45 ` Christopher Faylor
  0 siblings, 2 replies; 5+ messages in thread
From: Ein Terakawa @ 2012-03-29  2:56 UTC (permalink / raw)
  To: cygwin-patches

This is a proof of concept demonstration which
makes Ctrl-C behave in a way a lot of people expect
concerning non-Cygwin console programs.

What it does actually is it generates CTRL_BREAK_EVENT with 
Windows Console API GenerateConsoleCtrlEvent on the arrival of SIGINT.
And to make this scheme to be functional it is required to specify
CREATE_NEW_PROCESS_GROUP when creating new non-Cygwin processes.

To my surprise there seem to be no way to generate CTRL_C_EVENT using API.

I must also point out that virtually all of the terminal emulators
are sneakily keeping hidden Windows Console in the background.
Thus several features of the Windows Console is still available to
processes running in the cygwin environment.
One of such features is this 'process group' and the other one is
'code page' which you manipulate with chcp.com utility.


Following is a bunch of random posts, I picked up from this list, 
talking about the same topic. Ordered by its significance under my judge.
These should help you understand (or remind) what it is about.


Date: Mon, 04 Dec 2006 06:24:41 -0800
Subject: Re: Ctrl-C and non-cygwin programs
http://cygwin.com/ml/cygwin/2006-12/msg00151.html

Date: Thu, 20 May 2010 22:50:49 +0000 (UTC)
Subject: A workaround for CTRL-C not working on Windows console apps in ptys
http://sourceware.org/ml/cygwin/2010-05/msg00524.html

Date: Mon, 19 Jan 2009 11:41:51 -0500
Subject: Signal handling in WIN32 console programs
http://sourceware.org/ml/cygwin/2009-01/msg00587.html

Date: Fri, 24 Aug 2001 17:25:14 -0400
Subject: control-c issue when running VC++ console programs in bash.exe
http://cygwin.com/ml/cygwin/2001-08/msg01111.html


As you can see this is a haunting problem and
the situation hasn't changed a bit over this past decade.
At least please let this issue be added to the FAQ.

I believe this patch is fairly small and worth giving a field test.


Lastly first third of the patch is a workaround of a problem observed
with cygwin1.dll of cvs HEAD.
To reproduce:
1. Launch a terminal emulator like rxvt or mintty.
2. Execute cmd.exe or more.com from shell prompt.
3. Type in Enter, Ctrl-C, then Enter again.
Whole processes including the terminal emulator will just hung up.

---
ChangeLog for winsup/cygwin:

2012-03-28  Ein Terakawa <applause@elfmimi.jp>

	* exceptions.cc: (sigpacket::process) Do not sigflush in response
	to SIGINT for a non-Cygwin process to work around hung-up.
	Translate SIGINT into CTRL_BREAK_EVENT for a non-Cygwin process.
	* spawn.cc: (child_info_spawn::worker) CREATE_NEW_PROCESS_GROUP for
	each new non-Cygwin process.
---
Index: winsup/cygwin/exceptions.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/exceptions.cc,v
retrieving revision 1.375
diff -c -p -r1.375 exceptions.cc
*** winsup/cygwin/exceptions.cc	12 Feb 2012 22:43:33 -0000	1.375
--- winsup/cygwin/exceptions.cc	28 Mar 2012 14:39:58 -0000
*************** sigpacket::process ()
*** 1166,1171 ****
--- 1166,1174 ----
    switch (si.si_signo)
      {
      case SIGINT:
+       if (have_execed)
+         break;
+       /* fall through */
      case SIGQUIT:
      case SIGSTOP:
      case SIGTSTP:
*************** sigpacket::process ()
*** 1252,1257 ****
--- 1255,1266 ----
        if (si.si_signo == SIGTSTP || si.si_signo == SIGTTIN || si.si_signo == SIGTTOU)
  	goto stop;
  
+       if (si.si_signo == SIGINT && have_execed)
+         {
+           GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, GetProcessId(ch_spawn));
+           goto done;
+         }
+ 
        goto exit_sig;
      }
  
Index: winsup/cygwin/spawn.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/spawn.cc,v
retrieving revision 1.329
diff -c -p -r1.329 spawn.cc
*** winsup/cygwin/spawn.cc	21 Mar 2012 15:54:50 -0000	1.329
--- winsup/cygwin/spawn.cc	28 Mar 2012 14:39:58 -0000
*************** child_info_spawn::worker (const char *pr
*** 573,579 ****
    cygbench ("spawn-worker");
  
    if (!real_path.iscygexec())
!     ::cygheap->fdtab.set_file_pointers_for_exec ();
  
    moreinfo->envp = build_env (envp, envblock, moreinfo->envc,
  			      real_path.iscygexec ());
--- 573,582 ----
    cygbench ("spawn-worker");
  
    if (!real_path.iscygexec())
!     {
!       ::cygheap->fdtab.set_file_pointers_for_exec ();
!       c_flags |= CREATE_NEW_PROCESS_GROUP;
!     }
  
    moreinfo->envp = build_env (envp, envblock, moreinfo->envc,
  			      real_path.iscygexec ());

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Ctrl-C and non-Cygwin programs
  2012-03-29  2:56 [PATCH] Ctrl-C and non-Cygwin programs Ein Terakawa
@ 2012-03-29  3:24 ` Ryan Johnson
  2012-04-01 22:45 ` Christopher Faylor
  1 sibling, 0 replies; 5+ messages in thread
From: Ryan Johnson @ 2012-03-29  3:24 UTC (permalink / raw)
  To: cygwin-patches

On 28/03/2012 10:55 PM, Ein Terakawa wrote:
> Lastly first third of the patch is a workaround of a problem observed
> with cygwin1.dll of cvs HEAD.
> To reproduce:
> 1. Launch a terminal emulator like rxvt or mintty.
> 2. Execute cmd.exe or more.com from shell prompt.
> 3. Type in Enter, Ctrl-C, then Enter again.
> Whole processes including the terminal emulator will just hung up.
Interesting... I may have just hit this earlier today trying to run a
Java program inside mintty. I assumed it was just Java being weird,
since killing it releases the terminal.

I'll try to build a cygwin1.dll with the patch, but my source tree seems
to be in a bad state so it could be a while.

Ryan

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Ctrl-C and non-Cygwin programs
  2012-03-29  2:56 [PATCH] Ctrl-C and non-Cygwin programs Ein Terakawa
  2012-03-29  3:24 ` Ryan Johnson
@ 2012-04-01 22:45 ` Christopher Faylor
  1 sibling, 0 replies; 5+ messages in thread
From: Christopher Faylor @ 2012-04-01 22:45 UTC (permalink / raw)
  To: cygwin-patches

On Thu, Mar 29, 2012 at 11:55:51AM +0900, Ein Terakawa wrote:
>This is a proof of concept demonstration which
>makes Ctrl-C behave in a way a lot of people expect
>concerning non-Cygwin console programs.
>
>What it does actually is it generates CTRL_BREAK_EVENT with 
>Windows Console API GenerateConsoleCtrlEvent on the arrival of SIGINT.
>And to make this scheme to be functional it is required to specify
>CREATE_NEW_PROCESS_GROUP when creating new non-Cygwin processes.
>
>To my surprise there seem to be no way to generate CTRL_C_EVENT using API.
>
>I must also point out that virtually all of the terminal emulators
>are sneakily keeping hidden Windows Console in the background.

Yes, Cygwin does this by design.  It helps some programs work better if
they detect that a console is available.

>Lastly first third of the patch is a workaround of a problem observed
>with cygwin1.dll of cvs HEAD.
>To reproduce:
>1. Launch a terminal emulator like rxvt or mintty.
>2. Execute cmd.exe or more.com from shell prompt.
>3. Type in Enter, Ctrl-C, then Enter again.
>Whole processes including the terminal emulator will just hung up.

I added a fix to Cygwin a couple of days ago which was supposed to fix
this.  It just avoided trying to process a SIGINT if the user typed
CTRL-C when a non-Cygwin program had been execed.

I could duplicate this problem before the patch and couldn't after the
patch.  However, Corinna reports that she can still duplicate the hang.
Is that true for others also?

In any event, making a determination for every SIGINT seems like the
wrong way to handle this.  Catching it only for the case where a console
user has typed CTRL-C is how I'd like to handle it even if that means
more tweaking for the code I just checked in.

>	* spawn.cc: (child_info_spawn::worker) CREATE_NEW_PROCESS_GROUP for
>	each new non-Cygwin process.

This change is puzzling.  If you look at the existing spawn code, it is
already adding CREATE_NEW_PROCESS_GROUP to the CreateProcess flags iff
we aren't about to create a cygwin process + a few other tests.  See the
code under the comment that starts with /* If a native application
should be spawned, ... */

If the current test isn't adequate then the only thing I can see is that
the additional test fhandler_console::tc_getpgid () != myself->pgid is
causing a new proces group not to be created when it should be.

cgf

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Ctrl-C and non-Cygwin programs
  2012-05-25 18:43 Mark Lofdahl
@ 2012-05-26 20:53 ` Christopher Faylor
  0 siblings, 0 replies; 5+ messages in thread
From: Christopher Faylor @ 2012-05-26 20:53 UTC (permalink / raw)
  To: cygwin-patches

On Fri, May 25, 2012 at 11:43:11AM -0700, Mark Lofdahl wrote:
>References: <4F73CF37.4020001@elfmimi.jp>
>
>On 28/03/2012 10:55 PM, Ein Terakawa wrote:
>
>>What it does actually is it generates CTRL_BREAK_EVENT with 
>>Windows Console API GenerateConsoleCtrlEvent on the arrival of SIGINT.
>>And to make this scheme to be functional it is required to specify
>>CREATE_NEW_PROCESS_GROUP when creating new non-Cygwin processes.
>
>
>Is there any way for me to get the old behavior? I rely heavily on the
>ability to press ctrl-c in my non-cygwin console app and have that app
>receive a CTRL_C_EVENT instead of a CTRL_BREAK_EVENT. Everything worked fine
>for me before this patch.
>
>>To my surprise there seem to be no way to generate CTRL_C_EVENT using API.
>
>It is possible to generate a CTRL_C_EVENT, if you pass 0 as the process
>group id, in which case the event is passed to all process that share the
>console. Don't know if that would work in this situation.
>http://msdn.microsoft.com/en-us/library/windows/desktop/ms683155.aspx

You're in the wrong mailing list.  You don't ask for stuff here, you provide
patches.  Please use the main Cygwin list.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] Ctrl-C and non-Cygwin programs
@ 2012-05-25 18:43 Mark Lofdahl
  2012-05-26 20:53 ` Christopher Faylor
  0 siblings, 1 reply; 5+ messages in thread
From: Mark Lofdahl @ 2012-05-25 18:43 UTC (permalink / raw)
  To: cygwin-patches

References: <4F73CF37.4020001@elfmimi.jp>

On 28/03/2012 10:55 PM, Ein Terakawa wrote:

>What it does actually is it generates CTRL_BREAK_EVENT with 
>Windows Console API GenerateConsoleCtrlEvent on the arrival of SIGINT.
>And to make this scheme to be functional it is required to specify
>CREATE_NEW_PROCESS_GROUP when creating new non-Cygwin processes.


Is there any way for me to get the old behavior? I rely heavily on the
ability to press ctrl-c in my non-cygwin console app and have that app
receive a CTRL_C_EVENT instead of a CTRL_BREAK_EVENT. Everything worked fine
for me before this patch.

>To my surprise there seem to be no way to generate CTRL_C_EVENT using API.

It is possible to generate a CTRL_C_EVENT, if you pass 0 as the process
group id, in which case the event is passed to all process that share the
console. Don't know if that would work in this situation.
http://msdn.microsoft.com/en-us/library/windows/desktop/ms683155.aspx


Thanks,
Mark Lofdahl

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-05-26 20:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-29  2:56 [PATCH] Ctrl-C and non-Cygwin programs Ein Terakawa
2012-03-29  3:24 ` Ryan Johnson
2012-04-01 22:45 ` Christopher Faylor
2012-05-25 18:43 Mark Lofdahl
2012-05-26 20:53 ` Christopher Faylor

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