* [PATCH 0/5] Some revisions for pty and console code.
@ 2021-01-15 8:32 Takashi Yano
2021-01-15 8:32 ` [PATCH 1/5] Cygwin: pty: Add workaround for rlwrap 0.40 or later Takashi Yano
` (4 more replies)
0 siblings, 5 replies; 8+ messages in thread
From: Takashi Yano @ 2021-01-15 8:32 UTC (permalink / raw)
To: cygwin-patches
Takashi Yano (5):
Cygwin: pty: Add workaround for rlwrap 0.40 or later.
Cygwin: console: Revise the code to switch xterm mode.
Cygwin: pty: Make close_pseudoconsole() be a static member function.
Cygwin: pty: Prevent pty from changing code page of parent console.
Cygwin: pty: Make master thread functions be static.
winsup/cygwin/fhandler.h | 56 ++++--
winsup/cygwin/fhandler_console.cc | 159 +++++++++++-----
winsup/cygwin/fhandler_tty.cc | 305 +++++++++++++++++++++++-------
winsup/cygwin/select.cc | 15 +-
winsup/cygwin/spawn.cc | 42 +++-
winsup/cygwin/tty.cc | 2 +
winsup/cygwin/tty.h | 2 +
7 files changed, 440 insertions(+), 141 deletions(-)
--
2.30.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/5] Cygwin: pty: Add workaround for rlwrap 0.40 or later.
2021-01-15 8:32 [PATCH 0/5] Some revisions for pty and console code Takashi Yano
@ 2021-01-15 8:32 ` Takashi Yano
2021-01-15 8:32 ` [PATCH 2/5] Cygwin: console: Revise the code to switch xterm mode Takashi Yano
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Takashi Yano @ 2021-01-15 8:32 UTC (permalink / raw)
To: cygwin-patches
- The workaround for rlwrap introduced by commit 8199b0cc does not
take effect for rlwrap 0.40 or later. This patch add a workaround
for rlwrap 0.40 or later as well.
---
winsup/cygwin/fhandler_tty.cc | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 77f7bfe43..8ff74cdde 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -1720,6 +1720,11 @@ int
fhandler_pty_master::tcgetattr (struct termios *t)
{
*t = cygwin_shared->tty[get_minor ()]->ti;
+ /* Workaround for rlwrap v0.40 or later */
+ if (get_ttyp ()->pcon_start)
+ t->c_lflag &= ~(ICANON | ECHO);
+ if (get_ttyp ()->h_pseudo_console)
+ t->c_iflag &= ~ICRNL;
return 0;
}
--
2.30.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/5] Cygwin: console: Revise the code to switch xterm mode.
2021-01-15 8:32 [PATCH 0/5] Some revisions for pty and console code Takashi Yano
2021-01-15 8:32 ` [PATCH 1/5] Cygwin: pty: Add workaround for rlwrap 0.40 or later Takashi Yano
@ 2021-01-15 8:32 ` Takashi Yano
2021-01-15 8:32 ` [PATCH 3/5] Cygwin: pty: Make close_pseudoconsole() be a static member function Takashi Yano
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Takashi Yano @ 2021-01-15 8:32 UTC (permalink / raw)
To: cygwin-patches
- If application changes the console mode, mode management introduced
by commit 10d8c278 will be corrupted. For example, stdout of jansi
v2.0.1 or later is piped to less, jansi resets the xterm mode flag
ENABLE_VIRTUAL_TERMINA_PROCESSING when jansi is terminated. This
causes garbled output in less because less needs this flag enabled.
This patch fixes the issue.
---
winsup/cygwin/fhandler.h | 18 ++++-
winsup/cygwin/fhandler_console.cc | 121 +++++++++++++++++++-----------
winsup/cygwin/select.cc | 15 +---
winsup/cygwin/spawn.cc | 35 ++++++++-
4 files changed, 127 insertions(+), 62 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index fe76c0781..45ac17af2 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2048,8 +2048,6 @@ class dev_console
bool raw_win32_keyboard_mode;
char cons_rabuf[40]; // cannot get longer than char buf[40] in char_command
char *cons_rapoi;
- LONG xterm_mode_input;
- LONG xterm_mode_output;
bool cursor_key_app_mode;
inline UINT get_console_cp ();
@@ -2086,11 +2084,19 @@ public:
input_signalled = 2,
input_winch = 3
};
+ struct handle_set_t
+ {
+ HANDLE input_handle;
+ HANDLE output_handle;
+ HANDLE input_mutex;
+ HANDLE output_mutex;
+ };
private:
static const unsigned MAX_WRITE_CHARS;
static console_state *shared_console_info;
static bool invisible_console;
HANDLE input_mutex, output_mutex;
+ handle_set_t handle_set;
/* Used when we encounter a truncated multi-byte sequence. The
lead bytes are stored here and revisited in the next write call. */
@@ -2212,8 +2218,12 @@ private:
size_t &raixput ();
size_t &rabuflen ();
- void request_xterm_mode_input (bool);
- void request_xterm_mode_output (bool);
+ const handle_set_t *get_handle_set (void) {return &handle_set;}
+ void get_duplicated_handle_set (handle_set_t *p);
+ static void close_handle_set (handle_set_t *p);
+
+ static void request_xterm_mode_input (bool, const handle_set_t *p);
+ static void request_xterm_mode_output (bool, const handle_set_t *p);
friend tty_min * tty_list::get_cttyp ();
};
diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
index 41cac37e6..a4c054e24 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -187,11 +187,7 @@ fhandler_console::set_unit ()
tty_min_state.setntty (DEV_CONS_MAJOR, console_unit (me));
devset = (fh_devices) shared_console_info->tty_min_state.getntty ();
if (created)
- {
- con.owner = myself->pid;
- con.xterm_mode_input = 0;
- con.xterm_mode_output = 0;
- }
+ con.owner = myself->pid;
}
if (!created && shared_console_info)
{
@@ -279,60 +275,65 @@ fhandler_console::rabuflen ()
return con_ra.rabuflen;
}
+/* The function request_xterm_mode_{in,out}put() should be static so that
+ they can be called even after the fhandler_console instance is deleted. */
void
-fhandler_console::request_xterm_mode_input (bool req)
+fhandler_console::request_xterm_mode_input (bool req, const handle_set_t *p)
{
if (con_is_legacy)
return;
+ WaitForSingleObject (p->input_mutex, INFINITE);
+ DWORD dwMode;
+ GetConsoleMode (p->input_handle, &dwMode);
if (req)
{
- if (InterlockedExchange (&con.xterm_mode_input, 1) == 0)
+ if (!(dwMode & ENABLE_VIRTUAL_TERMINAL_INPUT))
{
- DWORD dwMode;
- GetConsoleMode (get_handle (), &dwMode);
dwMode |= ENABLE_VIRTUAL_TERMINAL_INPUT;
- SetConsoleMode (get_handle (), dwMode);
+ SetConsoleMode (p->input_handle, dwMode);
if (con.cursor_key_app_mode) /* Restore DECCKM */
- WriteConsoleW (get_output_handle (), L"\033[?1h", 5, NULL, 0);
+ {
+ request_xterm_mode_output (true, p);
+ WriteConsoleW (p->output_handle, L"\033[?1h", 5, NULL, 0);
+ }
}
}
else
{
- if (InterlockedExchange (&con.xterm_mode_input, 0) == 1)
+ if (dwMode & ENABLE_VIRTUAL_TERMINAL_INPUT)
{
- DWORD dwMode;
- GetConsoleMode (get_handle (), &dwMode);
dwMode &= ~ENABLE_VIRTUAL_TERMINAL_INPUT;
- SetConsoleMode (get_handle (), dwMode);
+ SetConsoleMode (p->input_handle, dwMode);
}
}
+ ReleaseMutex (p->input_mutex);
}
void
-fhandler_console::request_xterm_mode_output (bool req)
+fhandler_console::request_xterm_mode_output (bool req, const handle_set_t *p)
{
if (con_is_legacy)
return;
+ WaitForSingleObject (p->output_mutex, INFINITE);
+ DWORD dwMode;
+ GetConsoleMode (p->output_handle, &dwMode);
if (req)
{
- if (InterlockedExchange (&con.xterm_mode_output, 1) == 0)
+ if (!(dwMode & ENABLE_VIRTUAL_TERMINAL_PROCESSING))
{
- DWORD dwMode;
- GetConsoleMode (get_output_handle (), &dwMode);
dwMode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
- SetConsoleMode (get_output_handle (), dwMode);
+ SetConsoleMode (p->output_handle, dwMode);
}
}
else
{
- if (InterlockedExchange (&con.xterm_mode_output, 0) == 1)
+ if (dwMode & ENABLE_VIRTUAL_TERMINAL_PROCESSING)
{
- DWORD dwMode;
- GetConsoleMode (get_output_handle (), &dwMode);
dwMode &= ~ENABLE_VIRTUAL_TERMINAL_PROCESSING;
- SetConsoleMode (get_output_handle (), dwMode);
+ SetConsoleMode (p->output_handle, dwMode);
}
}
+ ReleaseMutex (p->output_mutex);
}
/* Return the tty structure associated with a given tty number. If the
@@ -440,8 +441,8 @@ fhandler_console::fix_tab_position (void)
{
/* Re-setting ENABLE_VIRTUAL_TERMINAL_PROCESSING
fixes the tab position. */
- request_xterm_mode_output (false);
- request_xterm_mode_output (true);
+ request_xterm_mode_output (false, &handle_set);
+ request_xterm_mode_output (true, &handle_set);
}
bool
@@ -506,7 +507,7 @@ fhandler_console::read (void *pv, size_t& buflen)
/* if system has 24 bit color capability, use xterm compatible mode. */
if (wincap.has_con_24bit_colors ())
- request_xterm_mode_input (true);
+ request_xterm_mode_input (true, &handle_set);
while (!input_ready && !get_cons_readahead_valid ())
{
@@ -514,8 +515,6 @@ fhandler_console::read (void *pv, size_t& buflen)
if ((bgres = bg_check (SIGTTIN)) <= bg_eof)
{
buflen = bgres;
- if (wincap.has_con_24bit_colors ())
- request_xterm_mode_input (false);
return;
}
@@ -533,8 +532,6 @@ fhandler_console::read (void *pv, size_t& buflen)
case WAIT_TIMEOUT:
set_sig_errno (EAGAIN);
buflen = (size_t) -1;
- if (wincap.has_con_24bit_colors ())
- request_xterm_mode_input (false);
return;
default:
goto err;
@@ -582,22 +579,16 @@ fhandler_console::read (void *pv, size_t& buflen)
#undef buf
buflen = copied_chars;
- if (wincap.has_con_24bit_colors ())
- request_xterm_mode_input (false);
return;
err:
__seterrno ();
buflen = (size_t) -1;
- if (wincap.has_con_24bit_colors ())
- request_xterm_mode_input (false);
return;
sig_exit:
set_sig_errno (EINTR);
buflen = (size_t) -1;
- if (wincap.has_con_24bit_colors ())
- request_xterm_mode_input (false);
}
fhandler_console::input_states
@@ -1106,6 +1097,7 @@ fhandler_console::open (int flags, mode_t)
return 0;
}
set_handle (h);
+ handle_set.input_handle = h;
h = CreateFileW (L"CONOUT$", GENERIC_READ | GENERIC_WRITE,
FILE_SHARE_READ | FILE_SHARE_WRITE, &sec_none,
@@ -1117,8 +1109,11 @@ fhandler_console::open (int flags, mode_t)
return 0;
}
set_output_handle (h);
+ handle_set.output_handle = h;
setup_io_mutex ();
+ handle_set.input_mutex = input_mutex;
+ handle_set.output_mutex = output_mutex;
if (con.fillin (get_output_handle ()))
{
@@ -1191,8 +1186,10 @@ fhandler_console::close ()
&obi, sizeof obi, NULL);
if ((NT_SUCCESS (status) && obi.HandleCount == 1)
|| myself->pid == con.owner)
- request_xterm_mode_output (false);
- request_xterm_mode_input (false);
+ {
+ request_xterm_mode_output (false, &handle_set);
+ request_xterm_mode_input (false, &handle_set);
+ }
}
release_output_mutex ();
@@ -1354,10 +1351,14 @@ fhandler_console::output_tcsetattr (int, struct termios const *t)
/* All the output bits we can ignore */
acquire_output_mutex (INFINITE);
- if (wincap.has_con_24bit_colors ())
- request_xterm_mode_output (false);
DWORD flags = ENABLE_PROCESSED_OUTPUT | ENABLE_WRAP_AT_EOL_OUTPUT;
+ DWORD oflags;
+ GetConsoleMode (get_output_handle (), &oflags);
+ if (wincap.has_con_24bit_colors () && !con_is_legacy
+ && (oflags & ENABLE_VIRTUAL_TERMINAL_PROCESSING))
+ flags |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
+
int res = SetConsoleMode (get_output_handle (), flags) ? 0 : -1;
if (res)
__seterrno_from_win_error (GetLastError ());
@@ -1419,6 +1420,10 @@ fhandler_console::input_tcsetattr (int, struct termios const *t)
((wincap.has_con_24bit_colors () && !con_is_legacy) ?
0 : ENABLE_MOUSE_INPUT);
+ if (wincap.has_con_24bit_colors () && !con_is_legacy
+ && (oflags & ENABLE_VIRTUAL_TERMINAL_INPUT))
+ flags |= ENABLE_VIRTUAL_TERMINAL_INPUT;
+
int res;
if (flags == oflags)
res = 0;
@@ -2973,7 +2978,7 @@ fhandler_console::write (const void *vsrc, size_t len)
/* If system has 24 bit color capability, use xterm compatible mode. */
if (wincap.has_con_24bit_colors ())
- request_xterm_mode_output (true);
+ request_xterm_mode_output (true, &handle_set);
if (wincap.has_con_24bit_colors () && !con_is_legacy)
{
DWORD dwMode;
@@ -3657,3 +3662,35 @@ fhandler_console::__release_output_mutex (const char *fn, int ln)
strace.prntf (_STRACE_TERMIOS, fn, "(%d): release output_mutex", ln);
#endif
}
+
+void
+fhandler_console::get_duplicated_handle_set (handle_set_t *p)
+{
+ DuplicateHandle (GetCurrentProcess (), get_handle (),
+ GetCurrentProcess (), &p->input_handle,
+ 0, FALSE, DUPLICATE_SAME_ACCESS);
+ DuplicateHandle (GetCurrentProcess (), get_output_handle (),
+ GetCurrentProcess (), &p->output_handle,
+ 0, FALSE, DUPLICATE_SAME_ACCESS);
+ DuplicateHandle (GetCurrentProcess (), input_mutex,
+ GetCurrentProcess (), &p->input_mutex,
+ 0, FALSE, DUPLICATE_SAME_ACCESS);
+ DuplicateHandle (GetCurrentProcess (), output_mutex,
+ GetCurrentProcess (), &p->output_mutex,
+ 0, FALSE, DUPLICATE_SAME_ACCESS);
+}
+
+/* The function close_handle_set() should be static so that they can
+ be called even after the fhandler_console instance is deleted. */
+void
+fhandler_console::close_handle_set (handle_set_t *p)
+{
+ CloseHandle (p->input_handle);
+ p->input_handle = NULL;
+ CloseHandle (p->output_handle);
+ p->output_handle = NULL;
+ CloseHandle (p->input_mutex);
+ p->input_mutex = NULL;
+ CloseHandle (p->output_mutex);
+ p->output_mutex = NULL;
+}
diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index 501714fa7..dcb9b2d6e 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -1087,28 +1087,15 @@ verify_console (select_record *me, fd_set *rfds, fd_set *wfds,
return peek_console (me, true);
}
-static void console_cleanup (select_record *, select_stuff *);
-
static int
console_startup (select_record *me, select_stuff *stuff)
{
fhandler_console *fh = (fhandler_console *) me->fh;
if (wincap.has_con_24bit_colors ())
- {
- fh->request_xterm_mode_input (true);
- me->cleanup = console_cleanup;
- }
+ fhandler_console::request_xterm_mode_input (true, fh->get_handle_set ());
return 1;
}
-static void
-console_cleanup (select_record *me, select_stuff *stuff)
-{
- fhandler_console *fh = (fhandler_console *) me->fh;
- if (wincap.has_con_24bit_colors ())
- fh->request_xterm_mode_input (false);
-}
-
select_record *
fhandler_console::select_read (select_stuff *ss)
{
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 5057af932..94909df4c 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -606,6 +606,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
sa = &sec_none_nih;
fhandler_pty_slave *ptys_primary = NULL;
+ fhandler_console *cons_native = NULL;
for (int i = 0; i < 3; i ++)
{
const int chk_order[] = {1, 0, 2};
@@ -621,10 +622,23 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
{
fhandler_console *cons = (fhandler_console *) fh;
if (wincap.has_con_24bit_colors () && !iscygwin ())
- if (fd == 1 || fd == 2)
- cons->request_xterm_mode_output (false);
+ {
+ if (cons_native == NULL)
+ cons_native = cons;
+ if (fd == 0)
+ fhandler_console::request_xterm_mode_input (false,
+ cons->get_handle_set ());
+ else if (fd == 1 || fd == 2)
+ fhandler_console::request_xterm_mode_output (false,
+ cons->get_handle_set ());
+ }
}
}
+ struct fhandler_console::handle_set_t cons_handle_set = { 0, };
+ if (cons_native)
+ /* Console handles will be closed by close_all_handle(),
+ therefore, duplicate them here */
+ cons_native->get_duplicated_handle_set (&cons_handle_set);
if (!iscygwin ())
{
@@ -942,6 +956,15 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
WaitForSingleObject (pi.hProcess, INFINITE);
ptys_primary->close_pseudoconsole ();
}
+ else if (cons_native)
+ {
+ WaitForSingleObject (pi.hProcess, INFINITE);
+ fhandler_console::request_xterm_mode_output (true,
+ &cons_handle_set);
+ fhandler_console::request_xterm_mode_input (true,
+ &cons_handle_set);
+ fhandler_console::close_handle_set (&cons_handle_set);
+ }
myself.exit (EXITCODE_NOSET);
break;
case _P_WAIT:
@@ -951,6 +974,14 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
res = -1;
if (enable_pcon)
ptys_primary->close_pseudoconsole ();
+ else if (cons_native)
+ {
+ fhandler_console::request_xterm_mode_output (true,
+ &cons_handle_set);
+ fhandler_console::request_xterm_mode_input (true,
+ &cons_handle_set);
+ fhandler_console::close_handle_set (&cons_handle_set);
+ }
break;
case _P_DETACH:
res = 0; /* Lost all memory of this child. */
--
2.30.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/5] Cygwin: pty: Make close_pseudoconsole() be a static member function.
2021-01-15 8:32 [PATCH 0/5] Some revisions for pty and console code Takashi Yano
2021-01-15 8:32 ` [PATCH 1/5] Cygwin: pty: Add workaround for rlwrap 0.40 or later Takashi Yano
2021-01-15 8:32 ` [PATCH 2/5] Cygwin: console: Revise the code to switch xterm mode Takashi Yano
@ 2021-01-15 8:32 ` Takashi Yano
2021-01-15 8:32 ` [PATCH 4/5] Cygwin: pty: Prevent pty from changing code page of parent console Takashi Yano
2021-01-15 8:32 ` [PATCH 5/5] Cygwin: pty: Make master thread functions be static Takashi Yano
4 siblings, 0 replies; 8+ messages in thread
From: Takashi Yano @ 2021-01-15 8:32 UTC (permalink / raw)
To: cygwin-patches
- The function close_pseudoconsole() should be static so that it
can be safely called in spawn.cc even after the fhandler_pty_slave
instance has been deleted. That is, there is a problem with the
current code. This patch fixes the issue.
---
winsup/cygwin/fhandler.h | 3 ++-
winsup/cygwin/fhandler_tty.cc | 22 ++++++++++++----------
winsup/cygwin/spawn.cc | 6 ++++--
3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 45ac17af2..2077b5245 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2339,12 +2339,13 @@ class fhandler_pty_slave: public fhandler_pty_common
return fh;
}
bool setup_pseudoconsole (STARTUPINFOEXW *si, bool nopcon);
- void close_pseudoconsole (void);
+ static void close_pseudoconsole (tty *ttyp);
bool term_has_pcon_cap (const WCHAR *env);
void set_switch_to_pcon (void);
void reset_switch_to_pcon (void);
void mask_switch_to_pcon_in (bool mask);
void setup_locale (void);
+ tty *get_ttyp () { return (tty *) tc (); } /* Override as public */
};
#define __ptsname(buf, unit) __small_sprintf ((buf), "/dev/pty%d", (unit))
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 8ff74cdde..0c92f41d4 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -2563,21 +2563,23 @@ fallback:
return false;
}
+/* The function close_pseudoconsole() should be static so that it can
+ be called even after the fhandler_pty_slave instance is deleted. */
void
-fhandler_pty_slave::close_pseudoconsole (void)
+fhandler_pty_slave::close_pseudoconsole (tty *ttyp)
{
- if (get_ttyp ()->h_pseudo_console)
+ if (ttyp->h_pseudo_console)
{
- get_ttyp ()->wait_pcon_fwd ();
- HPCON_INTERNAL *hp = (HPCON_INTERNAL *) get_ttyp ()->h_pseudo_console;
+ ttyp->wait_pcon_fwd ();
+ HPCON_INTERNAL *hp = (HPCON_INTERNAL *) ttyp->h_pseudo_console;
HANDLE tmp = hp->hConHostProcess;
- ClosePseudoConsole (get_ttyp ()->h_pseudo_console);
+ ClosePseudoConsole (ttyp->h_pseudo_console);
CloseHandle (tmp);
- get_ttyp ()->h_pseudo_console = NULL;
- get_ttyp ()->switch_to_pcon_in = false;
- get_ttyp ()->pcon_pid = 0;
- get_ttyp ()->pcon_start = false;
- get_ttyp ()->do_not_resize_pcon = false;
+ ttyp->h_pseudo_console = NULL;
+ ttyp->switch_to_pcon_in = false;
+ ttyp->pcon_pid = 0;
+ ttyp->pcon_start = false;
+ ttyp->do_not_resize_pcon = false;
}
}
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 94909df4c..bf1b08057 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -664,6 +664,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
init_console_handler (myself->ctty > 0);
bool enable_pcon = false;
+ tty *ptys_ttyp = NULL;
STARTUPINFOEXW si_pcon;
ZeroMemory (&si_pcon, sizeof (si_pcon));
STARTUPINFOW *si_tmp = &si;
@@ -677,6 +678,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
c_flags |= EXTENDED_STARTUPINFO_PRESENT;
si_tmp = &si_pcon.StartupInfo;
enable_pcon = true;
+ ptys_ttyp = ptys_primary->get_ttyp ();
}
}
@@ -954,7 +956,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
if (enable_pcon)
{
WaitForSingleObject (pi.hProcess, INFINITE);
- ptys_primary->close_pseudoconsole ();
+ fhandler_pty_slave::close_pseudoconsole (ptys_ttyp);
}
else if (cons_native)
{
@@ -973,7 +975,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
if (waitpid (cygpid, &res, 0) != cygpid)
res = -1;
if (enable_pcon)
- ptys_primary->close_pseudoconsole ();
+ fhandler_pty_slave::close_pseudoconsole (ptys_ttyp);
else if (cons_native)
{
fhandler_console::request_xterm_mode_output (true,
--
2.30.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 4/5] Cygwin: pty: Prevent pty from changing code page of parent console.
2021-01-15 8:32 [PATCH 0/5] Some revisions for pty and console code Takashi Yano
` (2 preceding siblings ...)
2021-01-15 8:32 ` [PATCH 3/5] Cygwin: pty: Make close_pseudoconsole() be a static member function Takashi Yano
@ 2021-01-15 8:32 ` Takashi Yano
2021-01-18 10:23 ` Corinna Vinschen
2021-01-15 8:32 ` [PATCH 5/5] Cygwin: pty: Make master thread functions be static Takashi Yano
4 siblings, 1 reply; 8+ messages in thread
From: Takashi Yano @ 2021-01-15 8:32 UTC (permalink / raw)
To: cygwin-patches
- After commit 232fde0e, pty changes console code page when the first
non-cygwin app is executed. If pty is started in real console device,
pty changes the code page of root console. This causes very annoying
result because changing code page changes the font of command prompt
if console is in legacy mode. This patch avoids this by creating a
new invisible console for the first pty started in console device.
---
winsup/cygwin/fhandler.h | 5 +-
winsup/cygwin/fhandler_console.cc | 38 +++++++--
winsup/cygwin/fhandler_tty.cc | 126 +++++++++++++++++++++++++++++-
winsup/cygwin/spawn.cc | 1 +
winsup/cygwin/tty.cc | 2 +
winsup/cygwin/tty.h | 2 +
6 files changed, 163 insertions(+), 11 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 2077b5245..ffd19a590 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2127,7 +2127,7 @@ private:
int input_tcsetattr (int a, const struct termios *t);
void set_cursor_maybe ();
static bool create_invisible_console (HWINSTA);
- static bool create_invisible_console_workaround ();
+ static bool create_invisible_console_workaround (bool force);
static console_state *open_shared_console (HWND, HANDLE&, bool&);
void fix_tab_position (void);
@@ -2185,7 +2185,7 @@ private:
bool send_winch_maybe ();
void setup ();
bool set_unit ();
- static bool need_invisible ();
+ static bool need_invisible (bool force=false);
static void free_console ();
static const char *get_nonascii_key (INPUT_RECORD& input_rec, char *);
@@ -2346,6 +2346,7 @@ class fhandler_pty_slave: public fhandler_pty_common
void mask_switch_to_pcon_in (bool mask);
void setup_locale (void);
tty *get_ttyp () { return (tty *) tc (); } /* Override as public */
+ void create_invisible_console (void);
};
#define __ptsname(buf, unit) __small_sprintf ((buf), "/dev/pty%d", (unit))
diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
index a4c054e24..dd00079fa 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -53,6 +53,23 @@ fhandler_console::console_state NO_COPY *fhandler_console::shared_console_info;
bool NO_COPY fhandler_console::invisible_console;
+/* Mutex for AttachConsole()/FreeConsole() in fhandler_tty.cc */
+HANDLE NO_COPY attach_mutex;
+
+static inline void
+acquire_attach_mutex (DWORD t)
+{
+ if (attach_mutex)
+ WaitForSingleObject (attach_mutex, t);
+}
+
+static inline void
+release_attach_mutex ()
+{
+ if (attach_mutex)
+ ReleaseMutex (attach_mutex);
+}
+
/* con_ra is shared in the same process.
Only one console can exist in a process, therefore, static is suitable. */
static struct fhandler_base::rabuf_t con_ra;
@@ -599,6 +616,8 @@ fhandler_console::process_input_message (void)
if (!shared_console_info)
return input_error;
+ acquire_attach_mutex (INFINITE);
+
termios *ti = &(get_ttyp ()->ti);
fhandler_console::input_states stat = input_processing;
@@ -608,6 +627,7 @@ fhandler_console::process_input_message (void)
if (!PeekConsoleInputW (get_handle (), input_rec, INREC_SIZE, &total_read))
{
termios_printf ("PeekConsoleInput failed, %E");
+ release_attach_mutex ();
return input_error;
}
@@ -972,6 +992,7 @@ out:
/* Discard processed recored. */
DWORD dummy;
ReadConsoleInputW (get_handle (), input_rec, min (total_read, i+1), &dummy);
+ release_attach_mutex ();
return stat;
}
@@ -2973,6 +2994,7 @@ fhandler_console::write (const void *vsrc, size_t len)
if (bg <= bg_eof)
return (ssize_t) bg;
+ acquire_attach_mutex (INFINITE);
push_process_state process_state (PID_TTYOU);
acquire_output_mutex (INFINITE);
@@ -3298,6 +3320,7 @@ fhandler_console::write (const void *vsrc, size_t len)
syscall_printf ("%ld = fhandler_console::write(...)", len);
+ release_attach_mutex ();
return len;
}
@@ -3469,12 +3492,13 @@ fhandler_console::create_invisible_console (HWINSTA horig)
function is currently only called at startup and during exec, it shouldn't
be a big deal. */
bool
-fhandler_console::create_invisible_console_workaround ()
+fhandler_console::create_invisible_console_workaround (bool force)
{
- if (!AttachConsole (-1))
+ /* If force is set, avoid to reattach to existing console. */
+ if (force || !AttachConsole (-1))
{
bool taskbar;
- DWORD err = GetLastError ();
+ DWORD err = force ? 0 : GetLastError ();
path_conv helper ("/bin/cygwin-console-helper.exe");
HANDLE hello = NULL;
HANDLE goodbye = NULL;
@@ -3559,10 +3583,12 @@ fhandler_console::free_console ()
}
bool
-fhandler_console::need_invisible ()
+fhandler_console::need_invisible (bool force)
{
BOOL b = false;
- if (exists ())
+ /* If force is set, forcibly create a new invisible console
+ even if a console device already exists. */
+ if (exists () && !force)
invisible_console = false;
else
{
@@ -3600,7 +3626,7 @@ fhandler_console::need_invisible ()
invisible_console = true;
}
else if (wincap.has_broken_alloc_console ())
- b = create_invisible_console_workaround ();
+ b = create_invisible_console_workaround (force);
else
b = create_invisible_console (h);
}
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 0c92f41d4..789bcdfdf 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -59,6 +59,39 @@ struct pipe_reply {
DWORD error;
};
+extern HANDLE attach_mutex; /* Defined in fhandler_console.cc */
+
+static DWORD
+get_console_process_id (DWORD pid, bool match)
+{
+ DWORD tmp;
+ DWORD num, num_req;
+ num = 1;
+ num_req = GetConsoleProcessList (&tmp, num);
+ DWORD *list;
+ while (true)
+ {
+ list = (DWORD *)
+ HeapAlloc (GetProcessHeap (), 0, num_req * sizeof (DWORD));
+ num = num_req;
+ num_req = GetConsoleProcessList (list, num);
+ if (num_req > num)
+ HeapFree (GetProcessHeap (), 0, list);
+ else
+ break;
+ }
+ num = num_req;
+
+ tmp = 0;
+ for (DWORD i=0; i<num; i++)
+ if ((match && list[i] == pid) || (!match && list[i] != pid))
+ /* Last one is the oldest. */
+ /* https://github.com/microsoft/terminal/issues/95 */
+ tmp = list[i];
+ HeapFree (GetProcessHeap (), 0, list);
+ return tmp;
+}
+
static bool isHybrid;
static void
@@ -289,7 +322,33 @@ fhandler_pty_master::accept_input ()
if (to_be_read_from_pcon ())
{
write_to = to_slave;
- UINT cp_to = GetConsoleCP ();
+
+ UINT cp_to;
+ pinfo pinfo_target = pinfo (get_ttyp ()->invisible_console_pid);
+ DWORD target_pid = 0;
+ if (pinfo_target)
+ target_pid = pinfo_target->dwProcessId;
+ pinfo pinfo_resume = pinfo (myself->ppid);
+ DWORD resume_pid;
+ if (pinfo_resume)
+ resume_pid = pinfo_resume->dwProcessId;
+ else
+ resume_pid = get_console_process_id (myself->dwProcessId, false);
+ if (target_pid && resume_pid)
+ {
+ /* Slave attaches to a different console than master.
+ Therefore reattach here. */
+ WaitForSingleObject (attach_mutex, INFINITE);
+ FreeConsole ();
+ AttachConsole (target_pid);
+ cp_to = GetConsoleCP ();
+ FreeConsole ();
+ AttachConsole (resume_pid);
+ ReleaseMutex (attach_mutex);
+ }
+ else
+ cp_to = GetConsoleCP ();
+
if (get_ttyp ()->term_code_page != cp_to)
{
static mbstate_t mbp;
@@ -659,7 +718,20 @@ fhandler_pty_slave::open (int flags, mode_t)
set_output_handle (to_master_local);
set_output_handle_cyg (to_master_cyg_local);
- fhandler_console::need_invisible ();
+ if (_major (myself->ctty) == DEV_CONS_MAJOR
+ && !(!pinfo (myself->ppid) && getenv ("ConEmuPID")))
+ /* This process is supposed to be a master process which is
+ running on console. Invisible console will be created in
+ primary slave process to prevent overriding code page
+ of root console by setup_locale(). */
+ /* ... except for ConEmu cygwin-connector in which this
+ code does not work as expected because it calls Win32
+ API directly rather than cygwin read()/write(). Due to
+ this behaviour, protection based on attach_mutex does
+ not take effect. */
+ get_ttyp ()->need_invisible_console = true;
+ else
+ fhandler_console::need_invisible ();
set_open_status ();
return 1;
@@ -1572,6 +1644,7 @@ fhandler_pty_master::close ()
}
release_output_mutex ();
master_fwd_thread->terminate_thread ();
+ CloseHandle (attach_mutex);
}
}
@@ -1847,6 +1920,7 @@ void
fhandler_pty_slave::fixup_after_exec ()
{
reset_switch_to_pcon ();
+ create_invisible_console ();
if (!close_on_exec ())
fixup_after_fork (NULL); /* No parent handle required. */
@@ -2135,7 +2209,32 @@ fhandler_pty_master::pty_master_fwd_thread ()
continue;
}
- UINT cp_from = GetConsoleOutputCP ();
+ UINT cp_from;
+ pinfo pinfo_target = pinfo (get_ttyp ()->invisible_console_pid);
+ DWORD target_pid = 0;
+ if (pinfo_target)
+ target_pid = pinfo_target->dwProcessId;
+ pinfo pinfo_resume = pinfo (myself->ppid);
+ DWORD resume_pid;
+ if (pinfo_resume)
+ resume_pid = pinfo_resume->dwProcessId;
+ else
+ resume_pid = get_console_process_id (myself->dwProcessId, false);
+ if (target_pid && resume_pid)
+ {
+ /* Slave attaches to a different console than master.
+ Therefore reattach here. */
+ WaitForSingleObject (attach_mutex, INFINITE);
+ FreeConsole ();
+ AttachConsole (target_pid);
+ cp_from = GetConsoleOutputCP ();
+ FreeConsole ();
+ AttachConsole (resume_pid);
+ ReleaseMutex (attach_mutex);
+ }
+ else
+ cp_from = GetConsoleOutputCP ();
+
if (get_ttyp ()->term_code_page != cp_from)
{
size_t nlen = NT_MAX_PATH;
@@ -2250,6 +2349,8 @@ fhandler_pty_master::setup ()
if (!(input_mutex = CreateMutex (&sa, FALSE, buf)))
goto err;
+ attach_mutex = CreateMutex (&sa, FALSE, NULL);
+
/* Create master control pipe which allows the master to duplicate
the pty pipe handles to processes which deserve it. */
__small_sprintf (buf, "\\\\.\\pipe\\cygwin-%S-pty%d-master-ctl",
@@ -2311,6 +2412,7 @@ err:
close_maybe (input_available_event);
close_maybe (output_mutex);
close_maybe (input_mutex);
+ close_maybe (attach_mutex);
close_maybe (from_master);
close_maybe (from_master_cyg);
close_maybe (to_master);
@@ -2757,3 +2859,21 @@ maybe_dumb:
get_ttyp ()->pcon_cap_checked = true;
return false;
}
+
+void
+fhandler_pty_slave::create_invisible_console ()
+{
+ if (get_ttyp ()->need_invisible_console)
+ {
+ /* Detach from console device and create new invisible console. */
+ FreeConsole();
+ fhandler_console::need_invisible (true);
+ get_ttyp ()->need_invisible_console = false;
+ get_ttyp ()->invisible_console_pid = myself->pid;
+ }
+ if (get_ttyp ()->invisible_console_pid
+ && !pinfo (get_ttyp ()->invisible_console_pid))
+ /* If primary slave process does not exist anymore,
+ this process becomes the primary. */
+ get_ttyp ()->invisible_console_pid = myself->pid;
+}
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index bf1b08057..42044ab53 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -648,6 +648,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
{
fhandler_pty_slave *ptys =
(fhandler_pty_slave *)(fhandler_base *) cfd;
+ ptys->create_invisible_console ();
ptys->setup_locale ();
}
}
diff --git a/winsup/cygwin/tty.cc b/winsup/cygwin/tty.cc
index d4b8d7651..c6e13f111 100644
--- a/winsup/cygwin/tty.cc
+++ b/winsup/cygwin/tty.cc
@@ -246,6 +246,8 @@ tty::init ()
has_csi6n = false;
has_set_title = false;
do_not_resize_pcon = false;
+ need_invisible_console = false;
+ invisible_console_pid = 0;
}
HANDLE
diff --git a/winsup/cygwin/tty.h b/winsup/cygwin/tty.h
index 2c1ac7f5d..a975aba45 100644
--- a/winsup/cygwin/tty.h
+++ b/winsup/cygwin/tty.h
@@ -105,6 +105,8 @@ private:
bool has_csi6n;
bool has_set_title;
bool do_not_resize_pcon;
+ bool need_invisible_console;
+ pid_t invisible_console_pid;
public:
HANDLE from_master () const { return _from_master; }
--
2.30.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 5/5] Cygwin: pty: Make master thread functions be static.
2021-01-15 8:32 [PATCH 0/5] Some revisions for pty and console code Takashi Yano
` (3 preceding siblings ...)
2021-01-15 8:32 ` [PATCH 4/5] Cygwin: pty: Prevent pty from changing code page of parent console Takashi Yano
@ 2021-01-15 8:32 ` Takashi Yano
4 siblings, 0 replies; 8+ messages in thread
From: Takashi Yano @ 2021-01-15 8:32 UTC (permalink / raw)
To: cygwin-patches
- The functions pty_master_thread() and pty_master_fwd_thread()
should be static (i.e. should not access class member) because
the instance is deleted if the master is dup()'ed and the first
master is closed. In this case, because the dup()'ed instance
still exists, these master threads are also still alive even
though the instance has been deleted. As a result, accesing
class members in these functions causes accessi violation.
Addresses:
https://cygwin.com/pipermail/cygwin-developers/2021-January/012030.html
---
winsup/cygwin/fhandler.h | 30 ++++++-
winsup/cygwin/fhandler_tty.cc | 154 ++++++++++++++++++++++------------
2 files changed, 128 insertions(+), 56 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index ffd19a590..d134b180c 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -2274,8 +2274,9 @@ class fhandler_pty_common: public fhandler_termios
void resize_pseudo_console (struct winsize *);
protected:
- BOOL process_opost_output (HANDLE h,
- const void *ptr, ssize_t& len, bool is_echo);
+ static BOOL process_opost_output (HANDLE h, const void *ptr, ssize_t& len,
+ bool is_echo, tty *ttyp,
+ bool is_nonblocking);
};
class fhandler_pty_slave: public fhandler_pty_common
@@ -2352,6 +2353,24 @@ class fhandler_pty_slave: public fhandler_pty_common
#define __ptsname(buf, unit) __small_sprintf ((buf), "/dev/pty%d", (unit))
class fhandler_pty_master: public fhandler_pty_common
{
+public:
+ /* Parameter set for the static function pty_master_thread() */
+ struct master_thread_param_t {
+ HANDLE from_master;
+ HANDLE from_master_cyg;
+ HANDLE to_master;
+ HANDLE to_master_cyg;
+ HANDLE master_ctl;
+ HANDLE input_available_event;
+ };
+ /* Parameter set for the static function pty_master_fwd_thread() */
+ struct master_fwd_thread_param_t {
+ HANDLE to_master_cyg;
+ HANDLE from_slave;
+ HANDLE output_mutex;
+ tty *ttyp;
+ };
+private:
int pktmode; // non-zero if pty in a packet mode.
HANDLE master_ctl; // Control socket for handle duplication
cygthread *master_thread; // Master control thread
@@ -2360,14 +2379,15 @@ class fhandler_pty_master: public fhandler_pty_common
DWORD dwProcessId; // Owner of master handles
HANDLE to_master_cyg, from_master_cyg;
cygthread *master_fwd_thread; // Master forwarding thread
+ HANDLE thread_param_copied_event;
public:
HANDLE get_echo_handle () const { return echo_r; }
/* Constructor */
fhandler_pty_master (int);
- DWORD pty_master_thread ();
- DWORD pty_master_fwd_thread ();
+ static DWORD pty_master_thread (const master_thread_param_t *p);
+ static DWORD pty_master_fwd_thread (const master_fwd_thread_param_t *p);
int process_slave_output (char *buf, size_t len, int pktmode_on);
void doecho (const void *str, DWORD len);
int accept_input ();
@@ -2410,6 +2430,8 @@ public:
return fh;
}
bool to_be_read_from_pcon (void);
+ void get_master_thread_param (master_thread_param_t *p);
+ void get_master_fwd_thread_param (master_fwd_thread_param_t *p);
};
class fhandler_dev_null: public fhandler_base
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index 789bcdfdf..e4993bf31 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -301,7 +301,8 @@ fhandler_pty_master::doecho (const void *str, DWORD len)
{
ssize_t towrite = len;
acquire_output_mutex (INFINITE);
- if (!process_opost_output (echo_w, str, towrite, true))
+ if (!process_opost_output (echo_w, str, towrite, true,
+ get_ttyp (), is_nonblocking ()))
termios_printf ("Write to echo pipe failed, %E");
release_output_mutex ();
}
@@ -874,7 +875,8 @@ fhandler_pty_slave::write (const void *ptr, size_t len)
reset_switch_to_pcon ();
acquire_output_mutex (INFINITE);
- if (!process_opost_output (get_output_handle_cyg (), ptr, towrite, false))
+ if (!process_opost_output (get_output_handle_cyg (), ptr, towrite, false,
+ get_ttyp (), is_nonblocking ()))
{
DWORD err = GetLastError ();
termios_printf ("WriteFile failed, %E");
@@ -1955,8 +1957,15 @@ fhandler_pty_slave::fixup_after_exec ()
calls to CallNamedPipe should have a big enough timeout value. For now this
is 500ms. Hope that's enough. */
+/* The function pty_master_thread() should be static because the instance
+ is deleted if the master is dup()'ed and the original is closed. In
+ this case, dup()'ed instance still exists, therefore, master thread
+ is also still alive even though the instance has been deleted. As a
+ result, accesing member variables in this function causes access
+ violation. */
+
DWORD
-fhandler_pty_master::pty_master_thread ()
+fhandler_pty_master::pty_master_thread (const master_thread_param_t *p)
{
bool exit = false;
GENERIC_MAPPING map = { EVENT_QUERY_STATE, EVENT_MODIFY_STATE, 0,
@@ -1970,7 +1979,7 @@ fhandler_pty_master::pty_master_thread ()
NTSTATUS status;
termios_printf ("Entered");
- while (!exit && (ConnectNamedPipe (master_ctl, NULL)
+ while (!exit && (ConnectNamedPipe (p->master_ctl, NULL)
|| GetLastError () == ERROR_PIPE_CONNECTED))
{
pipe_reply repl = { NULL, NULL, NULL, NULL, 0 };
@@ -1979,21 +1988,21 @@ fhandler_pty_master::pty_master_thread ()
ACCESS_MASK access = EVENT_MODIFY_STATE;
HANDLE client = NULL;
- if (!ReadFile (master_ctl, &req, sizeof req, &len, NULL))
+ if (!ReadFile (p->master_ctl, &req, sizeof req, &len, NULL))
{
termios_printf ("ReadFile, %E");
goto reply;
}
- if (!GetNamedPipeClientProcessId (master_ctl, &pid))
+ if (!GetNamedPipeClientProcessId (p->master_ctl, &pid))
pid = req.pid;
- if (get_object_sd (input_available_event, sd))
+ if (get_object_sd (p->input_available_event, sd))
{
termios_printf ("get_object_sd, %E");
goto reply;
}
cygheap->user.deimpersonate ();
deimp = true;
- if (!ImpersonateNamedPipeClient (master_ctl))
+ if (!ImpersonateNamedPipeClient (p->master_ctl))
{
termios_printf ("ImpersonateNamedPipeClient, %E");
goto reply;
@@ -2036,28 +2045,28 @@ fhandler_pty_master::pty_master_thread ()
termios_printf ("OpenProcess, %E");
goto reply;
}
- if (!DuplicateHandle (GetCurrentProcess (), from_master,
+ if (!DuplicateHandle (GetCurrentProcess (), p->from_master,
client, &repl.from_master,
0, TRUE, DUPLICATE_SAME_ACCESS))
{
termios_printf ("DuplicateHandle (from_master), %E");
goto reply;
}
- if (!DuplicateHandle (GetCurrentProcess (), from_master_cyg,
+ if (!DuplicateHandle (GetCurrentProcess (), p->from_master_cyg,
client, &repl.from_master_cyg,
0, TRUE, DUPLICATE_SAME_ACCESS))
{
termios_printf ("DuplicateHandle (from_master_cyg), %E");
goto reply;
}
- if (!DuplicateHandle (GetCurrentProcess (), to_master,
+ if (!DuplicateHandle (GetCurrentProcess (), p->to_master,
client, &repl.to_master,
0, TRUE, DUPLICATE_SAME_ACCESS))
{
termios_printf ("DuplicateHandle (to_master), %E");
goto reply;
}
- if (!DuplicateHandle (GetCurrentProcess (), to_master_cyg,
+ if (!DuplicateHandle (GetCurrentProcess (), p->to_master_cyg,
client, &repl.to_master_cyg,
0, TRUE, DUPLICATE_SAME_ACCESS))
{
@@ -2074,9 +2083,9 @@ reply:
sd.free ();
termios_printf ("Reply: from %p, to %p, error %u",
repl.from_master, repl.to_master, repl.error );
- if (!WriteFile (master_ctl, &repl, sizeof repl, &len, NULL))
+ if (!WriteFile (p->master_ctl, &repl, sizeof repl, &len, NULL))
termios_printf ("WriteFile, %E");
- if (!DisconnectNamedPipe (master_ctl))
+ if (!DisconnectNamedPipe (p->master_ctl))
termios_printf ("DisconnectNamedPipe, %E");
}
termios_printf ("Leaving");
@@ -2086,11 +2095,20 @@ reply:
static DWORD WINAPI
pty_master_thread (VOID *arg)
{
- return ((fhandler_pty_master *) arg)->pty_master_thread ();
+ fhandler_pty_master::master_thread_param_t p;
+ ((fhandler_pty_master *) arg)->get_master_thread_param (&p);
+ return fhandler_pty_master::pty_master_thread (&p);
}
+/* The function pty_master_fwd_thread() should be static because the
+ instance is deleted if the master is dup()'ed and the original is
+ closed. In this case, dup()'ed instance still exists, therefore,
+ master forwarding thread is also still alive even though the instance
+ has been deleted. As a result, accesing member variables in this
+ function causes access violation. */
+
DWORD
-fhandler_pty_master::pty_master_fwd_thread ()
+fhandler_pty_master::pty_master_fwd_thread (const master_fwd_thread_param_t *p)
{
DWORD rlen;
tmp_pathbuf tp;
@@ -2101,17 +2119,17 @@ fhandler_pty_master::pty_master_fwd_thread ()
termios_printf ("Started.");
for (;;)
{
- get_ttyp ()->pcon_last_time = GetTickCount ();
- if (!ReadFile (from_slave, outbuf, NT_MAX_PATH, &rlen, NULL))
+ p->ttyp->pcon_last_time = GetTickCount ();
+ if (!ReadFile (p->from_slave, outbuf, NT_MAX_PATH, &rlen, NULL))
{
termios_printf ("ReadFile for forwarding failed, %E");
break;
}
ssize_t wlen = rlen;
char *ptr = outbuf;
- if (get_ttyp ()->h_pseudo_console)
+ if (p->ttyp->h_pseudo_console)
{
- if (!get_ttyp ()->has_set_title)
+ if (!p->ttyp->has_set_title)
{
/* Remove Set title sequence */
char *p0, *p1;
@@ -2183,10 +2201,10 @@ fhandler_pty_master::pty_master_fwd_thread ()
else
state = 0;
- if (get_ttyp ()->term_code_page != CP_UTF8)
+ if (p->ttyp->term_code_page != CP_UTF8)
{
size_t nlen = NT_MAX_PATH;
- convert_mb_str (get_ttyp ()->term_code_page, mbbuf, &nlen,
+ convert_mb_str (p->ttyp->term_code_page, mbbuf, &nlen,
CP_UTF8, ptr, wlen, &mbp);
ptr = mbbuf;
@@ -2198,7 +2216,7 @@ fhandler_pty_master::pty_master_fwd_thread ()
DWORD written;
while (rlen>0)
{
- if (!WriteFile (to_master_cyg, ptr, wlen, &written, NULL))
+ if (!WriteFile (p->to_master_cyg, ptr, wlen, &written, NULL))
{
termios_printf ("WriteFile for forwarding failed, %E");
break;
@@ -2210,7 +2228,7 @@ fhandler_pty_master::pty_master_fwd_thread ()
}
UINT cp_from;
- pinfo pinfo_target = pinfo (get_ttyp ()->invisible_console_pid);
+ pinfo pinfo_target = pinfo (p->ttyp->invisible_console_pid);
DWORD target_pid = 0;
if (pinfo_target)
target_pid = pinfo_target->dwProcessId;
@@ -2235,20 +2253,21 @@ fhandler_pty_master::pty_master_fwd_thread ()
else
cp_from = GetConsoleOutputCP ();
- if (get_ttyp ()->term_code_page != cp_from)
+ if (p->ttyp->term_code_page != cp_from)
{
size_t nlen = NT_MAX_PATH;
- convert_mb_str (get_ttyp ()->term_code_page, mbbuf, &nlen,
+ convert_mb_str (p->ttyp->term_code_page, mbbuf, &nlen,
cp_from, ptr, wlen, &mbp);
ptr = mbbuf;
wlen = rlen = nlen;
}
- acquire_output_mutex (INFINITE);
+ WaitForSingleObject (p->output_mutex, INFINITE);
while (rlen>0)
{
- if (!process_opost_output (to_master_cyg, ptr, wlen, false))
+ if (!process_opost_output (p->to_master_cyg, ptr, wlen, false,
+ p->ttyp, false))
{
termios_printf ("WriteFile for forwarding failed, %E");
break;
@@ -2256,7 +2275,7 @@ fhandler_pty_master::pty_master_fwd_thread ()
ptr += wlen;
wlen = (rlen -= wlen);
}
- release_output_mutex ();
+ ReleaseMutex (p->output_mutex);
}
return 0;
}
@@ -2264,7 +2283,9 @@ fhandler_pty_master::pty_master_fwd_thread ()
static DWORD WINAPI
pty_master_fwd_thread (VOID *arg)
{
- return ((fhandler_pty_master *) arg)->pty_master_fwd_thread ();
+ fhandler_pty_master::master_fwd_thread_param_t p;
+ ((fhandler_pty_master *) arg)->get_master_fwd_thread_param (&p);
+ return fhandler_pty_master::pty_master_fwd_thread (&p);
}
bool
@@ -2312,6 +2333,15 @@ fhandler_pty_master::setup ()
goto err;
}
+ __small_sprintf (pipename, "pty%d-to-slave", unit);
+ res = fhandler_pipe::create (&sec_none, &from_master, &to_slave,
+ fhandler_pty_common::pipesize, pipename, 0);
+ if (res)
+ {
+ errstr = "input pipe";
+ goto err;
+ }
+
ProtectHandle1 (from_slave, from_pty);
__small_sprintf (pipename, "pty%d-echoloop", unit);
@@ -2366,27 +2396,23 @@ fhandler_pty_master::setup ()
errstr = "pty master control pipe";
goto err;
}
+
+ thread_param_copied_event = CreateEvent(NULL, FALSE, FALSE, NULL);
master_thread = new cygthread (::pty_master_thread, this, "ptym");
if (!master_thread)
{
errstr = "pty master control thread";
goto err;
}
+ WaitForSingleObject (thread_param_copied_event, INFINITE);
master_fwd_thread = new cygthread (::pty_master_fwd_thread, this, "ptymf");
if (!master_fwd_thread)
{
errstr = "pty master forwarding thread";
goto err;
}
-
- __small_sprintf (pipename, "pty%d-to-slave", unit);
- res = fhandler_pipe::create (&sec_none, &from_master, &to_slave,
- fhandler_pty_common::pipesize, pipename, 0);
- if (res)
- {
- errstr = "input pipe";
- goto err;
- }
+ WaitForSingleObject (thread_param_copied_event, INFINITE);
+ CloseHandle (thread_param_copied_event);
t.set_from_master (from_master);
t.set_from_master_cyg (from_master_cyg);
@@ -2463,7 +2489,9 @@ fhandler_pty_master::fixup_after_exec ()
}
BOOL
-fhandler_pty_common::process_opost_output (HANDLE h, const void *ptr, ssize_t& len, bool is_echo)
+fhandler_pty_common::process_opost_output (HANDLE h, const void *ptr,
+ ssize_t& len, bool is_echo,
+ tty *ttyp, bool is_nonblocking)
{
ssize_t towrite = len;
BOOL res = TRUE;
@@ -2471,7 +2499,7 @@ fhandler_pty_common::process_opost_output (HANDLE h, const void *ptr, ssize_t& l
{
if (!is_echo)
{
- if (tc ()->output_stopped && is_nonblocking ())
+ if (ttyp->output_stopped && is_nonblocking)
{
if (towrite < len)
break;
@@ -2482,11 +2510,11 @@ fhandler_pty_common::process_opost_output (HANDLE h, const void *ptr, ssize_t& l
return TRUE;
}
}
- while (tc ()->output_stopped)
+ while (ttyp->output_stopped)
cygwait (10);
}
- if (!(get_ttyp ()->ti.c_oflag & OPOST)) // raw output mode
+ if (!(ttyp->ti.c_oflag & OPOST)) // raw output mode
{
DWORD n = MIN (OUT_BUFFER_SIZE, towrite);
res = WriteFile (h, ptr, n, &n, NULL);
@@ -2506,13 +2534,13 @@ fhandler_pty_common::process_opost_output (HANDLE h, const void *ptr, ssize_t& l
switch (buf[rc])
{
case '\r':
- if ((get_ttyp ()->ti.c_oflag & ONOCR)
- && get_ttyp ()->column == 0)
+ if ((ttyp->ti.c_oflag & ONOCR)
+ && ttyp->column == 0)
{
rc++;
continue;
}
- if (get_ttyp ()->ti.c_oflag & OCRNL)
+ if (ttyp->ti.c_oflag & OCRNL)
{
outbuf[n++] = '\n';
rc++;
@@ -2520,22 +2548,22 @@ fhandler_pty_common::process_opost_output (HANDLE h, const void *ptr, ssize_t& l
else
{
outbuf[n++] = buf[rc++];
- get_ttyp ()->column = 0;
+ ttyp->column = 0;
}
break;
case '\n':
- if (get_ttyp ()->ti.c_oflag & ONLCR)
+ if (ttyp->ti.c_oflag & ONLCR)
{
outbuf[n++] = '\r';
- get_ttyp ()->column = 0;
+ ttyp->column = 0;
}
- if (get_ttyp ()->ti.c_oflag & ONLRET)
- get_ttyp ()->column = 0;
+ if (ttyp->ti.c_oflag & ONLRET)
+ ttyp->column = 0;
outbuf[n++] = buf[rc++];
break;
default:
outbuf[n++] = buf[rc++];
- get_ttyp ()->column++;
+ ttyp->column++;
break;
}
}
@@ -2877,3 +2905,25 @@ fhandler_pty_slave::create_invisible_console ()
this process becomes the primary. */
get_ttyp ()->invisible_console_pid = myself->pid;
}
+
+void
+fhandler_pty_master::get_master_thread_param (master_thread_param_t *p)
+{
+ p->from_master = from_master;
+ p->from_master_cyg = from_master_cyg;
+ p->to_master = to_master;
+ p->to_master_cyg = to_master_cyg;
+ p->master_ctl = master_ctl;
+ p->input_available_event = input_available_event;
+ SetEvent (thread_param_copied_event);
+}
+
+void
+fhandler_pty_master::get_master_fwd_thread_param (master_fwd_thread_param_t *p)
+{
+ p->to_master_cyg = to_master_cyg;
+ p->from_slave = from_slave;
+ p->output_mutex = output_mutex;
+ p->ttyp = get_ttyp ();
+ SetEvent (thread_param_copied_event);
+}
--
2.30.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/5] Cygwin: pty: Prevent pty from changing code page of parent console.
2021-01-15 8:32 ` [PATCH 4/5] Cygwin: pty: Prevent pty from changing code page of parent console Takashi Yano
@ 2021-01-18 10:23 ` Corinna Vinschen
2021-01-18 11:23 ` Takashi Yano
0 siblings, 1 reply; 8+ messages in thread
From: Corinna Vinschen @ 2021-01-18 10:23 UTC (permalink / raw)
To: cygwin-patches
Hi Takashi,
I'm going to push patches 1 - 3. In terms of patch 4 I have a few
questions:
On Jan 15 17:32, Takashi Yano via Cygwin-patches wrote:
> @@ -2185,7 +2185,7 @@ private:
> bool send_winch_maybe ();
> void setup ();
> bool set_unit ();
> - static bool need_invisible ();
> + static bool need_invisible (bool force=false);
Please add spaces, i. e., force = false
> +static DWORD
> +get_console_process_id (DWORD pid, bool match)
> +{
> + DWORD tmp;
> + DWORD num, num_req;
> + num = 1;
> + num_req = GetConsoleProcessList (&tmp, num);
> + DWORD *list;
So, assuming num_req is 1 after the call, shouldn't that skip the
rest of the code?
> + while (true)
> + {
> + list = (DWORD *)
> + HeapAlloc (GetProcessHeap (), 0, num_req * sizeof (DWORD));
> + num = num_req;
> + num_req = GetConsoleProcessList (list, num);
> + if (num_req > num)
> + HeapFree (GetProcessHeap (), 0, list);
> + else
> + break;
> + }
> + num = num_req;
> +
> + tmp = 0;
> + for (DWORD i=0; i<num; i++)
> + if ((match && list[i] == pid) || (!match && list[i] != pid))
> + /* Last one is the oldest. */
> + /* https://github.com/microsoft/terminal/issues/95 */
Given that, wouldn't it make more sense to count backwards, from
num - 1 to 0, from a performance perspective?
Thanks,
Corinna
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 4/5] Cygwin: pty: Prevent pty from changing code page of parent console.
2021-01-18 10:23 ` Corinna Vinschen
@ 2021-01-18 11:23 ` Takashi Yano
0 siblings, 0 replies; 8+ messages in thread
From: Takashi Yano @ 2021-01-18 11:23 UTC (permalink / raw)
To: cygwin-patches
Hi Corinna,
On Mon, 18 Jan 2021 11:23:40 +0100
Corinna Vinschen wrote:
> I'm going to push patches 1 - 3. In terms of patch 4 I have a few
> questions:
>
> On Jan 15 17:32, Takashi Yano via Cygwin-patches wrote:
> > @@ -2185,7 +2185,7 @@ private:
> > bool send_winch_maybe ();
> > void setup ();
> > bool set_unit ();
> > - static bool need_invisible ();
> > + static bool need_invisible (bool force=false);
>
> Please add spaces, i. e., force = false
>
> > +static DWORD
> > +get_console_process_id (DWORD pid, bool match)
> > +{
> > + DWORD tmp;
> > + DWORD num, num_req;
> > + num = 1;
> > + num_req = GetConsoleProcessList (&tmp, num);
> > + DWORD *list;
>
> So, assuming num_req is 1 after the call, shouldn't that skip the
> rest of the code?
>
> > + while (true)
> > + {
> > + list = (DWORD *)
> > + HeapAlloc (GetProcessHeap (), 0, num_req * sizeof (DWORD));
> > + num = num_req;
> > + num_req = GetConsoleProcessList (list, num);
> > + if (num_req > num)
> > + HeapFree (GetProcessHeap (), 0, list);
> > + else
> > + break;
> > + }
> > + num = num_req;
> > +
> > + tmp = 0;
> > + for (DWORD i=0; i<num; i++)
> > + if ((match && list[i] == pid) || (!match && list[i] != pid))
> > + /* Last one is the oldest. */
> > + /* https://github.com/microsoft/terminal/issues/95 */
>
> Given that, wouldn't it make more sense to count backwards, from
> num - 1 to 0, from a performance perspective?
Thanks for the advice. I will submit the revised patch.
--
Takashi Yano <takashi.yano@nifty.ne.jp>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-01-18 11:24 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-15 8:32 [PATCH 0/5] Some revisions for pty and console code Takashi Yano
2021-01-15 8:32 ` [PATCH 1/5] Cygwin: pty: Add workaround for rlwrap 0.40 or later Takashi Yano
2021-01-15 8:32 ` [PATCH 2/5] Cygwin: console: Revise the code to switch xterm mode Takashi Yano
2021-01-15 8:32 ` [PATCH 3/5] Cygwin: pty: Make close_pseudoconsole() be a static member function Takashi Yano
2021-01-15 8:32 ` [PATCH 4/5] Cygwin: pty: Prevent pty from changing code page of parent console Takashi Yano
2021-01-18 10:23 ` Corinna Vinschen
2021-01-18 11:23 ` Takashi Yano
2021-01-15 8:32 ` [PATCH 5/5] Cygwin: pty: Make master thread functions be static Takashi Yano
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).