Re: [PATCH 0/1] Fix PTY state management in pseudo console support.
This patch has a small bug. Please use v2 instead. On Sat, 31 Aug 2019 23:53:17 +0900 Takashi Yano wrote: > Pseudo console support in test release TEST: Cygwin 3.1.0-0.3, > introduced by commit 169d65a5774acc76ce3f3feeedcbae7405aa9b57, > has some bugs which cause mismatch between state variables and > real pseudo console state regarding console attaching and r/w > pipe switching. This patch fixes this issue by redesigning the > state management. > > Takashi Yano (1): > Cygwin: pty: Fix state management for pseudo console support. > > winsup/cygwin/dtable.cc | 15 +- > winsup/cygwin/fhandler.h | 6 +- > winsup/cygwin/fhandler_console.cc | 6 +- > winsup/cygwin/fhandler_tty.cc | 401 -- > winsup/cygwin/fork.cc | 24 +- > winsup/cygwin/spawn.cc| 65 ++--- > 6 files changed, 280 insertions(+), 237 deletions(-) > > -- > 2.21.0 > -- Takashi Yano
[PATCH v2 1/1] Cygwin: pty: Fix state management for pseudo console support.
- Pseudo console support introduced by commit
169d65a5774acc76ce3f3feeedcbae7405aa9b57 has some bugs which
cause mismatch between state variables and real pseudo console
state regarding console attaching and r/w pipe switching. This
patch fixes this issue by redesigning the state management.
---
winsup/cygwin/dtable.cc | 15 +-
winsup/cygwin/fhandler.h | 6 +-
winsup/cygwin/fhandler_console.cc | 6 +-
winsup/cygwin/fhandler_tty.cc | 404 --
winsup/cygwin/fork.cc | 24 +-
winsup/cygwin/spawn.cc| 65 ++---
6 files changed, 283 insertions(+), 237 deletions(-)
diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index ba5d16206..6266f1bc2 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -150,8 +150,11 @@ dtable::stdio_init ()
bool need_fixup_handle = false;
fhandler_pty_slave *ptys = NULL;
bool is_pty[3] = {false, false, false};
- for (int fd = 0; fd < 3; fd ++)
+ bool already_attached = false;
+ int chk_order[] = {1, 0, 2};
+ for (int i = 0; i < 3; i ++)
{
+ int fd = chk_order[i];
fhandler_base *fh = cygheap->fdtab[fd];
if (fh && fh->get_major () == DEV_PTYS_MAJOR)
{
@@ -161,14 +164,18 @@ dtable::stdio_init ()
is_pty[fd] = true;
bool attached = !!fhandler_console::get_console_process_id
(ptys->getHelperProcessId (), true);
- if (!attached)
+ already_attached = already_attached || attached;
+ if (!already_attached)
{
/* Not attached to pseudo console in fork() or spawn()
by some reason. This happens if the executable is
a windows GUI binary, such as mintty. */
FreeConsole ();
- AttachConsole (ptys->getHelperProcessId ());
- need_fixup_handle = true;
+ if (AttachConsole (ptys->getHelperProcessId ()))
+ {
+ need_fixup_handle = true;
+ already_attached = true;
+ }
}
ptys->reset_switch_to_pcon ();
}
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index c75e40c0a..af2b948cb 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2106,19 +2106,22 @@ class fhandler_pty_common: public fhandler_termios
protected:
BOOL process_opost_output (HANDLE h,
const void *ptr, ssize_t& len, bool is_echo);
- bool check_switch_to_pcon (void);
};
class fhandler_pty_slave: public fhandler_pty_common
{
HANDLE inuse;// used to indicate that a tty is in use
HANDLE output_handle_cyg, io_handle_cyg;
+ DWORD pidRestore;
/* Helper functions for fchmod and fchown. */
bool fch_open_handles (bool chown);
int fch_set_sd (security_descriptor , bool chown);
void fch_close_handles ();
+ bool try_reattach_pcon ();
+ void restore_reattach_pcon ();
+
public:
/* Constructor */
fhandler_pty_slave (int);
@@ -2172,7 +2175,6 @@ class fhandler_pty_slave: public fhandler_pty_common
void set_switch_to_pcon (void);
void reset_switch_to_pcon (void);
void push_to_pcon_screenbuffer (const char *ptr, size_t len);
- bool has_master_opened (void);
void mask_switch_to_pcon (bool mask)
{
get_ttyp ()->mask_switch_to_pcon = mask;
diff --git a/winsup/cygwin/fhandler_console.cc
b/winsup/cygwin/fhandler_console.cc
index 997c50d23..ae7f66b80 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -3138,14 +3138,14 @@ fhandler_console::get_console_process_id (DWORD pid,
bool match)
DWORD tmp;
int num = GetConsoleProcessList (, 1);
DWORD *list = (DWORD *)
- HeapAlloc (GetProcessHeap (), 0, num * sizeof (DWORD));
- num = GetConsoleProcessList (list, num);
+ HeapAlloc (GetProcessHeap (), 0, (num + 16) * sizeof (DWORD));
+ num = GetConsoleProcessList (list, num + 16);
tmp = 0;
for (int i=0; iset_switch_to_pcon ();
- return;
}
}
@@ -381,25 +380,6 @@ fhandler_pty_common::__release_output_mutex (const char
*fn, int ln)
#endif
}
-static bool switch_to_pcon_prev;
-
-bool
-fhandler_pty_common::check_switch_to_pcon (void)
-{
- bool switch_to_pcon_now = get_ttyp ()->switch_to_pcon;
- if (!isHybrid && !switch_to_pcon_prev && switch_to_pcon_now)
-{
- Sleep (40);
- /* Check again */
- switch_to_pcon_now = get_ttyp ()->switch_to_pcon;
- if (switch_to_pcon_now)
- switch_to_pcon_prev = true;
-}
- else
-switch_to_pcon_prev = switch_to_pcon_now;
- return switch_to_pcon_prev;
-}
-
/* Process pty input. */
void
@@ -595,7 +575,7 @@ out:
fhandler_pty_slave::fhandler_pty_slave (int unit)
: fhandler_pty_common (), inuse (NULL), output_handle_cyg (NULL),
- io_handle_cyg (NULL)
+ io_handle_cyg (NULL),
[PATCH v2 0/1] Fix PTY state management in pseudo console support.
Pseudo console support in test release TEST: Cygwin 3.1.0-0.3, introduced by commit 169d65a5774acc76ce3f3feeedcbae7405aa9b57, has some bugs which cause mismatch between state variables and real pseudo console state regarding console attaching and r/w pipe switching. This patch fixes this issue by redesigning the state management. v2: Small bug fixed from v1. Takashi Yano (1): Cygwin: pty: Fix state management for pseudo console support. winsup/cygwin/dtable.cc | 15 +- winsup/cygwin/fhandler.h | 6 +- winsup/cygwin/fhandler_console.cc | 6 +- winsup/cygwin/fhandler_tty.cc | 404 -- winsup/cygwin/fork.cc | 24 +- winsup/cygwin/spawn.cc| 65 ++--- 6 files changed, 283 insertions(+), 237 deletions(-) -- 2.21.0
[PATCH 0/1] Fix PTY state management in pseudo console support.
Pseudo console support in test release TEST: Cygwin 3.1.0-0.3, introduced by commit 169d65a5774acc76ce3f3feeedcbae7405aa9b57, has some bugs which cause mismatch between state variables and real pseudo console state regarding console attaching and r/w pipe switching. This patch fixes this issue by redesigning the state management. Takashi Yano (1): Cygwin: pty: Fix state management for pseudo console support. winsup/cygwin/dtable.cc | 15 +- winsup/cygwin/fhandler.h | 6 +- winsup/cygwin/fhandler_console.cc | 6 +- winsup/cygwin/fhandler_tty.cc | 401 -- winsup/cygwin/fork.cc | 24 +- winsup/cygwin/spawn.cc| 65 ++--- 6 files changed, 280 insertions(+), 237 deletions(-) -- 2.21.0
[PATCH 1/1] Cygwin: pty: Fix state management for pseudo console support.
- Pseudo console support introduced by commit
169d65a5774acc76ce3f3feeedcbae7405aa9b57 has some bugs which
cause mismatch between state variables and real pseudo console
state regarding console attaching and r/w pipe switching. This
patch fixes this issue by redesigning the state management.
---
winsup/cygwin/dtable.cc | 15 +-
winsup/cygwin/fhandler.h | 6 +-
winsup/cygwin/fhandler_console.cc | 6 +-
winsup/cygwin/fhandler_tty.cc | 401 --
winsup/cygwin/fork.cc | 24 +-
winsup/cygwin/spawn.cc| 65 ++---
6 files changed, 280 insertions(+), 237 deletions(-)
diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index ba5d16206..6266f1bc2 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -150,8 +150,11 @@ dtable::stdio_init ()
bool need_fixup_handle = false;
fhandler_pty_slave *ptys = NULL;
bool is_pty[3] = {false, false, false};
- for (int fd = 0; fd < 3; fd ++)
+ bool already_attached = false;
+ int chk_order[] = {1, 0, 2};
+ for (int i = 0; i < 3; i ++)
{
+ int fd = chk_order[i];
fhandler_base *fh = cygheap->fdtab[fd];
if (fh && fh->get_major () == DEV_PTYS_MAJOR)
{
@@ -161,14 +164,18 @@ dtable::stdio_init ()
is_pty[fd] = true;
bool attached = !!fhandler_console::get_console_process_id
(ptys->getHelperProcessId (), true);
- if (!attached)
+ already_attached = already_attached || attached;
+ if (!already_attached)
{
/* Not attached to pseudo console in fork() or spawn()
by some reason. This happens if the executable is
a windows GUI binary, such as mintty. */
FreeConsole ();
- AttachConsole (ptys->getHelperProcessId ());
- need_fixup_handle = true;
+ if (AttachConsole (ptys->getHelperProcessId ()))
+ {
+ need_fixup_handle = true;
+ already_attached = true;
+ }
}
ptys->reset_switch_to_pcon ();
}
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index c75e40c0a..af2b948cb 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2106,19 +2106,22 @@ class fhandler_pty_common: public fhandler_termios
protected:
BOOL process_opost_output (HANDLE h,
const void *ptr, ssize_t& len, bool is_echo);
- bool check_switch_to_pcon (void);
};
class fhandler_pty_slave: public fhandler_pty_common
{
HANDLE inuse;// used to indicate that a tty is in use
HANDLE output_handle_cyg, io_handle_cyg;
+ DWORD pidRestore;
/* Helper functions for fchmod and fchown. */
bool fch_open_handles (bool chown);
int fch_set_sd (security_descriptor , bool chown);
void fch_close_handles ();
+ bool try_reattach_pcon ();
+ void restore_reattach_pcon ();
+
public:
/* Constructor */
fhandler_pty_slave (int);
@@ -2172,7 +2175,6 @@ class fhandler_pty_slave: public fhandler_pty_common
void set_switch_to_pcon (void);
void reset_switch_to_pcon (void);
void push_to_pcon_screenbuffer (const char *ptr, size_t len);
- bool has_master_opened (void);
void mask_switch_to_pcon (bool mask)
{
get_ttyp ()->mask_switch_to_pcon = mask;
diff --git a/winsup/cygwin/fhandler_console.cc
b/winsup/cygwin/fhandler_console.cc
index 997c50d23..ae7f66b80 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -3138,14 +3138,14 @@ fhandler_console::get_console_process_id (DWORD pid,
bool match)
DWORD tmp;
int num = GetConsoleProcessList (, 1);
DWORD *list = (DWORD *)
- HeapAlloc (GetProcessHeap (), 0, num * sizeof (DWORD));
- num = GetConsoleProcessList (list, num);
+ HeapAlloc (GetProcessHeap (), 0, (num + 16) * sizeof (DWORD));
+ num = GetConsoleProcessList (list, num + 16);
tmp = 0;
for (int i=0; iset_switch_to_pcon ();
- return;
}
}
@@ -381,25 +380,6 @@ fhandler_pty_common::__release_output_mutex (const char
*fn, int ln)
#endif
}
-static bool switch_to_pcon_prev;
-
-bool
-fhandler_pty_common::check_switch_to_pcon (void)
-{
- bool switch_to_pcon_now = get_ttyp ()->switch_to_pcon;
- if (!isHybrid && !switch_to_pcon_prev && switch_to_pcon_now)
-{
- Sleep (40);
- /* Check again */
- switch_to_pcon_now = get_ttyp ()->switch_to_pcon;
- if (switch_to_pcon_now)
- switch_to_pcon_prev = true;
-}
- else
-switch_to_pcon_prev = switch_to_pcon_now;
- return switch_to_pcon_prev;
-}
-
/* Process pty input. */
void
@@ -595,7 +575,7 @@ out:
fhandler_pty_slave::fhandler_pty_slave (int unit)
: fhandler_pty_common (), inuse (NULL), output_handle_cyg (NULL),
- io_handle_cyg (NULL)
+ io_handle_cyg (NULL),
