From 3b8c19ea460b4abb5b74f91e05a14c0fcf9cc6a6 Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Sat, 22 Jun 2024 12:14:23 +0900 Subject: [PATCH] fix(bufwrite): cursor flicker on :write MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PROBLEM: Cursor briefly flickers in cmdline when the "written" message is printed (very noticeable in Neovide with cursor animation). SOLUTION: - Mark UI "busy" (cursor hidden) while emitting the message, as done for the search message (cb2ca54331cd). - Note: ff68fd6b8a84 moved the message from `filemess()` into `buf_write..msg_progress`. - Fix a bug in `tui.c:flush_buf` which manifested after this change. See ANALYSIS below. https://github.com/libuv/libuv/issues/5182 ANALYSIS: After this change... ui_busy_start(); set_keep_msg(msg_progress(IObuff, msg_id, "success", 0, true, true), 0); ui_busy_stop(); ...ASAN analyzer fails on tui_spec.lua test "with non-tty (pipe) stdout/stderr": 2026-07-04T14:09:28.9521023Z = ==32405==ERROR: AddressSanitizer: ABRT on unknown address 0x03e900007e95 ... 4 0x7f5506a288fe in abort stdlib/abort.c:79:7 5 0x559afd7113a2 in uv__epoll_ctl_flush build/src/libuv/src/unix/linux.c:1335:7 6 0x559afd710a81 in uv__io_poll build/src/libuv/src/unix/linux.c:1448:9 7 0x559afd6f67a7 in uv_run build/src/libuv/src/unix/core.c:460:5 8 0x559afd2a72cc in flush_buf nvim/tui/tui.c:2642:5 9 0x559afd2bce9f in tui_flush nvim/tui/tui.c:1747:3 10 0x559afd2f39a0 in ui_client_event_flush nvim/auto/ui_events_client.generated.h:64:3 11 0x559afcb8e014 in parse_msgpack nvim/msgpack_rpc/channel.c:255:11 12 0x559afcb840e7 in receive_msgpack nvim/msgpack_rpc/channel.c:217:5 13 0x559afc53b2af in read_event nvim/event/rstream.c:180:23 14 0x559afc53ad52 in invoke_read_cb nvim/event/rstream.c:233:3 15 0x559afc5382d6 in read_cb nvim/event/rstream.c:135:3 16 0x559afd707b25 in uv__read build/src/libuv/src/unix/stream.c:1145:7 17 0x559afd70744a in uv__stream_io build/src/libuv/src/unix/stream.c:1208:5 18 0x559afd6f727e in uv__io_cb build/src/libuv/src/unix/core.c:930:5 19 0x559afd710d7a in uv__io_poll build/src/libuv/src/unix/linux.c:1546:11 20 0x559afd6f67a7 in uv_run build/src/libuv/src/unix/core.c:460:5 21 0x559afc526e2d in loop_uv_run nvim/event/loop.c:59:3 22 0x559afc526aa4 in loop_poll_events nvim/event/loop.c:80:26 23 0x559afd2fdbb7 in ui_client_run nvim/ui_client.c:172:5 24 0x559afc920d7b in main /home/runner/work/neovim/neovim/src/nvim/main.c:355:5 25 0x7f5506a2a1c9 in __libc_start_call_main main.h:58:16 26 0x7f5506a2a28a in __libc_start_main csu/../csu/libc-start.c:360:3 27 0x559afbd6d254 in _start The abort requires two rare conditions: 1. a `uv_write` whose buffer array *ends* in a zero-length buffer, which only happens by "flush while cursor-hidden". 2. an output fd that *epoll cannot watch*. The only CI test with such an fd is tui_spec.lua "with non-tty (pipe) stdout/stderr", which runs a TUI with `stdout > /dev/null`, then runs `:w testF`, which triggers the "written" message. Our change guarantees trailing-empty flushes (`:w` fires busy + `ui_flush()` while busy) in the only test that runs a TUI on `/dev/null`. 0. In a normal (non-busy) no-sync flush, post always contains `cursor_normal`. 1. Our change emits: ``` busy_start → msg_progress("…written") → ui_flush → busy_stop ``` 2. TUI flushes *while busy*, so `should_invisible() = true` and `flush_buf` skips the cursor-restore string (`bufs[2]` is zero-length). 3. libuv *cannot complete* a write whose tail is a zero-length buffer: EVERY trailing-empty write goes through epoll, even when every byte was already written. https://github.com/libuv/libuv/issues/5182 4. In `tui_spec.lua:3298`, the output fd is `/dev/null`, whose kernel `file_operations` has no `.poll` method (`drivers/char/mem.c`), so `EPOLL_CTL_ADD` fails with `EPERM`. Trailing empty buffers are semantically pointless, and libuv punishes them: forced async, one epoll round-trip *per trailing empty*, plus a 0-byte `write()` syscall. Helped-by: Shougo Matsushita Helped-by: Fred Sundvik --- src/nvim/bufwrite.c | 3 +++ src/nvim/tui/tui.c | 15 ++++++++++----- test/functional/ui/messages_spec.lua | 24 ++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/src/nvim/bufwrite.c b/src/nvim/bufwrite.c index 27bb77f82c..c8930dc909 100644 --- a/src/nvim/bufwrite.c +++ b/src/nvim/bufwrite.c @@ -1710,7 +1710,10 @@ restore_backup: xstrlcat(IObuff, shortmess(SHM_WRI) ? _(" [w]") : _(" written"), IOSIZE); } } + // Hide cursor while emitting "written" message, so cursor doesn't flicker in cmdline. #25974 + ui_busy_start(); set_keep_msg(msg_progress(IObuff, msg_id, "success", 0, true, true), 0); + ui_busy_stop(); } // When written everything correctly: reset 'modified'. Unless not diff --git a/src/nvim/tui/tui.c b/src/nvim/tui/tui.c index 3a0fd6bbfc..a1b57a932a 100644 --- a/src/nvim/tui/tui.c +++ b/src/nvim/tui/tui.c @@ -2634,12 +2634,17 @@ static void flush_buf(TUIData *tui, FlushBufFinish finish) fwrite(bufs[i].base, bufs[i].len, 1, tui->screenshot); } } else { - int ret - = uv_write(&req, (uv_stream_t *)&tui->output_handle, bufs, ARRAY_SIZE(bufs), NULL); - if (ret) { - ELOG("uv_write failed: %s", uv_strerror(ret)); + unsigned nbufs = ARRAY_SIZE(bufs); + while (nbufs > 0 && bufs[nbufs - 1].len == 0) { + nbufs--; // Trim trailing zero-length buffers. https://github.com/libuv/libuv/issues/5182 + } + if (nbufs > 0) { + int ret = uv_write(&req, (uv_stream_t *)&tui->output_handle, bufs, nbufs, NULL); + if (ret) { + ELOG("uv_write failed: %s", uv_strerror(ret)); + } + uv_run(&tui->write_loop, UV_RUN_DEFAULT); } - uv_run(&tui->write_loop, UV_RUN_DEFAULT); } tui->buf_to_flush = NULL; tui->bufpos = 0; diff --git a/test/functional/ui/messages_spec.lua b/test/functional/ui/messages_spec.lua index fa45293ebe..e79decbc3f 100644 --- a/test/functional/ui/messages_spec.lua +++ b/test/functional/ui/messages_spec.lua @@ -1727,6 +1727,30 @@ describe('ui/builtin messages', function() } end) + it('no cursor flicker during :write message (marks UI busy) #25974', function() + local fname = 'Xtest_write_busy' + finally(function() + os.remove(fname) + end) + local busy_start, busy_stop = 0, 0 + screen._handle_busy_start = (function(orig) + return function() + orig(screen) + busy_start = busy_start + 1 + end + end)(screen._handle_busy_start) + screen._handle_busy_stop = (function(orig) + return function() + orig(screen) + busy_stop = busy_stop + 1 + end + end)(screen._handle_busy_stop) + command('write ' .. fname) + screen:expect({ any = 'written' }) + eq(true, busy_start >= 1) -- cursor was hidden while the message was emitted + eq(busy_start, busy_stop) -- balanced: cursor restored afterwards + end) + it(':hi Group output', function() screen:try_resize(70, 7) feed(':hi ErrorMsg')