From d5bdcb2bd71d8ddba8005c78312984694612b0fa Mon Sep 17 00:00:00 2001 From: zeertzjq Date: Tue, 27 Dec 2022 09:01:57 +0800 Subject: [PATCH] fix(win_close): remove float grid after closing buffer It is not safe to remove the float grid when autocommands can still be triggered, as autocommands may use the float grid. (cherry picked from commit 9a5e80af5649573ec4b6093acc93312cc8cbf6cc) --- src/nvim/window.c | 44 +++++++++++++++---------------- test/functional/ui/float_spec.lua | 28 ++++++++++++++++++++ 2 files changed, 50 insertions(+), 22 deletions(-) diff --git a/src/nvim/window.c b/src/nvim/window.c index defea150b9..6660bb4cc0 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -2736,28 +2736,6 @@ int win_close(win_T *win, bool free_buf, bool force) } } - bool was_floating = win->w_floating; - if (ui_has(kUIMultigrid)) { - ui_call_win_close(win->w_grid_alloc.handle); - } - - if (win->w_floating) { - ui_comp_remove_grid(&win->w_grid_alloc); - assert(first_tabpage != NULL); // suppress clang "Dereference of NULL pointer" - if (win->w_float_config.external) { - for (tabpage_T *tp = first_tabpage; tp != NULL; tp = tp->tp_next) { - if (tp == curtab) { - continue; - } - if (tp->tp_curwin == win) { - // NB: an autocmd can still abort the closing of this window, - // bur carring out this change anyway shouldn't be a catastrophe. - tp->tp_curwin = tp->tp_firstwin; - } - } - } - } - // Fire WinClosed just before starting to free window-related resources. do_autocmd_winclosed(win); // autocmd may have freed the window already. @@ -2803,6 +2781,28 @@ int win_close(win_T *win, bool free_buf, bool force) // let terminal buffers know that this window dimensions may be ignored win->w_closing = true; + bool was_floating = win->w_floating; + if (ui_has(kUIMultigrid)) { + ui_call_win_close(win->w_grid_alloc.handle); + } + + if (win->w_floating) { + ui_comp_remove_grid(&win->w_grid_alloc); + assert(first_tabpage != NULL); // suppress clang "Dereference of NULL pointer" + if (win->w_float_config.external) { + for (tabpage_T *tp = first_tabpage; tp != NULL; tp = tp->tp_next) { + if (tp == curtab) { + continue; + } + if (tp->tp_curwin == win) { + // NB: an autocmd can still abort the closing of this window, + // bur carring out this change anyway shouldn't be a catastrophe. + tp->tp_curwin = tp->tp_firstwin; + } + } + } + } + // Free the memory used for the window and get the window that received // the screen space. wp = win_free_mem(win, &dir, NULL); diff --git a/test/functional/ui/float_spec.lua b/test/functional/ui/float_spec.lua index 60a30d7558..f17f1f2908 100644 --- a/test/functional/ui/float_spec.lua +++ b/test/functional/ui/float_spec.lua @@ -8521,6 +8521,34 @@ describe('float window', function() ]]} end end) + + describe('no crash after moving and closing float window #21547', function() + local function test_float_move_close(cmd) + local float_opts = {relative = 'editor', row = 1, col = 1, width = 10, height = 10} + meths.open_win(meths.create_buf(false, false), true, float_opts) + if multigrid then + screen:expect({float_pos = {[4] = {{id = 1001}, 'NW', 1, 1, 1, true}}}) + end + command(cmd) + exec_lua([[ + vim.api.nvim_win_set_config(0, {relative = 'editor', row = 2, col = 2}) + vim.api.nvim_win_close(0, {}) + vim.api.nvim_echo({{''}}, false, {}) + ]]) + if multigrid then + screen:expect({float_pos = {}}) + end + assert_alive() + end + + it('if WinClosed autocommand flushes UI', function() + test_float_move_close('autocmd WinClosed * ++once redraw') + end) + + it('if closing buffer flushes UI', function() + test_float_move_close('autocmd BufWinLeave * ++once redraw') + end) + end) end describe('with ext_multigrid', function()