From 90dd2b1473cf19a6467838dbae078eea4bdc3c61 Mon Sep 17 00:00:00 2001 From: Florian Larysch Date: Wed, 29 Nov 2017 18:43:02 +0100 Subject: tui: never flush buffers in midst of unibilium output e83845285 fixed an issue where long (true color) escape sequences got interrupted by the cursor visibility toggling caused by buffer flushes. cdfaecb25 introduces a new issue which causes similar problems: While the old buffer flushing code appended the cursor visibility escapes to the buffer before/after flushing, the new code effectively prepends the sequences. Assume the following sequence of events occurs: - A long escape code is issued using unibi_out when the buffer is almost full - out() gets called for a prefix of that escape code, causing the buffer to fill up - flush_buf(ui, false) is called and (correctly) does not insert any cursor toggling escapes - The rest of the escape code is written into the now empty buffer - At some later point, some other part of nvim calls flush_buf(ui, true), which then toggles the cursor, corrupting the escape code This could possibly also be fixed by tracking the state of the buffer (i.e. does it contain a partially output escape code?), but this seems fragile in the same way e83845285 turned out to be. The root cause for all these problems is the mismatch between nvim's (implicit) assumption that the buffer is flushable at any point in time and the non-atomicity of unibilium's character based callback interface. The proper fix (without modifying unibilium) is to ensure nvim's assumption about the buffer state holds at all times. To that end, add a "cork" flag which ensures one unibi_out-call never splits its output across a buffer flush; if an escape code does not fit into the current buffer, flush it without any part of the escape code in it and insert the whole escape code in the emptied buffer. This is a little more complex because it modifies the buffer in place rather than printing into another buffer, checking the remaining space in the terminal buffer and then memcpy'ing it. --- src/nvim/tui/tui.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/nvim/tui/tui.c b/src/nvim/tui/tui.c index 2436295ad4..d07eb9a923 100644 --- a/src/nvim/tui/tui.c +++ b/src/nvim/tui/tui.c @@ -96,6 +96,7 @@ typedef struct { bool immediate_wrap_after_last_column; bool mouse_enabled; bool busy, is_invisible; + bool cork, overflow; cursorentry_T cursor_shapes[SHAPE_IDX_COUNT]; HlAttrs print_attrs; bool default_attr; @@ -200,6 +201,8 @@ static void terminfo_start(UI *ui) data->default_attr = false; data->is_invisible = true; data->busy = false; + data->cork = false; + data->overflow = false; data->showing_mode = SHAPE_IDX_N; data->unibi_ext.enable_mouse = -1; data->unibi_ext.disable_mouse = -1; @@ -1219,8 +1222,18 @@ static void unibi_goto(UI *ui, int row, int col) } \ if (str) { \ unibi_var_t vars[26 + 26]; \ + size_t orig_pos = data->bufpos; \ + \ memset(&vars, 0, sizeof(vars)); \ + data->cork = true; \ +retry: \ unibi_format(vars, vars + 26, str, data->params, out, ui, NULL, NULL); \ + if (data->overflow) { \ + data->bufpos = orig_pos; \ + flush_buf(ui, true); \ + goto retry; \ + } \ + data->cork = false; \ } \ } while (0) static void unibi_out(UI *ui, int unibi_index) @@ -1239,8 +1252,17 @@ static void out(void *ctx, const char *str, size_t len) TUIData *data = ui->data; size_t available = sizeof(data->buf) - data->bufpos; + if (data->cork && data->overflow) { + return; + } + if (len > available) { - flush_buf(ui, false); + if (data->cork) { + data->overflow = true; + return; + } else { + flush_buf(ui, true); + } } memcpy(data->buf + data->bufpos, str, len); @@ -1696,6 +1718,7 @@ static void flush_buf(UI *ui, bool toggle_cursor) bufs, (unsigned)(bufp - bufs), NULL); uv_run(&data->write_loop, UV_RUN_DEFAULT); data->bufpos = 0; + data->overflow = false; } #if TERMKEY_VERSION_MAJOR > 0 || TERMKEY_VERSION_MINOR > 18 -- cgit From 54087d80f3603165c0bdeab9f14c9aba2ddb6c67 Mon Sep 17 00:00:00 2001 From: Florian Larysch Date: Tue, 5 Dec 2017 21:18:30 +0100 Subject: tui: always hide cursor when flushing The previous commit ensures that we can never flush the buffer in a state where toggling cursor visibility can corrupt other escape codes. Thus, we can remove the workaround added as part of e838452, simplyfing the code and hiding the cursor on more occasions. --- src/nvim/tui/tui.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/nvim/tui/tui.c b/src/nvim/tui/tui.c index d07eb9a923..61d3bde450 100644 --- a/src/nvim/tui/tui.c +++ b/src/nvim/tui/tui.c @@ -297,7 +297,7 @@ static void terminfo_stop(UI *ui) unibi_out_ext(ui, data->unibi_ext.disable_bracketed_paste); // Disable focus reporting unibi_out_ext(ui, data->unibi_ext.disable_focus_reporting); - flush_buf(ui, true); + flush_buf(ui); uv_tty_reset_mode(); uv_close((uv_handle_t *)&data->output_handle, NULL); uv_run(&data->write_loop, UV_RUN_DEFAULT); @@ -1061,7 +1061,7 @@ static void tui_flush(UI *ui) cursor_goto(ui, saved_row, saved_col); - flush_buf(ui, true); + flush_buf(ui); } #ifdef UNIX @@ -1230,7 +1230,7 @@ retry: \ unibi_format(vars, vars + 26, str, data->params, out, ui, NULL, NULL); \ if (data->overflow) { \ data->bufpos = orig_pos; \ - flush_buf(ui, true); \ + flush_buf(ui); \ goto retry; \ } \ data->cork = false; \ @@ -1261,7 +1261,7 @@ static void out(void *ctx, const char *str, size_t len) data->overflow = true; return; } else { - flush_buf(ui, true); + flush_buf(ui); } } @@ -1679,7 +1679,7 @@ static void augment_terminfo(TUIData *data, const char *term, "\x1b[?1002l\x1b[?1006l"); } -static void flush_buf(UI *ui, bool toggle_cursor) +static void flush_buf(UI *ui) { uv_write_t req; uv_buf_t bufs[3]; @@ -1690,7 +1690,7 @@ static void flush_buf(UI *ui, bool toggle_cursor) return; } - if (toggle_cursor && !data->is_invisible) { + if (!data->is_invisible) { // cursor is visible. Write a "cursor invisible" command before writing the // buffer. bufp->base = data->invis; @@ -1705,7 +1705,7 @@ static void flush_buf(UI *ui, bool toggle_cursor) bufp++; } - if (toggle_cursor && !data->busy && data->is_invisible) { + if (!data->busy && data->is_invisible) { // not busy and the cursor is invisible. Write a "cursor normal" command // after writing the buffer. bufp->base = data->norm; -- cgit