From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2210) id C067538708A8; Tue, 9 Jun 2020 11:01:54 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C067538708A8 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Ken Brown To: cygwin-cvs@sourceware.org Subject: [newlib-cygwin] Cygwin: pty: Fix screen distortion after less for native apps again. X-Act-Checkin: newlib-cygwin X-Git-Author: Takashi Yano via Cygwin-patches X-Git-Refname: refs/heads/master X-Git-Oldrev: e6ce6f1430eee503a388cf1fc7fe6634de09fb0f X-Git-Newrev: 8014dc709922a0d9934540d79257fe899544046f Message-Id: <20200609110154.C067538708A8@sourceware.org> Date: Tue, 9 Jun 2020 11:01:54 +0000 (GMT) X-BeenThere: cygwin-cvs@cygwin.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Cygwin core component git logs List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Jun 2020 11:01:54 -0000 https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=8014dc709922a0d9934540d79257fe899544046f commit 8014dc709922a0d9934540d79257fe899544046f Author: Takashi Yano via Cygwin-patches Date: Thu Jun 4 10:43:19 2020 +0900 Cygwin: pty: Fix screen distortion after less for native apps again. - Commit c4b060e3fe3bed05b3a69ccbcc20993ad85e163d seems to be not enough. Moreover, it does not work as expected at all in Win10 1809. This patch essentially reverts that commit and add another fix. After all, the cause of the problem was a race issue in switch_to_pcon_out flag. That is, this flag is set when native app starts, however, it is delayed by wait_pcon_fwd(). Since the flag is not set yet when less starts, the data which should go into the output_handle accidentally goes into output_handle_cyg. This patch fixes the problem more essentially for the cause of the problem than previous one. Diff: --- winsup/cygwin/fhandler.h | 1 - winsup/cygwin/fhandler_tty.cc | 49 +++++++++++++------------------------------ winsup/cygwin/tty.cc | 7 ++++++- winsup/cygwin/tty.h | 1 - 4 files changed, 21 insertions(+), 37 deletions(-) diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h index 4035c7e56..c6ce6d8e1 100644 --- a/winsup/cygwin/fhandler.h +++ b/winsup/cygwin/fhandler.h @@ -2354,7 +2354,6 @@ class fhandler_pty_slave: public fhandler_pty_common void setup_locale (void); void set_freeconsole_on_close (bool val); void trigger_redraw_screen (void); - void wait_pcon_fwd (void); void pull_pcon_input (void); void update_pcon_input_state (bool need_lock); }; diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc index bcc7648f3..126249d9f 100644 --- a/winsup/cygwin/fhandler_tty.cc +++ b/winsup/cygwin/fhandler_tty.cc @@ -1277,6 +1277,7 @@ fhandler_pty_slave::set_switch_to_pcon (int fd_set) { if (fd < 0) fd = fd_set; + acquire_output_mutex (INFINITE); if (fd == 0 && !get_ttyp ()->switch_to_pcon_in) { pull_pcon_input (); @@ -1286,13 +1287,13 @@ fhandler_pty_slave::set_switch_to_pcon (int fd_set) get_ttyp ()->switch_to_pcon_in = true; if (isHybrid && !get_ttyp ()->switch_to_pcon_out) { - wait_pcon_fwd (); + get_ttyp ()->wait_pcon_fwd (); get_ttyp ()->switch_to_pcon_out = true; } } else if ((fd == 1 || fd == 2) && !get_ttyp ()->switch_to_pcon_out) { - wait_pcon_fwd (); + get_ttyp ()->wait_pcon_fwd (); if (get_ttyp ()->pcon_pid == 0 || !pinfo (get_ttyp ()->pcon_pid)) get_ttyp ()->pcon_pid = myself->pid; @@ -1300,6 +1301,7 @@ fhandler_pty_slave::set_switch_to_pcon (int fd_set) if (isHybrid) get_ttyp ()->switch_to_pcon_in = true; } + release_output_mutex (); } void @@ -1314,12 +1316,14 @@ fhandler_pty_slave::reset_switch_to_pcon (void) return; if (do_not_reset_switch_to_pcon) return; + acquire_output_mutex (INFINITE); if (get_ttyp ()->switch_to_pcon_out) /* Wait for pty_master_fwd_thread() */ - wait_pcon_fwd (); + get_ttyp ()->wait_pcon_fwd (); get_ttyp ()->pcon_pid = 0; get_ttyp ()->switch_to_pcon_in = false; get_ttyp ()->switch_to_pcon_out = false; + release_output_mutex (); init_console_handler (true); } @@ -1372,7 +1376,7 @@ fhandler_pty_slave::push_to_pcon_screenbuffer (const char *ptr, size_t len, p0 = (char *) memmem (p1, nlen - (p1-buf), "\033[?1049h", 8); if (p0) { - p0 += 8; + //p0 += 8; get_ttyp ()->screen_alternated = true; if (get_ttyp ()->switch_to_pcon_out) do_not_reset_switch_to_pcon = true; @@ -1384,7 +1388,7 @@ fhandler_pty_slave::push_to_pcon_screenbuffer (const char *ptr, size_t len, p1 = (char *) memmem (p0, nlen - (p0-buf), "\033[?1049l", 8); if (p1) { - //p1 += 8; + p1 += 8; get_ttyp ()->screen_alternated = false; do_not_reset_switch_to_pcon = false; memmove (p0, p1, buf+nlen - p1); @@ -1504,8 +1508,9 @@ fhandler_pty_slave::write (const void *ptr, size_t len) reset_switch_to_pcon (); - bool output_to_pcon = - get_ttyp ()->switch_to_pcon_out && !get_ttyp ()->screen_alternated; + acquire_output_mutex (INFINITE); + bool output_to_pcon = get_ttyp ()->switch_to_pcon_out; + release_output_mutex (); UINT target_code_page = output_to_pcon ? GetConsoleOutputCP () : get_ttyp ()->term_code_page; @@ -2420,8 +2425,6 @@ fhandler_pty_master::close () } release_output_mutex (); master_fwd_thread->terminate_thread (); - CloseHandle (get_ttyp ()->fwd_done); - get_ttyp ()->fwd_done = NULL; } } @@ -2903,17 +2906,6 @@ fhandler_pty_slave::set_freeconsole_on_close (bool val) freeconsole_on_close = val; } -void -fhandler_pty_slave::wait_pcon_fwd (void) -{ - acquire_output_mutex (INFINITE); - get_ttyp ()->pcon_last_time = GetTickCount (); - ResetEvent (get_ttyp ()->fwd_done); - release_output_mutex (); - while (get_ttyp ()->fwd_done - && cygwait (get_ttyp ()->fwd_done, 1) == WAIT_TIMEOUT); -} - void fhandler_pty_slave::trigger_redraw_screen (void) { @@ -2967,12 +2959,14 @@ fhandler_pty_slave::fixup_after_attach (bool native_maybe, int fd_set) { DWORD mode = ENABLE_PROCESSED_OUTPUT | ENABLE_WRAP_AT_EOL_OUTPUT; SetConsoleMode (get_output_handle (), mode); + acquire_output_mutex (INFINITE); if (!get_ttyp ()->switch_to_pcon_out) - wait_pcon_fwd (); + get_ttyp ()->wait_pcon_fwd (); if (get_ttyp ()->pcon_pid == 0 || !pinfo (get_ttyp ()->pcon_pid)) get_ttyp ()->pcon_pid = myself->pid; get_ttyp ()->switch_to_pcon_out = true; + release_output_mutex (); if (get_ttyp ()->need_redraw_screen) trigger_redraw_screen (); @@ -3258,19 +3252,9 @@ fhandler_pty_master::pty_master_fwd_thread () { if (get_pseudo_console ()) { - /* The forwarding in pseudo console sometimes stops for - 16-32 msec even if it already has data to transfer. - If the time without transfer exceeds 32 msec, the - forwarding is supposed to be finished. */ - const int sleep_in_pcon = 16; - const int time_to_wait = sleep_in_pcon * 2 + 1/* margine */; get_ttyp ()->pcon_last_time = GetTickCount (); while (::bytes_available (rlen, from_slave) && rlen == 0) { - acquire_output_mutex (INFINITE); - if (GetTickCount () - get_ttyp ()->pcon_last_time > time_to_wait) - SetEvent (get_ttyp ()->fwd_done); - release_output_mutex (); /* Forcibly transfer input if it is requested by slave. This happens when input data should be transfered from the input pipe for cygwin apps to the input pipe @@ -3342,7 +3326,6 @@ fhandler_pty_master::pty_master_fwd_thread () /* OPOST processing was already done in pseudo console, so just write it to to_master_cyg. */ DWORD written; - acquire_output_mutex (INFINITE); while (rlen>0) { if (!WriteFile (to_master_cyg, ptr, wlen, &written, NULL)) @@ -3353,7 +3336,6 @@ fhandler_pty_master::pty_master_fwd_thread () ptr += written; wlen = (rlen -= written); } - release_output_mutex (); mb_str_free (buf); continue; } @@ -3695,7 +3677,6 @@ fhandler_pty_master::setup () errstr = "pty master forwarding thread"; goto err; } - get_ttyp ()->fwd_done = CreateEvent (&sec_none, true, false, NULL); t.winsize.ws_col = 80; t.winsize.ws_row = 25; diff --git a/winsup/cygwin/tty.cc b/winsup/cygwin/tty.cc index efdae4697..4cb68f776 100644 --- a/winsup/cygwin/tty.cc +++ b/winsup/cygwin/tty.cc @@ -244,7 +244,6 @@ tty::init () pcon_pid = 0; term_code_page = 0; need_redraw_screen = true; - fwd_done = NULL; pcon_last_time = 0; pcon_in_empty = true; req_transfer_input_to_pcon = false; @@ -307,6 +306,12 @@ tty::set_switch_to_pcon_out (bool v) void tty::wait_pcon_fwd (void) { + /* The forwarding in pseudo console sometimes stops for + 16-32 msec even if it already has data to transfer. + If the time without transfer exceeds 32 msec, the + forwarding is supposed to be finished. pcon_last_time + is reset to GetTickCount() in pty master forwarding + thread when the last data is transfered. */ const int sleep_in_pcon = 16; const int time_to_wait = sleep_in_pcon * 2 + 1/* margine */; pcon_last_time = GetTickCount (); diff --git a/winsup/cygwin/tty.h b/winsup/cygwin/tty.h index 7d6fc8fef..920e32b16 100644 --- a/winsup/cygwin/tty.h +++ b/winsup/cygwin/tty.h @@ -105,7 +105,6 @@ private: pid_t pcon_pid; UINT term_code_page; bool need_redraw_screen; - HANDLE fwd_done; DWORD pcon_last_time; bool pcon_in_empty; bool req_transfer_input_to_pcon;