nhyatt/putty-source
1
0
Fork 0
mirror of https://git.tartarus.org/simon/putty.git synced 2025-01-09 17:38:00 +00:00
Code Issues Projects Releases Wiki Activity

Suspend terminal output while a window resize is pending.

Browse Source 
This is the payoff from the last few commits of refactoring. It fixes
the following race-condition bug in terminal application redraw:

 * server sends a window-resizing escape sequence
 * terminal requests a window resize from the front end
 * server sends further escape sequences to perform a redraw of some
   full-screen application, which assume that the window resize has
   occurred and the window is already its new size
 * terminal processes all those sequences in the context of the old
   window size, while the front end is still thinking
 * window resize completes in the front end and term_size() tells the
   terminal it now has its new size, but it's too late, the screen
   redraw has made a total mess.

(Perhaps the server might even send its window resize + followup
redraw all in one SSH packet, so that it's all queued in term->inbuf
in one go.)

As far as I can see, handling of this case has been broken more or
less forever in the GTK frontend (where window resizes are inherently
asynchronous due to the way X11 works, and we've never done anything
to compensate for that). On Windows, where window size is changed via
SetWindowPos which is synchronous, it used to work, but broke in
commit d74308e90e (i.e. between 0.74 and 0.75), which made all
the ancillary window updates run on the same delayed-action timer as
ordinary text display.

So, it's time to fix it, and I think now I should be able to fix it in
GTK as well as on Windows.

Now, as soon as we've set the term->win_resize_pending flag (in
response to a resize escape sequence), the next return to the top of
the main loop in term_out will terminate output processing early,
leaving any further terminal data still in the term->inbuf bufchain.
Once we get a term_size() callback from the front end telling us our
new size, we reset term->win_resize_pending, which unblocks output
processing again, and we also queue a toplevel callback to have
another try at term_out() so that it will be unblocked promptly.

To implement this I've changed term->win_resize_pending from a bool
into a three-state enumeration, so that we can tell the difference
between 'pending' in the sense of not yet having sent our resize
request to the frontend, and in the sense of waiting for the frontend
to reply. That way, a window resize from the GUI user at least won't
be mistaken for the response to our resize request if it arrives in
the former state. (It can still be mistaken for one in the latter
case, but if the user is resizing the window at the same time as the
server-side application is doing critically size-dependent redrawing,
I don't think there can be any reasonable expectation of nothing going
wrong.)

As mentioned in the previous commit, some failure modes under X11 (in
particular the window manager process getting wedged in some way) can
result in no response being received to a ConfigureWindow request. In
that situation, it seems to me that we really _shouldn't_ sit there
waiting forever - perhaps it's technically the WM's fault and not
ours, but what kind of X window are you most likely to want to use to
do emergency WM repair? A terminal window, of course, so it would be
exceptionally unhelpful to make any terminal window stop working
completely in this situation! Hence, there's a fallback timeout in
terminal.c, so that if we don't receive a response in _too_ long,
we'll assume one is not forthcoming, and resume processing terminal
data at the old window size. The fallback timeout is set to 5 seconds,
following existing practice in libXt (DEFAULT_WM_TIMEOUT).
This commit is contained in:
Simon Tatham 2021-12-19 10:37:02 +00:00
parent 19b12ee56c
commit 420fe75552
2 changed files with 101 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
terminal/terminal.c
View File

@ -100,6 +100,7 @@ static void scroll(Terminal *, int, int, int, bool);
static void parse_optionalrgb(optionalrgb *out, unsigned *values);
static void term_added_data(Terminal *term);
static void term_update_raw_mouse_mode(Terminal *term);
static void term_out_cb(void *);
static termline *newtermline(Terminal *term, int cols, bool bce)
{
@ -1219,6 +1220,12 @@ static void term_timer(void *ctx, unsigned long now)
if (term->window_update_pending)
term_update_callback(term);
if (term->win_resize_pending == WIN_RESIZE_AWAIT_REPLY &&
now == term->win_resize_timeout) {
term->win_resize_pending = WIN_RESIZE_NO;
queue_toplevel_callback(term_out_cb, term);
}
}
static void term_update_callback(void *ctx)
@ -1415,10 +1422,12 @@ void term_update(Terminal *term)
term->win_move_pending_y);
term->win_move_pending = false;
}
if (term->win_resize_pending) {
if (term->win_resize_pending == WIN_RESIZE_NEED_SEND) {
term->win_resize_pending = WIN_RESIZE_AWAIT_REPLY;
win_request_resize(term->win, term->win_resize_pending_w,
term->win_resize_pending_h);
term->win_resize_pending = false;
term->win_resize_timeout = schedule_timer(
WIN_RESIZE_TIMEOUT, term_timer, term);
}
if (term->win_zorder_pending) {
win_set_zorder(term->win, term->win_zorder_top);
@ -2043,7 +2052,7 @@ Terminal *term_init(Conf *myconf, struct unicode_data *ucsdata, TermWin *win)
term->winpixsize_x = term->winpixsize_y = 0;
term->win_move_pending = false;
term->win_resize_pending = false;
term->win_resize_pending = WIN_RESIZE_NO;
term->win_zorder_pending = false;
term->win_minimise_pending = false;
term->win_maximise_pending = false;
@ -2142,6 +2151,14 @@ void term_size(Terminal *term, int newrows, int newcols, int newsavelines)
int sblen;
int save_alt_which = term->alt_which;
/* If we were holding buffered terminal data because we were
* waiting for confirmation of a resize, queue a callback to start
* processing it again. */
if (term->win_resize_pending == WIN_RESIZE_AWAIT_REPLY) {
term->win_resize_pending = WIN_RESIZE_NO;
queue_toplevel_callback(term_out_cb, term);
}
if (newrows == term->rows && newcols == term->cols &&
newsavelines == term->savelines)
return; /* nothing to do */
@ -2977,7 +2994,7 @@ static void term_request_resize(Terminal *term, int cols, int rows)
if (term->cols == cols && term->rows == rows)
return; /* don't need to do anything */
term->win_resize_pending = true;
term->win_resize_pending = WIN_RESIZE_NEED_SEND;
term->win_resize_pending_w = cols;
term->win_resize_pending_h = rows;
term_schedule_update(term);
@ -3654,6 +3671,29 @@ static void term_out(Terminal *term)
c = unget;
unget = -1;
} else {
/*
* If we're waiting for a terminal resize triggered by an
* escape sequence, we defer processing the terminal
* output until we receive acknowledgment from the front
* end that the resize has happened, so that further
* output will be processed in the context of the new
* size.
*
* This test goes inside the main while-loop, so that we
* exit early if we encounter a resize escape sequence
* part way through term->inbuf.
*
* It's also in the branch of this if statement that
* doesn't deal with a character left in 'unget' by the
* previous loop iteration, because if we break out of
* this loop with an ungot character still pending, we'll
* lose it. (And in any case, if the previous thing that
* happened was a truncated UTF-8 sequence, then it won't
* have scheduled a pending resize.)
*/
if (term->win_resize_pending != WIN_RESIZE_NO)
break;
if (nchars_got == nchars_used) {
/* Delete the previous chunk from the bufchain */
bufchain_consume(&term->inbuf, nchars_used);
@ -5554,6 +5594,12 @@ static void term_out(Terminal *term)
logflush(term->logctx);
}
/* Wrapper on term_out with the right prototype to be a toplevel callback */
void term_out_cb(void *ctx)
{
term_out((Terminal *)ctx);
}
/*
* Small subroutine to parse three consecutive escape-sequence
* arguments representing a true-colour RGB triple into an

53
terminal/terminal.h
View File

@ -381,8 +381,6 @@ struct terminal_tag {
*/
bool win_move_pending;
int win_move_pending_x, win_move_pending_y;
bool win_resize_pending;
int win_resize_pending_w, win_resize_pending_h;
bool win_zorder_pending;
bool win_zorder_top;
bool win_minimise_pending;
@ -396,6 +394,57 @@ struct terminal_tag {
bool win_scrollbar_update_pending;
bool win_palette_pending;
unsigned win_palette_pending_min, win_palette_pending_limit;
/*
* Unlike the rest of the above 'pending' flags, the one for
* window resizing has to be more complicated, because it's very
* likely that a server sending a window-resize escape sequence is
* going to follow it up immediately with further terminal output
* that draws a full-screen application expecting the terminal to
* be the new size.
*
* So, once we've requested a window resize from the TermWin, we
* have to stop processing terminal data until we get back the
* notification that our window really has changed size (or until
* we find out that it's not going to).
*
* Hence, window resizes go through a small state machine with two
* different kinds of 'pending'. NEED_SEND is the state where
* we've received an escape sequence asking for a new size but not
* yet sent it to the TermWin via win_request_resize; AWAIT_REPLY
* is the state where we've sent it to the TermWin and are
* expecting a call back to term_size().
*
* So _both_ of those 'pending' states inhibit terminal output
* processing.
*
* (Hence, once we're in either state, we should never handle
* another resize sequence, so the only possible path through this
* state machine is to get all the way back to the ground state
* before doing anything else interesting.)
*/
enum {
WIN_RESIZE_NO, WIN_RESIZE_NEED_SEND, WIN_RESIZE_AWAIT_REPLY
} win_resize_pending;
int win_resize_pending_w, win_resize_pending_h;
/*
* Not every frontend / TermWin implementation can be relied on
* 100% to reply to a resize request in a timely manner. (In X11
* it's all asynchronous and goes via the window manager, and if
* your window manager is seriously unwell, you'd rather not have
* terminal windows start becoming unusable as a knock-on effect,
* since those are just the thing you might need to use for
* emergency WM maintenance!) So when we enter AWAIT_REPLY status,
* we also set a 5-second timer, after which we'll regretfully
* conclude that a resize is probably not going to happen after
* all.
*
* However, in non-emergency cases, the plan is that this
* shouldn't be needed, for one reason or another.
*/
long win_resize_timeout;
#define WIN_RESIZE_TIMEOUT (TICKSPERSEC*5)
};
static inline bool in_utf(Terminal *term)