[PATCH] Cygwin: console: Fix issues of apps which open pty.

2022-02-26 Thread Takashi Yano
- After some recent changes for special keys handling break the
  apps which open pty (such as script command). If the app which
  opens pty is executed in console, the following issues occur.
1) If the script command was started from non-cygwin shell
   (such as cmd.exe), another cygwin app started in pty slave
   cannot receive Ctrl-C.
2) If non-cygwin app is executed in pty slave, the app which
   opened the pty (e.g. script command) crashes by Ctrl-C.
  This patch fixes these issues.
---
 winsup/cygwin/fhandler.h  |  2 +-
 winsup/cygwin/fhandler_console.cc | 20 ++--
 winsup/cygwin/fhandler_termios.cc | 14 +-
 winsup/cygwin/fhandler_tty.cc |  2 +-
 4 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 7ddf7e450..0652075b0 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1921,7 +1921,7 @@ class fhandler_termios: public fhandler_base
   HANDLE& get_output_handle_nat () { return output_handle; }
   static process_sig_state process_sigs (char c, tty *ttyp,
 fhandler_termios *fh);
-  static bool process_stop_start (char c, tty *ttyp, bool on_ixany);
+  static bool process_stop_start (char c, tty *ttyp);
   line_edit_status line_edit (const char *rptr, size_t nread, termios&,
  ssize_t *bytes_read = NULL);
   void set_output_handle (HANDLE h) { output_handle = h; }
diff --git a/winsup/cygwin/fhandler_console.cc 
b/winsup/cygwin/fhandler_console.cc
index 843a96f9a..aa0f26450 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -189,7 +189,7 @@ void
 fhandler_console::cons_master_thread (handle_set_t *p, tty *ttyp)
 {
   termios &ti = ttyp->ti;
-  DWORD output_stopped_at = 0;
+  int processed_up_to = -1;
   while (con.owner == myself->pid)
 {
   DWORD total_read, n, i;
@@ -210,6 +210,7 @@ fhandler_console::cons_master_thread (handle_set_t *p, tty 
*ttyp)
 input_rec, INREC_SIZE, &total_read);
  break;
case WAIT_TIMEOUT:
+ processed_up_to = -1;
case WAIT_SIGNALED:
case WAIT_CANCELED:
  break;
@@ -232,11 +233,10 @@ fhandler_console::cons_master_thread (handle_set_t *p, 
tty *ttyp)
  ttyp->kill_pgrp (SIGWINCH);
}
}
-  for (i = 0; i < total_read; i++)
+  for (i = processed_up_to + 1; i < total_read; i++)
{
  wchar_t wc;
  char c;
- bool was_output_stopped;
  bool processed = false;
  switch (input_rec[i].EventType)
{
@@ -256,14 +256,12 @@ fhandler_console::cons_master_thread (handle_set_t *p, 
tty *ttyp)
  ttyp->output_stopped = false;
  if (ti.c_lflag & NOFLSH)
goto remove_record;
+ processed_up_to = -1;
  goto skip_writeback;
default: /* not signalled */
  break;
}
- was_output_stopped = ttyp->output_stopped;
- processed = process_stop_start (c, ttyp, i > output_stopped_at);
- if (!was_output_stopped && ttyp->output_stopped)
-   output_stopped_at = i;
+ processed = process_stop_start (c, ttyp);
  break;
case WINDOW_BUFFER_SIZE_EVENT:
  SHORT y = con.dwWinSize.Y;
@@ -284,14 +282,16 @@ fhandler_console::cons_master_thread (handle_set_t *p, 
tty *ttyp)
 remove_record:
  if (processed)
{ /* Remove corresponding record. */
- memmove (input_rec + i, input_rec + i + 1,
-  sizeof (INPUT_RECORD) * (total_read - i - 1));
+ if (total_read > i + 1)
+   memmove (input_rec + i, input_rec + i + 1,
+sizeof (INPUT_RECORD) * (total_read - i - 1));
  total_read--;
  i--;
}
}
+  processed_up_to = total_read - 1;
   if (total_read)
-   /* Write back input records other than interrupt. */
+   /* Writeback input records other than interrupt. */
WriteConsoleInputW (p->input_handle, input_rec, total_read, &n);
 skip_writeback:
   ReleaseMutex (p->input_mutex);
diff --git a/winsup/cygwin/fhandler_termios.cc 
b/winsup/cygwin/fhandler_termios.cc
index 383e20764..b236c1b62 100644
--- a/winsup/cygwin/fhandler_termios.cc
+++ b/winsup/cygwin/fhandler_termios.cc
@@ -344,10 +344,12 @@ fhandler_termios::process_sigs (char c, tty* ttyp, 
fhandler_termios *fh)
  (myself->dwProcessId, false);
  if (resume_pid && fh && !fh->is_console ())
{
+ if (::cygheap->ctty && ::cygheap->ctty->is_console ())
+   init_console_handler (false);
  FreeConsole ();
  AttachConsole (p->dwProcessId);
}
- if (fh && p == myself)
+ if (fh && p

[PATCH] Cygwin: pty: Stop to send CTRL_C_EVENT if pcon activated.

2022-02-26 Thread Takashi Yano
- The commit "Cygwin: console: Redesign handling of special keys."
  removes special treatment for pty in with pseudo console activated,
  however, it is necessary on second thought. This is because sending
  CTRL_C_EVENT to non-cygwin apps will be done in pseudo console,
  therefore, sending it in fhandler_pty_master::write() duplicates
  that event for non-cygwin apps.
---
 winsup/cygwin/fhandler.h  |  2 ++
 winsup/cygwin/fhandler_termios.cc | 11 ---
 winsup/cygwin/fhandler_tty.cc | 11 +++
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 0652075b0..fda5a4ec9 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1958,6 +1958,7 @@ class fhandler_termios: public fhandler_base
   virtual void cleanup_before_exit () {}
   virtual void setpgid_aux (pid_t pid) {}
   virtual bool need_console_handler () { return false; }
+  virtual bool need_send_ctrl_c_event () { return true; }
 };
 
 enum ansi_intensity
@@ -2492,6 +2493,7 @@ public:
   void get_master_thread_param (master_thread_param_t *p);
   void get_master_fwd_thread_param (master_fwd_thread_param_t *p);
   void set_mask_flusho (bool m) { get_ttyp ()->mask_flusho = m; }
+  bool need_send_ctrl_c_event ();
 };
 
 class fhandler_dev_null: public fhandler_base
diff --git a/winsup/cygwin/fhandler_termios.cc 
b/winsup/cygwin/fhandler_termios.cc
index b236c1b62..568523390 100644
--- a/winsup/cygwin/fhandler_termios.cc
+++ b/winsup/cygwin/fhandler_termios.cc
@@ -359,16 +359,12 @@ fhandler_termios::process_sigs (char c, tty* ttyp, 
fhandler_termios *fh)
 instead. */
  if ((p->process_state & PID_NEW_PG)
  && (p->process_state & PID_NOTCYGWIN))
-   {
- GenerateConsoleCtrlEvent (CTRL_BREAK_EVENT,
-   p->dwProcessId);
- need_discard_input = true;
-   }
- else if (!ctrl_c_event_sent)
+   GenerateConsoleCtrlEvent (CTRL_BREAK_EVENT, p->dwProcessId);
+ else if ((!fh || fh->need_send_ctrl_c_event () || cyg_leader)
+ && !ctrl_c_event_sent)
{
  GenerateConsoleCtrlEvent (CTRL_C_EVENT, 0);
  ctrl_c_event_sent = true;
- need_discard_input = true;
}
  if (resume_pid && fh && !fh->is_console ())
{
@@ -377,6 +373,7 @@ fhandler_termios::process_sigs (char c, tty* ttyp, 
fhandler_termios *fh)
  if (::cygheap->ctty && ::cygheap->ctty->is_console ())
init_console_handler (true);
}
+ need_discard_input = true;
}
   if (p && p->ctty == ttyp->ntty && p->pgid == pgid)
{
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 2440318b8..9855f54eb 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -4105,3 +4105,14 @@ fhandler_pty_slave::setpgid_aux (pid_t pid)
 }
   ReleaseMutex (pcon_mutex);
 }
+
+bool
+fhandler_pty_master::need_send_ctrl_c_event ()
+{
+  /* If pseudo console is activated, sending CTRL_C_EVENT to non-cygwin
+ apps will be done in pseudo console, therefore, sending it in
+ fhandler_pty_master::write() duplicates that event for non-cygwin
+ apps. So return false if pseudo console is activated. */
+  return !(to_be_read_from_pcon () && get_ttyp ()->pcon_activated
+&& get_ttyp ()->pcon_input_state == tty::to_nat);
+}
-- 
2.35.1



[PATCH] Cygwin: console: Prevent the order of typeahead input from swapped.

2022-02-26 Thread Takashi Yano
- If a lot of keys are typed very quickly in the app which does
  not read console, the order of input keys in console input buffer
  occasionally swapped. Although this extremely rarely happens,
  is obviously a bug of cons_master_thread. This patch fixes the
  issue.
---
 winsup/cygwin/fhandler_console.cc | 53 +--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/fhandler_console.cc 
b/winsup/cygwin/fhandler_console.cc
index aa0f26450..88c0f894b 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -208,6 +208,20 @@ fhandler_console::cons_master_thread (handle_set_t *p, tty 
*ttyp)
case WAIT_OBJECT_0:
  ReadConsoleInputW (p->input_handle,
 input_rec, INREC_SIZE, &total_read);
+ if (total_read == INREC_SIZE /* Working space full */
+ && cygwait (p->input_handle, (DWORD) 0) == WAIT_OBJECT_0)
+   {
+ const int incr = 1;
+ size_t bytes = sizeof (INPUT_RECORD) * (total_read - incr);
+ /* Discard oldest incr events. */
+ memmove (input_rec, input_rec + incr, bytes);
+ total_read -= incr;
+ processed_up_to =
+   (processed_up_to + 1 >= incr) ? processed_up_to - incr : -1;
+ ReadConsoleInputW (p->input_handle,
+input_rec + total_read, incr, &n);
+ total_read += n;
+   }
  break;
case WAIT_TIMEOUT:
  processed_up_to = -1;
@@ -291,8 +305,43 @@ remove_record:
}
   processed_up_to = total_read - 1;
   if (total_read)
-   /* Writeback input records other than interrupt. */
-   WriteConsoleInputW (p->input_handle, input_rec, total_read, &n);
+   {
+ /* Writeback input records other than interrupt. */
+ WriteConsoleInputW (p->input_handle, input_rec, total_read, &n);
+ size_t bytes = sizeof (INPUT_RECORD) * total_read;
+ do
+   {
+ const int additional_size = 128; /* Possible max number of
+ incoming events during
+ above process. */
+ const int new_size = INREC_SIZE + additional_size;
+ INPUT_RECORD tmp[new_size];
+ /* Check if writeback was successfull. */
+ PeekConsoleInputW (p->input_handle, tmp, new_size, &n);
+ if (memcmp (input_rec, tmp, bytes) == 0)
+   break; /* OK */
+ /* Try to fix */
+ DWORD incr = n - total_read;
+ DWORD ofst;
+ for (ofst = 1; ofst <= incr; ofst++)
+   if (memcmp (input_rec, tmp + ofst, bytes) == 0)
+ {
+   ReadConsoleInputW (p->input_handle, tmp, new_size, &n);
+   DWORD m;
+   WriteConsoleInputW (p->input_handle, tmp + ofst,
+   total_read, &m);
+   WriteConsoleInputW (p->input_handle, tmp, ofst, &m);
+   if ( n > ofst + total_read)
+ WriteConsoleInputW (p->input_handle,
+ tmp + ofst + total_read,
+ n - (ofst + total_read), &m);
+   break;
+ }
+ if (ofst > incr) /* Hard to fix */
+   break; /* Giving up */
+   }
+ while (true);
+   }
 skip_writeback:
   ReleaseMutex (p->input_mutex);
   cygwait (40);
-- 
2.35.1



Re: [PATCH] Cygwin: pty, console: Refactor the code processing special keys.

2022-02-26 Thread Takashi Yano
Hi Johannes,

Thank you for the comment.

On Fri, 25 Feb 2022 14:37:59 +0100 (CET)
Johannes Schindelin wrote:
> Hi Takashi,
> 
> On Sun, 20 Feb 2022, Takashi Yano wrote:
> 
> > - This patch commonize the code which processes special keys in pty
> >   and console to improve maintanancibility.
> 
> I very much welcome the direction. Thank you for working on this!
> 
> > As a result, some small bugs have been fixed.
> 
> Whenever I read something like this in commit messages, I wish for more
> details. Could you describe those bugs, and how exactly they were fixed?

The bugs I noticed were as follows.

1. In console, even with NOFLSH flag, type ahead input was flushed
   by Ctrl-C. This has been fixed by adding following code to
   fhandler_console::cons_master_thread().

> + case not_signalled_but_done:
> processed = true;
> +   ttyp->output_stopped = false;
> +   if (ti.c_lflag & NOFLSH)
> + goto remove_record;
> +   goto skip_writeback;

2. If output of non-cygwin app and input of cygwin app are connected
   by a pipe, we needed press Ctrl-C twice to terminate them when the
   cygwin app does not read stdin at the moment. This issue has been
   fixed by the code added to exceptions.cc you asked next.

Other bugs might exist, but did not check old behaviour in detail.
The priority was to make sure that the new code works without problems.
I have checked the behaviour of the current code using more than 200
test cases.

> > diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
> > index a914110fe..356d69d6a 100644
> > --- a/winsup/cygwin/exceptions.cc
> > +++ b/winsup/cygwin/exceptions.cc
> > @@ -1147,6 +1147,19 @@ ctrl_c_handler (DWORD type)
> >  return TRUE;
> >
> >tty_min *t = cygwin_shared->tty.get_cttyp ();
> > +
> > +  /* If process group leader is non-cygwin process or not exist,
> > + send signal to myself. */
> > +  pinfo pi (t->getpgid ());
> > +  if ((!pi || (pi->process_state & PID_NOTCYGWIN))
> > +  && (!have_execed || have_execed_cygwin)
> > +  && t->getpgid () == myself->pgid
> > +  && type == CTRL_C_EVENT)
> > +{
> > +  t->output_stopped = false;
> > +  sig_send(myself, SIGINT);
> > +}
> > +
> 
> From the commit message, I would have expected this patch to be
> essentially a clean and obvious refactoring of the existing code.
> 
> However, I cannot find any removed code in the diff that would explain
> this newly-added code.

Sorry, but this commit was not pure refactoring as a result. Some
small issues, such as the issues above, are fixed at the same time.

On the other hand, other bugs might be introduced accidentally.
As expected, some additional patches have been needed.
https://cygwin.com/pipermail/cygwin-patches/2022q1/011789.html
https://cygwin.com/pipermail/cygwin-patches/2022q1/011790.html

> When I see something like this in patches, I usually take to the commit
> message to explain what is going on. But in this case, I am left even more
> puzzled than before.
> 
> > diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
> > index 4e86ab58a..3e0d7d5a6 100644
> > --- a/winsup/cygwin/fhandler.h
> > +++ b/winsup/cygwin/fhandler.h
> > @@ -1901,6 +1902,13 @@ class fhandler_termios: public fhandler_base
> >virtual void release_input_mutex_if_necessary (void) {};
> >virtual void discard_input () {};
> >
> > +  enum process_sig_state {
> > +signalled,
> > +not_signalled,
> > +not_signalled_but_done,
> > +not_signalled_with_cyg_reader
> > +  };
> > +
> 
> My hope for this refactor is that it will get much easier to understand
> for developers who are unfamiliar with the pseudo console code.
> 
> In this instance, I am wishing for a code comment above the `enum` that
> explains its role, and the meaning of the four values. In particular the
> difference of the last two compared to the second is quite lost on me.
> > @@ -1943,6 +1954,8 @@ class fhandler_termios: public fhandler_base
> >}
> >static bool path_iscygexec_a (LPCSTR n, LPSTR c);
> >static bool path_iscygexec_w (LPCWSTR n, LPWSTR c);
> > +  virtual bool is_pty_master_with_pcon () { return false; }
> 
> It would probably make sense to document the role of a pty master with
> respect to pseudo consoles somewhere. If no better location can be found,
> then it would probably make most sense to add a comment above this new
> line.

I will add some comments to the source files.

> > +  virtual void cleanup_before_exit () {}
> >  };
> >
> >  enum ansi_intensity
> > diff --git a/winsup/cygwin/fhandler_console.cc 
> > b/winsup/cygwin/fhandler_console.cc
> > index 50f350c49..475c1acdb 100644
> > --- a/winsup/cygwin/fhandler_console.cc
> > +++ b/winsup/cygwin/fhandler_console.cc
> > @@ -188,13 +188,28 @@ cons_master_thread (VOID *arg)
> >  void
> >  fhandler_console::cons_master_thread (handle_set_t *p, tty *ttyp)
> >  {
> > +  termios &ti = ttyp->ti;
> >DWORD 

[PATCH] Cygwin: console: Revert experimental code mixed accidentally.

2022-02-26 Thread Takashi Yano
- The commit "Cygwin: console: Restore CTRL_BREAK_EVENT handling."
  was accidentally mixed with experimental code in exceptions.cc.
  Due to this, non-cygwin app receives CTRL_C_EVENT twice in the
  following scenario.
   1) Run 'sleep 10 | '
   2) Hit Ctrl-C.
   3) The non-cygwin app receives CTRL_C_EVENT twice.
  This patch reverts the code with the problem.
---
 winsup/cygwin/exceptions.cc | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 73bf68939..6e0b862c7 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1174,8 +1174,14 @@ ctrl_c_handler (DWORD type)
(to indicate that we have handled the signal).  At this point, type
should be a CTRL_C_EVENT or CTRL_BREAK_EVENT. */
 {
+  int sig = SIGINT;
+  /* If intr and quit are both mapped to ^C, send SIGQUIT on ^BREAK */
+  if (type == CTRL_BREAK_EVENT
+ && t->ti.c_cc[VINTR] == 3 && t->ti.c_cc[VQUIT] == 3)
+   sig = SIGQUIT;
   t->last_ctrl_c = GetTickCount64 ();
-  fhandler_termios::process_sigs ('\003', (tty *) t, ::cygheap->ctty);
+  t->kill_pgrp (sig);
+  t->output_stopped = false;
   t->last_ctrl_c = GetTickCount64 ();
   return TRUE;
 }
-- 
2.35.1



[PATCH 0/2] Drop pointless loadlib.h use in utilities

2022-02-26 Thread Jon Turney
The only remaining uses of loadlib.h are in cygcheck and strace, where it's
used to load cygwin1.dll in both.

Things could be further simplified, but it's probably worth keeping it
around in it's present form since it's quite likely that
LoadLibrary()/GetProcAddress() might be used again in future.

Jon Turney (2):
  Cygwin: Drop pointless loadlib.h includes in utilities
  Cygwin: Drop use of loadlib.h in regtool

 winsup/utils/cygpath.cc |  1 -
 winsup/utils/module_info.cc |  1 -
 winsup/utils/path.cc|  1 -
 winsup/utils/ps.cc  |  1 -
 winsup/utils/regtool.cc | 13 +
 5 files changed, 1 insertion(+), 16 deletions(-)

-- 
2.35.1



[PATCH 1/2] Cygwin: Drop pointless loadlib.h includes in utilities

2022-02-26 Thread Jon Turney
These utilities used to LoadLibrary()/GetProcAddress(), but don't
anymore.
---
 winsup/utils/cygpath.cc | 1 -
 winsup/utils/module_info.cc | 1 -
 winsup/utils/path.cc| 1 -
 winsup/utils/ps.cc  | 1 -
 4 files changed, 4 deletions(-)

diff --git a/winsup/utils/cygpath.cc b/winsup/utils/cygpath.cc
index 701c34998..9873e7b16 100644
--- a/winsup/utils/cygpath.cc
+++ b/winsup/utils/cygpath.cc
@@ -29,7 +29,6 @@ details. */
 #include 
 
 #include "wide_path.h"
-#include "loadlib.h"
 
 static char *prog_name;
 static char *file_arg, *output_arg;
diff --git a/winsup/utils/module_info.cc b/winsup/utils/module_info.cc
index e0bd4b71a..3e2fc28e2 100644
--- a/winsup/utils/module_info.cc
+++ b/winsup/utils/module_info.cc
@@ -12,7 +12,6 @@ details. */
 #include 
 #define PSAPI_VERSION 1
 #include 
-#include "loadlib.h"
 
 /* Returns full name of Dll, which is loaded by hProcess at BaseAddress.
Uses psapi.dll. */
diff --git a/winsup/utils/path.cc b/winsup/utils/path.cc
index df0037c15..fe55a646d 100644
--- a/winsup/utils/path.cc
+++ b/winsup/utils/path.cc
@@ -28,7 +28,6 @@ details. */
 #ifdef FSTAB_ONLY
 #include 
 #endif
-#include "loadlib.h"
 
 #ifndef FSTAB_ONLY
 /* Used when treating / and \ as equivalent. */
diff --git a/winsup/utils/ps.cc b/winsup/utils/ps.cc
index b51657535..dbcacbab4 100644
--- a/winsup/utils/ps.cc
+++ b/winsup/utils/ps.cc
@@ -21,7 +21,6 @@ details. */
 #include 
 #include 
 #include 
-#include "loadlib.h"
 
 /* Maximum possible path length under NT.  There's no official define
for that value.  Note that PATH_MAX is only 4K. */
-- 
2.35.1



[PATCH 2/2] Cygwin: Drop use of loadlib.h in regtool

2022-02-26 Thread Jon Turney
Link directly with RegDeleteKeyExW(), available since Vista.

(It's unclear the LoadLibrary wrapper was ever doing anything useful
here, as (i) DLL lookup in PATH was avoided as advapi32 is already
loaded into the process, and (ii) advapi32 is a 'known DLL' which is
only ever loaded from system directory)
---
 winsup/utils/regtool.cc | 13 +
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/winsup/utils/regtool.cc b/winsup/utils/regtool.cc
index fd2dd0590..e919185ae 100644
--- a/winsup/utils/regtool.cc
+++ b/winsup/utils/regtool.cc
@@ -16,7 +16,6 @@ details. */
 #include 
 #include 
 #include 
-#include "loadlib.h"
 
 #define DEFAULT_KEY_SEPARATOR '\\'
 
@@ -589,10 +588,6 @@ cmd_add ()
   return 0;
 }
 
-extern "C" {
-  LONG WINAPI (*regDeleteKeyEx)(HKEY, LPCWSTR, REGSAM, DWORD);
-}
-
 int
 cmd_remove ()
 {
@@ -600,13 +595,7 @@ cmd_remove ()
 
   find_key (2, KEY_ALL_ACCESS);
   if (wow64)
-{
-  HMODULE mod = LoadLibrary ("advapi32.dll");
-  if (mod)
-   regDeleteKeyEx = (LONG WINAPI (*)(HKEY, LPCWSTR, REGSAM, DWORD)) 
GetProcAddress (mod, "RegDeleteKeyExW");
-}
-  if (regDeleteKeyEx)
-rv = (*regDeleteKeyEx) (key, value, wow64, 0);
+rv = RegDeleteKeyExW (key, value, wow64, 0);
   else
 rv = RegDeleteKeyW (key, value);
   if (rv != ERROR_SUCCESS)
-- 
2.35.1



[PATCH v2] Cygwin: pinfo: Fix exit code for non-cygwin apps which reads console.

2022-02-26 Thread Takashi Yano
- The recent commit "Cygwin: pinfo: Fix exit code when non-cygwin app
  exits by Ctrl-C." did not fix enough the issue. If a non-cygwin app
  is reading the console, it will not return STATUS_CONTROL_C_EXIT
  even if it is terminated by Ctrl-C. As a result, the previous patch
  does not take effect.
  This patch solves this issue by setting sigExeced to SIGINT in
  ctrl_c_handler(). In addition, sigExeced will be cleared if the app
  does not terminated within predetermined time period. The reason is
  that the app does not seem to be terminated by the signal sigExeced.
---
 winsup/cygwin/exceptions.cc |  6 +-
 winsup/cygwin/globals.cc|  2 +-
 winsup/cygwin/spawn.cc  | 15 ++-
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 6e0b862c7..070e52e76 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1139,7 +1139,11 @@ ctrl_c_handler (DWORD type)
 }
 
   if (ch_spawn.set_saw_ctrl_c ())
-return TRUE;
+{
+  if (myself->process_state & PID_NOTCYGWIN)
+   sigExeced = SIGINT;
+  return TRUE;
+}
 
   /* We're only the process group leader when we have a valid pinfo structure.
  If we don't have one, then the parent "stub" will handle the signal. */
diff --git a/winsup/cygwin/globals.cc b/winsup/cygwin/globals.cc
index ac5ad0307..d3a2e11a4 100644
--- a/winsup/cygwin/globals.cc
+++ b/winsup/cygwin/globals.cc
@@ -20,7 +20,7 @@ HANDLE NO_COPY hProcImpToken;
 HANDLE my_wr_proc_pipe;
 HMODULE NO_COPY cygwin_hmodule;
 HMODULE NO_COPY hntdll;
-int NO_COPY sigExeced;
+LONG NO_COPY sigExeced;
 WCHAR windows_system_directory[MAX_PATH];
 UINT windows_system_directory_length;
 WCHAR system_wow64_directory[MAX_PATH];
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 3647580a6..3b54309a2 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -953,7 +953,15 @@ child_info_spawn::worker (const char *prog_arg, const char 
*const *argv,
  if (sem)
__posix_spawn_sem_release (sem, 0);
  if (ptys_need_cleanup || cons_need_cleanup)
-   WaitForSingleObject (pi.hProcess, INFINITE);
+   {
+ LONG prev_sigExeced = sigExeced;
+ while (WaitForSingleObject (pi.hProcess, 100) == WAIT_TIMEOUT)
+   /* If child process does not exit in predetermined time
+  period, the process does not seem to be terminated by
+  the signal sigExeced. Therefore, clear sigExeced here. */
+   prev_sigExeced =
+ InterlockedCompareExchange (&sigExeced, 0, prev_sigExeced);
+   }
  if (ptys_need_cleanup)
{
  fhandler_pty_slave::cleanup_for_non_cygwin_app (&ptys_handle_set,
@@ -966,6 +974,11 @@ child_info_spawn::worker (const char *prog_arg, const char 
*const *argv,
  fhandler_console::cleanup_for_non_cygwin_app (&cons_handle_set);
  fhandler_console::close_handle_set (&cons_handle_set);
}
+ /* Make sure that ctrl_c_handler() is not on going. Calling
+init_console_handler(false) locks until returning from
+ctrl_c_handler(). This insures that setting sigExeced
+on Ctrl-C key has been completed. */
+ init_console_handler (false);
  myself.exit (EXITCODE_NOSET);
  break;
case _P_WAIT:
-- 
2.35.1



[PATCH] Cygwin: console: Correct the past fix for apps which open pty.

2022-02-26 Thread Takashi Yano
- The commit "Cygwin: console: Fix issues of apps which open pty."
  did not fix the second problem correctly. That commit looked to
  fix the issue, but the actual problem was that ctrl_c_handler()
  should be reregistered *AFTER* FreeConsole()/AttachConsole().
  This patch correct that.
---
 winsup/cygwin/fhandler_termios.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/fhandler_termios.cc 
b/winsup/cygwin/fhandler_termios.cc
index 568523390..767b28302 100644
--- a/winsup/cygwin/fhandler_termios.cc
+++ b/winsup/cygwin/fhandler_termios.cc
@@ -344,10 +344,10 @@ fhandler_termios::process_sigs (char c, tty* ttyp, 
fhandler_termios *fh)
  (myself->dwProcessId, false);
  if (resume_pid && fh && !fh->is_console ())
{
- if (::cygheap->ctty && ::cygheap->ctty->is_console ())
-   init_console_handler (false);
  FreeConsole ();
  AttachConsole (p->dwProcessId);
+ if (::cygheap->ctty && ::cygheap->ctty->is_console ())
+   init_console_handler (true);
}
  if (fh && p == myself && being_debugged ())
{ /* Avoid deadlock in gdb on console. */
-- 
2.35.1