Re: [PATCH v2 3/3] Cygwin: pty: Fixup pty state after a cygwin app exits

Takashi Yano <[email protected]> Wed, 24 Jun 2026 22:14:02 +0900
Newsgroups gmane.os.cygwin.patches
Message-ID <[email protected]>
The patch seriese pushed.
Thanks!

On Wed, 24 Jun 2026 00:17:57 -0700
Mark Geisert wrote:
> Hi Takashi,
> 
> All of your corrections and additions LGTM.  This one can be pushed.
> Thanks,
> 
> ..mark
> 
> On 6/23/2026 6:31 AM, Takashi Yano wrote:
> > Hi Mark,
> > 
> > Thanks for reviewing.
> > 
> > On Tue, 23 Jun 2026 01:07:16 -0700
> > Mark Geisert  wrote:
> >> Hi Takashi,
> >>
> >> On 6/13/2026 7:09 AM, Takashi Yano wrote:
> >>> Previously, the cygwin process on pty is always a child of another
> >>> cygwin app on pty. If a cygwin app is a child of non-cygwin app
> >>> in pseudo console, it was running on console originating from
> >>> pseudo console. Now, the child of a non-cygwin app on pseudo console
> >>> is running on pty, so, it is necessary to restore the pty state
> >>> to the state where the parent process is running. This patch
> >>> does the following fixup:
> >>>    1) Switch pipe mode to cyg-pipe to nat-pipe.
> >>                           ^^^^^^^^^^^^^^^^^^^^^^^ this can't be correct
> > 
> > Auch! "from cyg-pipe to nat-pipe"
> >>
> >>>    2) Notify the current cursor position to pseudo console
> >>>
> >>> These prevent the problems:
> >>>    1) Run 'cat' in cmd.exe and stop it by Ctrl-C. After that
> >>>       cmd.exe cannot receive key input.
> >>>    2) Run 'ps' in cmd.exe. The cursor position will not be
> >>>       maintained correctly after that.
> >>>
> >>> Signed-off-by: Takashi Yano <[email protected]>
> >>> Reviewed-by:
> >>> ---
> >>>    winsup/cygwin/fhandler/pty.cc           | 73 ++++++++++++++++++++++++-
> >>>    winsup/cygwin/local_includes/fhandler.h |  2 +
> >>>    winsup/cygwin/local_includes/tty.h      |  1 +
> >>>    3 files changed, 73 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc
> >>> index b3a8d57cc..f4473bb69 100644
> >>> --- a/winsup/cygwin/fhandler/pty.cc
> >>> +++ b/winsup/cygwin/fhandler/pty.cc
> >>> @@ -388,6 +388,52 @@ atexit_func (void)
> >>>        }
> >>>    }
> >>>    
> >>> +void
> >>> +fhandler_pty_slave::req_fixup_pcon_state (void)
> >>> +{
> >>> +  while (true)
> >>> +    {
> >>> +      WaitForSingleObject (input_mutex, mutex_timeout);
> >>> +      if (!get_ttyp ()->pcon_start_pid)
> >>> +	break;
> >>> +      /* Another request is on going. */
> >>> +      ReleaseMutex (input_mutex);
> >>> +      yield ();
> >>> +    }
> >>> +
> >>> +  DWORD n;
> >>> +  /* indicates that this "ESC[6n" is just for fixing-up corsor position */
> >>                                                              ^^^^^^ typo here
> > 
> > Fixed.
> > 
> >>> +  get_ttyp ()->req_fixup_pcon_cur_pos = true;
> >>> +  get_ttyp ()->req_xfer_input = true; /* indicates that this "ESC[6n"
> >>> +					 is just for transfer input */
> >>> +  get_ttyp ()->pcon_start = true;
> >>> +  get_ttyp ()->pcon_start_pid = myself->pid;
> >>> +  WriteFile (get_output_handle (), "\033[6n", 4, &n, NULL);
> >>> +  ReleaseMutex (input_mutex);
> >>> +  while (get_ttyp ()->pcon_start_pid)
> >>> +    /* wait for completion of fixing-up in master::write(). */
> >>> +    yield ();
> >>> +}
> >>> +
> >>> +void
> >>> +fhandler_pty_master::fixup_pcon_cursor_position (int x, int y)
> >>> +{
> >>> +  HANDLE pcon_owner = OpenProcess (PROCESS_DUP_HANDLE, FALSE,
> >>> +				   get_ttyp ()->nat_pipe_owner_pid);
> >>> +  HANDLE h_pcon_out = NULL;
> >>> +  DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_out,
> >>> +		   GetCurrentProcess (), &h_pcon_out,
> >>> +		   0, TRUE, DUPLICATE_SAME_ACCESS);
> >>> +  CloseHandle (pcon_owner);
> >>> +  DWORD target_pid = get_ttyp ()->nat_pipe_owner_pid;
> >>> +  DWORD resume_pid =
> >>> +    fhandler_pty_common::attach_console_temporarily (target_pid);
> >>> +  COORD cur_pos = {(SHORT) (x - 1), (SHORT) (y - 1)};
> >>> +  SetConsoleCursorPosition (h_pcon_out, cur_pos);
> >>> +  fhandler_pty_common::resume_from_temporarily_attach (resume_pid);
> >>> +  CloseHandle (h_pcon_out);
> >>> +}
> >>> +
> >>>    #define DEF_HOOK(name) static __typeof__ (name) *name##_Orig
> >>>    /* CreateProcess() is hooked for GDB etc. */
> >>>    DEF_HOOK (CreateProcessA);
> >>> @@ -1162,6 +1208,19 @@ err_no_msg:
> >>>    bool
> >>>    fhandler_pty_slave::open_setup (int flags)
> >>>    {
> >>> +  if (get_ttyp ()->pcon_activated)
> >>> +    {
> >>> +      HANDLE pcon_owner = OpenProcess (PROCESS_DUP_HANDLE, FALSE,
> >>> +				       get_ttyp ()->nat_pipe_owner_pid);
> >>> +      DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_in,
> >>> +		       GetCurrentProcess (), &get_handle_nat (),
> >>> +		       0, TRUE, DUPLICATE_SAME_ACCESS);
> >>> +      DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_out,
> >>> +		       GetCurrentProcess (), &get_output_handle_nat (),
> >>> +		       0, TRUE, DUPLICATE_SAME_ACCESS);
> >>> +      CloseHandle (pcon_owner);
> >>> +    }
> >>> +
> >>>      set_flags ((flags & ~O_TEXT) | O_BINARY);
> >>>      myself->set_ctty (this, flags);
> >>>      report_tty_counts (this, "opened", "");
> >>> @@ -1171,6 +1230,9 @@ fhandler_pty_slave::open_setup (int flags)
> >>>    void
> >>>    fhandler_pty_slave::cleanup ()
> >>>    {
> >>> +  if (get_ttyp ()->pcon_activated && get_ttyp ()->getpgid () == myself->pgid)
> >>> +    req_fixup_pcon_state ();
> >>> +
> >>>      /* This used to always call fhandler_pty_common::close when we were execing
> >>>         but that caused multiple closes of the handles associated with this pty.
> >>>         Since close_all_files is not called until after the cygwin process has
> >>> @@ -2478,7 +2540,14 @@ fhandler_pty_master::write (const void *ptr, size_t len)
> >>>    	      /* req_xfer_input is true if "ESC[6n" was sent just for
> >>>    		 triggering transfer_input() in master. In this case,
> >>>    		 the response sequence should not be written. */
> >>
> >> The above comment describes the req_xfer case, but says nothing about
> >> the the req_fixup_pcon_cur_pos case now inserted before it.
> > 
> > I'll modify the comment above like:
> >        /* req_fixup_pcon_cur_pos is true if "ESC[6n" was sent
> >           for requesting cursor-position-fixup that is needed
> >           when a non-cygwin app executes a cygwin app and the
> >           cygwin app exits.
> >           req_xfer_input is true if "ESC[6n" was sent just for
> >           triggering transfer_input() in master. In this case,
> >           the response sequence should not be written. */
> > 
> > Does this look OK to you?
> 
> That's great!
> 
> ..mark


-- 
Takashi Yano <[email protected]>