diff --git a/src/nvim/api/window.c b/src/nvim/api/window.c index 8600b37b12..c7f0f1e6c0 100644 --- a/src/nvim/api/window.c +++ b/src/nvim/api/window.c @@ -374,7 +374,7 @@ void nvim_win_hide(Window window, Error *err) } else if (tabpage == curtab) { win_close(win, false, false); } else { - win_close_othertab(win, false, tabpage); + win_close_othertab(win, false, tabpage, false); } }); } diff --git a/src/nvim/ex_docmd.c b/src/nvim/ex_docmd.c index bb8ddbf841..4d2ed4230e 100644 --- a/src/nvim/ex_docmd.c +++ b/src/nvim/ex_docmd.c @@ -4919,7 +4919,7 @@ void ex_win_close(int forceit, win_T *win, tabpage_T *tp) if (tp == NULL) { win_close(win, !need_hide && !buf_hide(buf), forceit); } else { - win_close_othertab(win, !need_hide && !buf_hide(buf), tp); + win_close_othertab(win, !need_hide && !buf_hide(buf), tp, forceit); } } diff --git a/src/nvim/window.c b/src/nvim/window.c index b38799514a..9dc87f7df1 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -2530,7 +2530,10 @@ void close_windows(buf_T *buf, bool keep_curwin) for (win_T *wp = tp->tp_lastwin; wp != NULL; wp = wp->w_prev) { if (wp->w_buffer == buf && !(win_locked(wp) || wp->w_buffer->b_locked > 0)) { - win_close_othertab(wp, false, tp); + if (!win_close_othertab(wp, false, tp, false)) { + // If closing the window fails give up, to avoid looping forever. + break; + } // Start all over, the tab page may be closed and // autocommands may change the window layout. @@ -2560,14 +2563,15 @@ bool one_window(win_T *win) FUNC_ATTR_PURE FUNC_ATTR_WARN_UNUSED_RESULT return firstwin == win && (win->w_next == NULL || win->w_next->w_floating); } -/// Check if floating windows in the current tab can be closed. +/// Check if floating windows in tabpage `tp` can be closed. /// Do not call this when the autocommand window is in use! /// +/// @param tp tabpage to check. Must be NULL for the current tabpage. /// @return true if all floating windows can be closed -static bool can_close_floating_windows(void) +static bool can_close_floating_windows(tabpage_T *tp) { - assert(!is_aucmd_win(lastwin)); - for (win_T *wp = lastwin; wp->w_floating; wp = wp->w_prev) { + assert(tp != curtab && (tp || !is_aucmd_win(lastwin))); + for (win_T *wp = tp ? tp->tp_lastwin : lastwin; wp->w_floating; wp = wp->w_prev) { buf_T *buf = wp->w_buffer; int need_hide = (bufIsChanged(buf) && buf->b_nwindows <= 1); @@ -2626,10 +2630,10 @@ static bool close_last_window_tabpage(win_T *win, bool free_buf, tabpage_T *prev // that below. goto_tabpage_tp(alt_tabpage(), false, true); - // Safety check: Autocommands may have closed the window when jumping - // to the other tab page. - if (valid_tabpage(prev_curtab) && prev_curtab->tp_firstwin == win) { - win_close_othertab(win, free_buf, prev_curtab); + // Safety check: Autocommands may have switched back to the old tab page + // or closed the window when jumping to the other tab page. + if (curtab != prev_curtab && valid_tabpage(prev_curtab) && prev_curtab->tp_firstwin == win) { + win_close_othertab(win, free_buf, prev_curtab, false); } entering_window(curwin); @@ -2710,7 +2714,7 @@ int win_close(win_T *win, bool free_buf, bool force) emsg(_("E814: Cannot close window, only autocmd window would remain")); return FAIL; } - if (force || can_close_floating_windows()) { + if (force || can_close_floating_windows(NULL)) { // close the last window until the there are no floating windows while (lastwin->w_floating) { // `force` flag isn't actually used when closing a floating window. @@ -2811,7 +2815,7 @@ int win_close(win_T *win, bool free_buf, bool force) if (curtab != prev_curtab && win_valid_any_tab(win) && win->w_buffer == NULL) { // Need to close the window anyway, since the buffer is NULL. - win_close_othertab(win, false, prev_curtab); + win_close_othertab(win, false, prev_curtab, force); return FAIL; } @@ -2969,14 +2973,38 @@ static void do_autocmd_winclosed(win_T *win) // thus "tp" may become invalid! // Caller must check if buffer is hidden and whether the tabline needs to be // updated. -void win_close_othertab(win_T *win, int free_buf, tabpage_T *tp) +// @return false when the window was not closed as a direct result of this call +// (e.g: not via autocmds). +bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force) FUNC_ATTR_NONNULL_ALL { + assert(tp != curtab); + // Get here with win->w_buffer == NULL when win_close() detects the tab page // changed. if (win_locked(win) || (win->w_buffer != NULL && win->w_buffer->b_locked > 0)) { - return; // window is already being closed + return false; // window is already being closed + } + + // Check if closing this window would leave only floating windows. + if (tp->tp_firstwin == win && win->w_next && win->w_next->w_floating) { + if (force || can_close_floating_windows(tp)) { + // close the last window until the there are no floating windows + while (tp->tp_lastwin->w_floating) { + // `force` flag isn't actually used when closing a floating window. + if (!win_close_othertab(tp->tp_lastwin, free_buf, tp, true)) { + // If closing the window fails give up, to avoid looping forever. + goto leave_open; + } + } + if (!win_valid_any_tab(win)) { + return false; // window already closed by autocommands + } + } else { + emsg(e_floatonly); + goto leave_open; + } } // Fire WinClosed just before starting to free window-related resources. @@ -2986,7 +3014,7 @@ void win_close_othertab(win_T *win, int free_buf, tabpage_T *tp) do_autocmd_winclosed(win); // autocmd may have freed the window already. if (!win_valid_any_tab(win)) { - return; + return false; } } @@ -2995,34 +3023,21 @@ void win_close_othertab(win_T *win, int free_buf, tabpage_T *tp) close_buffer(win, win->w_buffer, free_buf ? DOBUF_UNLOAD : 0, false, true); } - tabpage_T *ptp = NULL; - // Careful: Autocommands may have closed the tab page or made it the // current tab page. - for (ptp = first_tabpage; ptp != NULL && ptp != tp; ptp = ptp->tp_next) {} - if (ptp == NULL || tp == curtab) { - // If the buffer was removed from the window we have to give it any - // buffer. - if (win_valid_any_tab(win) && win->w_buffer == NULL) { - win->w_buffer = firstbuf; - firstbuf->b_nwindows++; - win_init_empty(win); - } - return; + if (!valid_tabpage(tp) || tp == curtab) { + goto leave_open; } - - // Autocommands may have closed the window already. - { - bool found_window = false; - FOR_ALL_WINDOWS_IN_TAB(wp, tp) { - if (wp == win) { - found_window = true; - break; - } - } - if (!found_window) { - return; - } + // Autocommands may have closed the window already, or nvim_win_set_config + // moved it to a different tab page. + if (!tabpage_win_valid(tp, win)) { + goto leave_open; + } + // Autocommands may again cause closing this window to leave only floats. + // Check again; we'll not bother closing floating windows this time. + if (tp->tp_firstwin == win && win->w_next && win->w_next->w_floating) { + emsg(e_floatonly); + goto leave_open; } bool free_tp = false; @@ -3039,13 +3054,14 @@ void win_close_othertab(win_T *win, int free_buf, tabpage_T *tp) if (tp == first_tabpage) { first_tabpage = tp->tp_next; } else { + tabpage_T *ptp; for (ptp = first_tabpage; ptp != NULL && ptp->tp_next != tp; ptp = ptp->tp_next) { // loop } if (ptp == NULL) { internal_error("win_close_othertab()"); - return; + return false; } ptp->tp_next = tp->tp_next; } @@ -3067,6 +3083,16 @@ void win_close_othertab(win_T *win, int free_buf, tabpage_T *tp) if (free_tp) { free_tabpage(tp); } + return true; + +leave_open: + // If the buffer was removed from the window we have to give it any buffer. + if (win_valid_any_tab(win) && win->w_buffer == NULL) { + win->w_buffer = firstbuf; + firstbuf->b_nwindows++; + win_init_empty(win); + } + return false; } /// Free the memory used for a window. diff --git a/test/functional/api/window_spec.lua b/test/functional/api/window_spec.lua index 1b64fada5a..e4565119fd 100644 --- a/test/functional/api/window_spec.lua +++ b/test/functional/api/window_spec.lua @@ -2153,6 +2153,63 @@ describe('API/win', function() }) ) end) + + it('no crash when closing the only non-float in other tabpage #31236', function() + local tp = api.nvim_get_current_tabpage() + local split_win = api.nvim_get_current_win() + local float_win = api.nvim_open_win( + 0, + false, + { relative = 'editor', width = 5, height = 5, row = 1, col = 1 } + ) + command('tabnew') + + api.nvim_win_close(split_win, false) + eq(false, api.nvim_win_is_valid(split_win)) + eq(false, api.nvim_win_is_valid(float_win)) + eq(false, api.nvim_tabpage_is_valid(tp)) + + tp = api.nvim_get_current_tabpage() + split_win = api.nvim_get_current_win() + local float_buf = api.nvim_create_buf(true, false) + float_win = api.nvim_open_win( + float_buf, + false, + { relative = 'editor', width = 5, height = 5, row = 1, col = 1 } + ) + -- Set these options to prevent the float from being automatically closed. + api.nvim_set_option_value('modified', true, { buf = float_buf }) + api.nvim_set_option_value('bufhidden', 'wipe', { buf = float_buf }) + command('tabnew') + + matches( + 'E5601: Cannot close window, only floating window would remain$', + pcall_err(api.nvim_win_close, split_win, false) + ) + eq(true, api.nvim_win_is_valid(split_win)) + eq(true, api.nvim_win_is_valid(float_win)) + eq(true, api.nvim_tabpage_is_valid(tp)) + + api.nvim_set_current_win(float_win) + api.nvim_win_close(split_win, true) -- Force it this time. + eq(false, api.nvim_win_is_valid(split_win)) + eq(false, api.nvim_win_is_valid(float_win)) + eq(false, api.nvim_tabpage_is_valid(tp)) + + -- Ensure opening a float after the initial check (like in WinClosed) doesn't crash... + exec([[ + tabnew + let g:tp = nvim_get_current_tabpage() + let g:win = win_getid() + tabprevious + autocmd! WinClosed * ++once call nvim_open_win(0, 0, #{win: g:win, relative: 'win', width: 5, height: 5, row: 5, col: 5}) + ]]) + matches( + 'E5601: Cannot close window, only floating window would remain$', + pcall_err(command, 'call nvim_win_close(g:win, 0)') + ) + eq(true, eval 'nvim_tabpage_is_valid(g:tp)') + end) end) describe('set_config', function() diff --git a/test/functional/editor/tabpage_spec.lua b/test/functional/editor/tabpage_spec.lua index b6fbebb20c..caf5e23e54 100644 --- a/test/functional/editor/tabpage_spec.lua +++ b/test/functional/editor/tabpage_spec.lua @@ -142,4 +142,13 @@ describe('tabpage', function() command('tabs') assert_alive() end) + + it('no crash if autocmd remains in tabpage of closing last window', function() + exec([[ + tabnew + let s:win = win_getid() + autocmd TabLeave * ++once tablast | tabonly + quit + ]]) + end) end)