From 13f5bc9586cd84eac758a7e109fc88587f858cf0 Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Sun, 16 Jul 2017 18:09:48 +0200 Subject: log: all UI events, not just UI-bridge Rename ui_bridge.c:UI_CALL to UI_BRIDGE_CALL. --- src/nvim/ui_bridge.c | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) (limited to 'src/nvim/ui_bridge.c') diff --git a/src/nvim/ui_bridge.c b/src/nvim/ui_bridge.c index 0165db7c0c..5585886612 100644 --- a/src/nvim/ui_bridge.c +++ b/src/nvim/ui_bridge.c @@ -24,30 +24,10 @@ #define UI(b) (((UIBridgeData *)b)->ui) -#if MIN_LOG_LEVEL <= DEBUG_LOG_LEVEL -static size_t uilog_seen = 0; -static argv_callback uilog_event = NULL; -#define UI_CALL(ui, name, argc, ...) \ - do { \ - if (uilog_event == ui_bridge_##name##_event) { \ - uilog_seen++; \ - } else { \ - if (uilog_seen > 0) { \ - DLOG("UI bridge: ...%zu times", uilog_seen); \ - } \ - DLOG("UI bridge: " STR(name)); \ - uilog_seen = 0; \ - uilog_event = ui_bridge_##name##_event; \ - } \ - ((UIBridgeData *)ui)->scheduler( \ - event_create(ui_bridge_##name##_event, argc, __VA_ARGS__), UI(ui)); \ - } while (0) -#else // Schedule a function call on the UI bridge thread. -#define UI_CALL(ui, name, argc, ...) \ +#define UI_BRIDGE_CALL(ui, name, argc, ...) \ ((UIBridgeData *)ui)->scheduler( \ event_create(ui_bridge_##name##_event, argc, __VA_ARGS__), UI(ui)) -#endif #define INT2PTR(i) ((void *)(intptr_t)i) #define PTR2INT(p) ((Integer)(intptr_t)p) @@ -128,7 +108,7 @@ static void ui_bridge_stop(UI *b) { UIBridgeData *bridge = (UIBridgeData *)b; bool stopped = bridge->stopped = false; - UI_CALL(b, stop, 1, b); + UI_BRIDGE_CALL(b, stop, 1, b); for (;;) { uv_mutex_lock(&bridge->mutex); stopped = bridge->stopped; @@ -154,7 +134,7 @@ static void ui_bridge_highlight_set(UI *b, HlAttrs attrs) { HlAttrs *a = xmalloc(sizeof(HlAttrs)); *a = attrs; - UI_CALL(b, highlight_set, 2, b, a); + UI_BRIDGE_CALL(b, highlight_set, 2, b, a); } static void ui_bridge_highlight_set_event(void **argv) { @@ -167,7 +147,7 @@ static void ui_bridge_suspend(UI *b) { UIBridgeData *data = (UIBridgeData *)b; uv_mutex_lock(&data->mutex); - UI_CALL(b, suspend, 1, b); + UI_BRIDGE_CALL(b, suspend, 1, b); data->ready = false; // suspend the main thread until CONTINUE is called by the UI thread while (!data->ready) { -- cgit From 3aa24042a8f7d591b8091b437fc9cdb03497bc7a Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Mon, 27 Nov 2017 00:35:09 +0100 Subject: tui: dump termcap info if -V3 ('verbose' >= 3) Get terminal debugging info by starting Nvim with 'verbose' level 3: nvim -V3log This is like Vim's `:set termcap`, which was removed in Nvim (and would be very awkward to restore because of the decoupled UI). --- src/nvim/ui_bridge.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/nvim/ui_bridge.c') diff --git a/src/nvim/ui_bridge.c b/src/nvim/ui_bridge.c index 5585886612..7573fa1653 100644 --- a/src/nvim/ui_bridge.c +++ b/src/nvim/ui_bridge.c @@ -82,6 +82,7 @@ UI *ui_bridge_attach(UI *ui, ui_main_fn ui_main, event_scheduler scheduler) abort(); } + // Suspend the main thread until CONTINUE is called by the UI thread. while (!rv->ready) { uv_cond_wait(&rv->cond, &rv->mutex); } @@ -149,7 +150,7 @@ static void ui_bridge_suspend(UI *b) uv_mutex_lock(&data->mutex); UI_BRIDGE_CALL(b, suspend, 1, b); data->ready = false; - // suspend the main thread until CONTINUE is called by the UI thread + // Suspend the main thread until CONTINUE is called by the UI thread. while (!data->ready) { uv_cond_wait(&data->cond, &data->mutex); } -- cgit From 34057045beca40406673ff421a4ef1e8e8c08853 Mon Sep 17 00:00:00 2001 From: Björn Linse Date: Tue, 12 Dec 2017 18:23:19 +0100 Subject: ui: forward relevant option updates to UIs (#7520) also make termguicolors mutable after startup --- src/nvim/ui_bridge.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'src/nvim/ui_bridge.c') diff --git a/src/nvim/ui_bridge.c b/src/nvim/ui_bridge.c index 7573fa1653..0a69cf0ecb 100644 --- a/src/nvim/ui_bridge.c +++ b/src/nvim/ui_bridge.c @@ -66,6 +66,7 @@ UI *ui_bridge_attach(UI *ui, ui_main_fn ui_main, event_scheduler scheduler) rv->bridge.suspend = ui_bridge_suspend; rv->bridge.set_title = ui_bridge_set_title; rv->bridge.set_icon = ui_bridge_set_icon; + rv->bridge.option_set = ui_bridge_option_set; rv->scheduler = scheduler; for (UIWidget i = 0; (int)i < UI_WIDGETS; i++) { @@ -144,6 +145,29 @@ static void ui_bridge_highlight_set_event(void **argv) xfree(argv[1]); } +static void ui_bridge_option_set(UI *ui, String name, Object value) +{ + // Assumes bridge is only used by TUI + if (strequal(name.data, "termguicolors")) { + ui->rgb = value.data.boolean; + } + String copy_name = copy_string(name); + Object *copy_value = xmalloc(sizeof(Object)); + *copy_value = copy_object(value); + UI_BRIDGE_CALL(ui, option_set, 4, ui, + copy_name.data, INT2PTR(copy_name.size), copy_value); +} +static void ui_bridge_option_set_event(void **argv) +{ + UI *ui = UI(argv[0]); + String name = (String){ .data = argv[1], .size = (size_t)argv[2] }; + Object value = *(Object *)argv[3]; + ui->option_set(ui, name, value); + api_free_string(name); + api_free_object(value); + xfree(argv[3]); +} + static void ui_bridge_suspend(UI *b) { UIBridgeData *data = (UIBridgeData *)b; -- cgit From c6fe06bbc09c0adbb3ef30be9f5b238e802b5038 Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Sat, 27 Jan 2018 13:52:45 +0100 Subject: tui: fix use-after-free after UI `stop` event #7922 ui_bridge:ui_bridge_stop() calls ui_detach_impl() last, so the check for ui_active() in ui:ui_refresh() doesn't help: tui_main() already freed the `ui` object. There is a race between ui_bridge_stop (thread T0) and tui_main (thread T1). UIBridgeData.stopped could be set while ui_bridge_stop() is in the middle of loop_poll_events(), which may invoke tui_scheduler() on T0. The pointers in tui_scheduler() may be invalid by then. Solution(?): Use the `UI.data` field as a "stopped" flag and check it in tui_scheduler(). ASAN use-after-free report observed in #7908: = ==20066==ERROR: AddressSanitizer: heap-use-after-free on address 0x611000000cd0 at pc 0x00000182abed bp 0x7ffe23b07070 sp 0x7ffe23b07068 = READ of size 8 at 0x611000000cd0 thread T0 = 0 0x182abec in tui_scheduler /home/travis/build/neovim/neovim/src/nvim/tui/tui.c:393:23 = 1 0x1876afd in ui_bridge_update_fg /home/travis/build/neovim/neovim/build/src/nvim/auto/ui_events_bridge.generated.h:205:3 = 2 0x186c130 in ui_resize /home/travis/build/neovim/neovim/src/nvim/ui.c:310:3 = 3 0x146b9c2 in screen_resize /home/travis/build/neovim/neovim/src/nvim/screen.c:7483:3 = 4 0x186a6f0 in ui_refresh /home/travis/build/neovim/neovim/src/nvim/ui.c:284:3 = 5 0x186bbe0 in ui_refresh_event /home/travis/build/neovim/neovim/src/nvim/ui.c:297:3 = 6 0xa2219a in multiqueue_process_events /home/travis/build/neovim/neovim/src/nvim/event/multiqueue.c:150:7 = 7 0xa1bd7f in loop_poll_events /home/travis/build/neovim/neovim/src/nvim/event/loop.c:63:3 = 8 0x1872709 in ui_bridge_stop /home/travis/build/neovim/neovim/src/nvim/ui_bridge.c:121:5 = 9 0x1864247 in ui_builtin_stop /home/travis/build/neovim/neovim/src/nvim/ui.c:143:3 = 10 0x1249ec8 in mch_exit /home/travis/build/neovim/neovim/src/nvim/os_unix.c:140:3 = 11 0xe56ba9 in getout /home/travis/build/neovim/neovim/src/nvim/main.c:671:3 = 12 0xfc4c8f in preserve_exit /home/travis/build/neovim/neovim/src/nvim/misc1.c:2653:3 = 13 0x1247c02 in deadly_signal /home/travis/build/neovim/neovim/src/nvim/os/signal.c:137:3 = 14 0x1247921 in on_signal /home/travis/build/neovim/neovim/src/nvim/os/signal.c:162:9 = 15 0xa35618 in signal_event /home/travis/build/neovim/neovim/src/nvim/event/signal.c:47:3 = 16 0xa2219a in multiqueue_process_events /home/travis/build/neovim/neovim/src/nvim/event/multiqueue.c:150:7 = 17 0xa1bd7f in loop_poll_events /home/travis/build/neovim/neovim/src/nvim/event/loop.c:63:3 = 18 0x1237bd6 in input_poll /home/travis/build/neovim/neovim/src/nvim/os/input.c:349:3 = 19 0x123334f in inbuf_poll /home/travis/build/neovim/neovim/src/nvim/os/input.c:372:24 = 20 0x123316d in os_inchar /home/travis/build/neovim/neovim/src/nvim/os/input.c:110:19 = 21 0x170d20e in state_enter /home/travis/build/neovim/neovim/src/nvim/state.c:55:13 = 22 0xbd7441 in command_line_enter /home/travis/build/neovim/neovim/src/nvim/ex_getln.c:384:3 = 23 0xbd0a60 in getcmdline /home/travis/build/neovim/neovim/src/nvim/ex_getln.c:1920:10 = 24 0xbdb365 in getexline /home/travis/build/neovim/neovim/src/nvim/ex_getln.c:2100:10 = 25 0xb00a6b in do_cmdline /home/travis/build/neovim/neovim/src/nvim/ex_docmd.c:528:47 = 26 0x10a7837 in nv_colon /home/travis/build/neovim/neovim/src/nvim/normal.c:4552:18 = 27 0x1091e15 in normal_execute /home/travis/build/neovim/neovim/src/nvim/normal.c:1136:3 = 28 0x170d439 in state_enter /home/travis/build/neovim/neovim/src/nvim/state.c:67:26 = 29 0x104ee14 in normal_enter /home/travis/build/neovim/neovim/src/nvim/normal.c:466:3 = 30 0xe4295c in main /home/travis/build/neovim/neovim/src/nvim/main.c:572:3 = 31 0x2b2ba340bf44 in __libc_start_main /build/eglibc-ripdx6/eglibc-2.19/csu/libc-start.c:287 = 32 0x44d24b in _start (/home/travis/build/neovim/neovim/build/bin/nvim+0x44d24b) = = 0x611000000cd0 is located 16 bytes inside of 240-byte region [0x611000000cc0,0x611000000db0) = freed by thread T1 here: = 0 0x4ee0e2 in __interceptor_free /local/mnt/workspace/tmp/ubuntu_rel/llvm/utils/release/final/llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:47:3 = 1 0xf4f6d4 in xfree /home/travis/build/neovim/neovim/src/nvim/memory.c:133:3 = 2 0x182a963 in tui_main /home/travis/build/neovim/neovim/src/nvim/tui/tui.c:383:3 = 3 0x18792b0 in ui_thread_run /home/travis/build/neovim/neovim/src/nvim/ui_bridge.c:106:3 = 4 0x2b2ba2697183 in start_thread /build/eglibc-ripdx6/eglibc-2.19/nptl/pthread_create.c:312 = = previously allocated by thread T0 here: = 0 0x4ee61a in calloc /local/mnt/workspace/tmp/ubuntu_rel/llvm/utils/release/final/llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:76:3 = 1 0xf4f787 in xcalloc /home/travis/build/neovim/neovim/src/nvim/memory.c:147:15 = 2 0x182000a in tui_start /home/travis/build/neovim/neovim/src/nvim/tui/tui.c:127:12 = 3 0x1863f7c in ui_builtin_start /home/travis/build/neovim/neovim/src/nvim/ui.c:125:3 = 4 0xe41bb9 in main /home/travis/build/neovim/neovim/src/nvim/main.c:457:5 = 5 0x2b2ba340bf44 in __libc_start_main /build/eglibc-ripdx6/eglibc-2.19/csu/libc-start.c:287 = = Thread T1 created by T0 here: = 0 0x4d774d in __interceptor_pthread_create /local/mnt/workspace/tmp/ubuntu_rel/llvm/utils/release/final/llvm.src/projects/compiler-rt/lib/asan/asan_interceptors.cc:317:3 = 1 0x1aae6b0 in uv_thread_create /home/travis/nvim-deps/build/src/libuv/src/unix/thread.c:75 = 2 0x18217fa in tui_start /home/travis/build/neovim/neovim/src/nvim/tui/tui.c:159:10 = 3 0x1863f7c in ui_builtin_start /home/travis/build/neovim/neovim/src/nvim/ui.c:125:3 = 4 0xe41bb9 in main /home/travis/build/neovim/neovim/src/nvim/main.c:457:5 = 5 0x2b2ba340bf44 in __libc_start_main /build/eglibc-ripdx6/eglibc-2.19/csu/libc-start.c:287 --- Alternative attempt: commit 6ad9c02491606a0c31e907f38c9931f324327aa5 Author: Justin M. Keyes Date: Sat Jan 27 15:12:58 2018 +0100 tui: fix use-after-free: swap in empty scheduler This should make life easier for UIs like VimR which implement their own in-process bridged UI: they don't need to worry that their `scheduler` might receive an invalid pointer. To avoid that, ui_bridge_stopped() swaps in an empty scheduler. Note that this requires the call to loop_poll_events() to be moved into the critical section. diff --git a/src/nvim/ui_bridge.c b/src/nvim/ui_bridge.c index 779585416f80..491052d19d3b 100644 --- a/src/nvim/ui_bridge.c +++ b/src/nvim/ui_bridge.c @@ -93,10 +93,18 @@ UI *ui_bridge_attach(UI *ui, ui_main_fn ui_main, event_scheduler scheduler) return &rv->bridge; } +static void ui_bridge_null_scheduler(Event event, void *d) +{ + WLOG("ignoring event (bridge stopped)"); +} + void ui_bridge_stopped(UIBridgeData *bridge) { uv_mutex_lock(&bridge->mutex); bridge->stopped = true; + // Replace with an empty scheduler, so that the UI internal scheduler does + // not get invoked with an invalid pointer. #7922 + bridge->scheduler = ui_bridge_null_scheduler; uv_mutex_unlock(&bridge->mutex); } @@ -111,14 +119,11 @@ static void ui_bridge_stop(UI *b) UIBridgeData *bridge = (UIBridgeData *)b; bool stopped = bridge->stopped = false; UI_BRIDGE_CALL(b, stop, 1, b); - for (;;) { + while (!stopped) { uv_mutex_lock(&bridge->mutex); stopped = bridge->stopped; - uv_mutex_unlock(&bridge->mutex); - if (stopped) { - break; - } loop_poll_events(&main_loop, 10); // Process one event (at most). + uv_mutex_unlock(&bridge->mutex); } uv_thread_join(&bridge->ui_thread); uv_mutex_destroy(&bridge->mutex); --- src/nvim/ui_bridge.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/nvim/ui_bridge.c') diff --git a/src/nvim/ui_bridge.c b/src/nvim/ui_bridge.c index 0a69cf0ecb..16dd42ebaa 100644 --- a/src/nvim/ui_bridge.c +++ b/src/nvim/ui_bridge.c @@ -118,11 +118,12 @@ static void ui_bridge_stop(UI *b) if (stopped) { break; } - loop_poll_events(&main_loop, 10); + loop_poll_events(&main_loop, 10); // Process one event. } uv_thread_join(&bridge->ui_thread); uv_mutex_destroy(&bridge->mutex); uv_cond_destroy(&bridge->cond); + xfree(bridge->ui); // Threads joined, now safe to free UI container. #7922 ui_detach_impl(b); xfree(b); } -- cgit From 5d8da126d0b5ab7f550a74264ba434a2ad04280e Mon Sep 17 00:00:00 2001 From: Björn Linse Date: Tue, 6 Feb 2018 19:46:45 +0100 Subject: ui/tui: highlighting refactor Make HlAttr contain highlighting state for both color modes (cterm and rgb). This allows us to implement termguicolors completely in the TUI. Simplify some logic duplicated between ui.c and screen.c. Also avoid some superfluous highlighting reset events. --- src/nvim/ui_bridge.c | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-) (limited to 'src/nvim/ui_bridge.c') diff --git a/src/nvim/ui_bridge.c b/src/nvim/ui_bridge.c index 16dd42ebaa..b0f8905771 100644 --- a/src/nvim/ui_bridge.c +++ b/src/nvim/ui_bridge.c @@ -59,9 +59,7 @@ UI *ui_bridge_attach(UI *ui, ui_main_fn ui_main, event_scheduler scheduler) rv->bridge.put = ui_bridge_put; rv->bridge.bell = ui_bridge_bell; rv->bridge.visual_bell = ui_bridge_visual_bell; - rv->bridge.update_fg = ui_bridge_update_fg; - rv->bridge.update_bg = ui_bridge_update_bg; - rv->bridge.update_sp = ui_bridge_update_sp; + rv->bridge.default_colors_set = ui_bridge_default_colors_set; rv->bridge.flush = ui_bridge_flush; rv->bridge.suspend = ui_bridge_suspend; rv->bridge.set_title = ui_bridge_set_title; @@ -146,29 +144,6 @@ static void ui_bridge_highlight_set_event(void **argv) xfree(argv[1]); } -static void ui_bridge_option_set(UI *ui, String name, Object value) -{ - // Assumes bridge is only used by TUI - if (strequal(name.data, "termguicolors")) { - ui->rgb = value.data.boolean; - } - String copy_name = copy_string(name); - Object *copy_value = xmalloc(sizeof(Object)); - *copy_value = copy_object(value); - UI_BRIDGE_CALL(ui, option_set, 4, ui, - copy_name.data, INT2PTR(copy_name.size), copy_value); -} -static void ui_bridge_option_set_event(void **argv) -{ - UI *ui = UI(argv[0]); - String name = (String){ .data = argv[1], .size = (size_t)argv[2] }; - Object value = *(Object *)argv[3]; - ui->option_set(ui, name, value); - api_free_string(name); - api_free_object(value); - xfree(argv[3]); -} - static void ui_bridge_suspend(UI *b) { UIBridgeData *data = (UIBridgeData *)b; -- cgit From 6e5cb0debd23693175bd05409d3f1af4015567df Mon Sep 17 00:00:00 2001 From: Björn Linse Date: Tue, 13 Feb 2018 13:45:49 +0100 Subject: ui: refactor ui options --- src/nvim/ui_bridge.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/nvim/ui_bridge.c') diff --git a/src/nvim/ui_bridge.c b/src/nvim/ui_bridge.c index b0f8905771..c936a17e52 100644 --- a/src/nvim/ui_bridge.c +++ b/src/nvim/ui_bridge.c @@ -67,7 +67,7 @@ UI *ui_bridge_attach(UI *ui, ui_main_fn ui_main, event_scheduler scheduler) rv->bridge.option_set = ui_bridge_option_set; rv->scheduler = scheduler; - for (UIWidget i = 0; (int)i < UI_WIDGETS; i++) { + for (UIExtension i = 0; (int)i < kUIExtCount; i++) { rv->bridge.ui_ext[i] = ui->ui_ext[i]; } -- cgit From 8b05da157728e960b15c7e719a20eedb61637703 Mon Sep 17 00:00:00 2001 From: Björn Linse Date: Thu, 22 Feb 2018 09:00:21 +0100 Subject: ui_bridge: make sure TUI receives no more UI events after "stop" event --- src/nvim/ui_bridge.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/nvim/ui_bridge.c') diff --git a/src/nvim/ui_bridge.c b/src/nvim/ui_bridge.c index c936a17e52..56e0c0c454 100644 --- a/src/nvim/ui_bridge.c +++ b/src/nvim/ui_bridge.c @@ -106,6 +106,9 @@ static void ui_thread_run(void *data) static void ui_bridge_stop(UI *b) { + // Detach brigde first, so that "stop" is the last event the TUI loop + // receives from the main thread. #8041 + ui_detach_impl(b); UIBridgeData *bridge = (UIBridgeData *)b; bool stopped = bridge->stopped = false; UI_BRIDGE_CALL(b, stop, 1, b); @@ -122,7 +125,6 @@ static void ui_bridge_stop(UI *b) uv_mutex_destroy(&bridge->mutex); uv_cond_destroy(&bridge->cond); xfree(bridge->ui); // Threads joined, now safe to free UI container. #7922 - ui_detach_impl(b); xfree(b); } static void ui_bridge_stop_event(void **argv) -- cgit