diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c index 641a2a7554..80ded39984 100644 --- a/src/nvim/buffer.c +++ b/src/nvim/buffer.c @@ -262,7 +262,7 @@ int open_buffer(bool read_stdin, exarg_T *eap, int flags_arg) if (ml_open(curbuf) == FAIL) { // There MUST be a memfile, otherwise we can't do anything // If we can't create one for the current buffer, take another buffer - close_buffer(NULL, curbuf, 0, false, false); + close_buffer(curwin, curbuf, 0, false, false, false); curbuf = NULL; FOR_ALL_BUFFERS(buf) { @@ -526,7 +526,12 @@ void buf_close_terminal(buf_T *buf) /// Close the link to a buffer. /// -/// @param win If not NULL, set b_last_cursor. +/// @param win If not NULL and we reached the end and unloaded "buf", b_nwindows is decremented +/// if w_buffer was "buf" after autocmds. w_buffer is set to NULL in that case. +/// Otherwise, w_buffer->b_nwindows is not decremented; callers should decrement it if +/// they still intend to switch "win"'s buffer or close "win"! If "win" was initially +/// curwin displaying "buf", it is re-entered if autocmds switched windows, if still +/// open. /// @param buf /// @param action Used when there is no longer a window for the buffer. /// Possible values: @@ -534,8 +539,7 @@ void buf_close_terminal(buf_T *buf) /// DOBUF_UNLOAD buffer is unloaded /// DOBUF_DEL buffer is unloaded and removed from buffer list /// DOBUF_WIPE buffer is unloaded and really deleted -/// When doing all but the first one on the current buffer, the -/// caller should get a new buffer very soon! +/// When doing all but the first, the caller should get a new buffer very soon! /// The 'bufhidden' option can force freeing and deleting. /// @param abort_if_last /// If true, do not close the buffer if autocommands cause @@ -544,15 +548,19 @@ void buf_close_terminal(buf_T *buf) /// close all other windows. /// @param ignore_abort /// If true, don't abort even when aborting() returns true. -/// @return true if b_nwindows was decremented directly by this call (e.g: not via autocmds). -bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool ignore_abort) +/// @param set_context +/// If true, also call buflist_setfpos for "win" if it's showing "buf", and set +/// b_last_cursor if "win" is the buffer's only window. +/// @return true when we got to the end and unloaded "buf". +bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool ignore_abort, + bool set_context) + FUNC_ATTR_NONNULL_ARG(2) { bool unload_buf = (action != 0); bool del_buf = (action == DOBUF_DEL || action == DOBUF_WIPE); bool wipe_buf = (action == DOBUF_WIPE); - bool is_curwin = (curwin != NULL && curwin->w_buffer == buf); - win_T *the_curwin = curwin; + bool is_curwin = (curwin != NULL && curwin == win && curwin->w_buffer == buf); tabpage_T *the_curtab = curtab; CHECK_CURBUF; @@ -587,9 +595,8 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i return false; } - // check no autocommands closed the window - if (win != NULL // Avoid bogus clang warning. - && win_valid_any_tab(win)) { + bool win_valid = win_valid_any_tab(win); + if (set_context && win_valid && win->w_buffer == buf) { // Set b_last_cursor when closing the last window for the buffer. // Remember the last cursor position and window options of the buffer. // This used to be only for the current window, but then options like @@ -606,7 +613,7 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i set_bufref(&bufref, buf); // When the buffer is no longer in a window, trigger BufWinLeave - if (buf->b_nwindows == 1) { + if (win_valid && win->w_buffer == buf && buf->b_nwindows == 1) { buf->b_locked++; buf->b_locked_split++; if (apply_autocmds(EVENT_BUFWINLEAVE, buf->b_fname, buf->b_fname, false, @@ -646,32 +653,29 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i if (!ignore_abort && aborting()) { return false; } + win_valid = win_valid && win_valid_any_tab(win); } // If the buffer was in curwin and the window has changed, go back to that // window, if it still exists. This avoids that ":edit x" triggering a // "tabnext" BufUnload autocmd leaves a window behind without a buffer. - if (is_curwin && curwin != the_curwin && win_valid_any_tab(the_curwin)) { + if (is_curwin && curwin != win && win_valid) { block_autocmds(); - goto_tabpage_win(the_curtab, the_curwin); + goto_tabpage_win(the_curtab, win); unblock_autocmds(); } - int nwindows = buf->b_nwindows; + // Remember if the buffer may be hidden soon, or is already hidden. + bool hiding_buf = buf->b_nwindows <= 0 + || (win_valid && win->w_buffer == buf && buf->b_nwindows == 1); - // decrease the link count from windows (unless not in any window) - if (buf->b_nwindows > 0) { - buf->b_nwindows--; - } - - if (diffopt_hiddenoff() && !unload_buf && buf->b_nwindows == 0) { + if (diffopt_hiddenoff() && !unload_buf && hiding_buf) { diff_buf_delete(buf); // Clear 'diff' for hidden buffer. } - // Return when a window is displaying the buffer or when it's not - // unloaded. - if (buf->b_nwindows > 0 || !unload_buf) { - return true; + // Return when another window is displaying the buffer or when not unloaded. + if (!hiding_buf || !unload_buf) { + return false; } // Always remove the buffer when there is no file name. @@ -681,52 +685,25 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i // Free all things allocated for this buffer. // Also calls the "BufDelete" autocommands when del_buf is true. - // Remember if we are closing the current buffer. Restore the number of - // windows, so that autocommands in buf_freeall() don't get confused. - bool is_curbuf = (buf == curbuf); - - buf->b_nwindows = nwindows; - - buf_freeall(buf, ((del_buf ? BFA_DEL : 0) - + (wipe_buf ? BFA_WIPE : 0) - + (ignore_abort ? BFA_IGNORE_ABORT : 0))); - - if (!bufref_valid(&bufref)) { - // Autocommands may have deleted the buffer. - return false; - } - // autocmds may abort script processing. - if (!ignore_abort && aborting()) { - return false; - } - - // It's possible that autocommands change curbuf to the one being deleted. - // This might cause the previous curbuf to be deleted unexpectedly. But - // in some cases it's OK to delete the curbuf, because a new one is - // obtained anyway. Therefore only return if curbuf changed to the - // deleted buffer. - if (buf == curbuf && !is_curbuf) { + // Abort if nothing was freed, or if autocommands delete the buffer. + if (!buf_freeall(buf, ((del_buf ? BFA_DEL : 0) + + (wipe_buf ? BFA_WIPE : 0) + + (ignore_abort ? BFA_IGNORE_ABORT : 0)))) { return false; } bool clear_w_buf = false; - if (win != NULL // Avoid bogus clang warning. - && win_valid_any_tab(win) - && win->w_buffer == buf) { + win_valid = win_valid && win_valid_any_tab(win); + if (win_valid && win->w_buffer == buf) { + // Autocommands may have opened (despite b_locked_split) or closed + // windows for this buffer. Decrement for the close we do here. + buf->b_nwindows--; // Defer clearing w_buffer until after operations that may invoke dict // watchers (e.g., buf_clear_file()), so callers like tabpagebuflist() - // never see a window in the winlist with a NULL buffer. + // never see a window in the window list with a NULL buffer. clear_w_buf = true; } - // Autocommands may have opened or closed windows for this buffer. - // Decrement the count for the close we do here. - // Don't decrement b_nwindows if the buffer wasn't displayed in any window - // before calling buf_freeall(). - if (nwindows > 0 && buf->b_nwindows > 0) { - buf->b_nwindows--; - } - // Remove the buffer from the list. // Do not wipe out the buffer if it is used in a window, or if autocommands // wiped out all other buffers (unless when inside free_all_mem() where all @@ -771,7 +748,10 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i // Init the options when loaded again. buf->b_p_initialized = false; } + // Dict watchers set b_locked, but also don't want them messing with windows... + textlock++; buf_clear_file(buf); + textlock--; if (clear_w_buf) { win->w_buffer = NULL; } @@ -780,6 +760,11 @@ bool close_buffer(win_T *win, buf_T *buf, int action, bool abort_if_last, bool i } } // NOTE: at this point "curbuf" may be invalid! + + // When closing curbuf for curwin, is_curwin checks should've ensured + // autocmds don't switch windows, unless they closed curwin. Otherwise + // callers may leave the window open to a NULL buffer! + assert(!win_valid || !is_curwin || win == curwin); return true; } @@ -816,7 +801,9 @@ void buf_clear(void) /// BFA_WIPE buffer is going to be wiped out /// BFA_KEEP_UNDO do not free undo information /// BFA_IGNORE_ABORT don't abort even when aborting() returns true -void buf_freeall(buf_T *buf, int flags) +/// @return true when we got to the end. +bool buf_freeall(buf_T *buf, int flags) + FUNC_ATTR_NONNULL_ALL { bool is_curbuf = (buf == curbuf); int is_curwin = (curwin != NULL && curwin->w_buffer == buf); @@ -840,21 +827,21 @@ void buf_freeall(buf_T *buf, int flags) && apply_autocmds(EVENT_BUFUNLOAD, buf->b_fname, buf->b_fname, false, buf) && !bufref_valid(&bufref)) { // Autocommands deleted the buffer. - return; + return false; } if ((flags & BFA_DEL) && buf->b_p_bl && apply_autocmds(EVENT_BUFDELETE, buf->b_fname, buf->b_fname, false, buf) && !bufref_valid(&bufref)) { // Autocommands may delete the buffer. - return; + return false; } if ((flags & BFA_WIPE) && apply_autocmds(EVENT_BUFWIPEOUT, buf->b_fname, buf->b_fname, false, buf) && !bufref_valid(&bufref)) { // Autocommands may delete the buffer. - return; + return false; } buf->b_locked--; buf->b_locked_split--; @@ -869,7 +856,7 @@ void buf_freeall(buf_T *buf, int flags) } // autocmds may abort script processing if ((flags & BFA_IGNORE_ABORT) == 0 && aborting()) { - return; + return false; } // It's possible that autocommands change curbuf to the one being deleted. @@ -877,7 +864,7 @@ void buf_freeall(buf_T *buf, int flags) // it's OK to delete the curbuf, because a new one is obtained anyway. // Therefore only return if curbuf changed to the deleted buffer. if (buf == curbuf && !is_curbuf) { - return; + return false; } // If curbuf, stop Visual mode just before freeing, but after autocmds that may restart it. @@ -918,6 +905,7 @@ void buf_freeall(buf_T *buf, int flags) } syntax_clear(&buf->b_s); // reset syntax info buf->b_flags &= ~BF_READERR; // a read error is no longer relevant + return true; } /// Free a buffer structure and the things it contains related to the buffer @@ -1077,11 +1065,11 @@ void handle_swap_exists(bufref_T *old_curbuf) // open a new, empty buffer. swap_exists_action = SEA_NONE; // don't want it again swap_exists_did_quit = true; - close_buffer(curwin, curbuf, DOBUF_UNLOAD, false, false); + close_buffer(curwin, curbuf, DOBUF_UNLOAD, false, false, true); if (old_curbuf == NULL || !bufref_valid(old_curbuf) || old_curbuf->br_buf == curbuf) { - // Block autocommands here because curwin->w_buffer is NULL. + // Block autocommands here because curwin->w_buffer may be NULL. block_autocmds(); buf = buflist_new(NULL, NULL, 1, BLN_CURBUF | BLN_LISTED); unblock_autocmds(); @@ -1261,7 +1249,7 @@ static int empty_curbuf(bool close_others, int forceit, int action) // the old one. But do_ecmd() may have done that already, check // if the buffer still exists. if (buf != curbuf && bufref_valid(&bufref) && buf->b_nwindows == 0) { - close_buffer(NULL, buf, action, false, false); + close_buffer(NULL, buf, action, false, false, false); } if (!close_others) { @@ -1466,7 +1454,7 @@ static int do_buffer_ext(int action, int start, int dir, int count, int flags) close_windows(buf, false); if (buf != curbuf && bufref_valid(&bufref) && buf->b_nwindows <= 0) { - close_buffer(NULL, buf, action, false, false); + close_buffer(NULL, buf, action, false, false, false); } return OK; } @@ -1667,7 +1655,6 @@ void set_curbuf(buf_T *buf, int action, bool update_jumplist) int unload = (action == DOBUF_UNLOAD || action == DOBUF_DEL || action == DOBUF_WIPE); OptInt old_tw = curbuf->b_p_tw; - const int last_winid = get_last_winid(); if (update_jumplist) { setpcmark(); @@ -1687,7 +1674,6 @@ void set_curbuf(buf_T *buf, int action, bool update_jumplist) bufref_T prevbufref; set_bufref(&prevbufref, prevbuf); set_bufref(&newbufref, buf); - const int prev_nwindows = prevbuf->b_nwindows; // Autocommands may delete the current buffer and/or the buffer we want to // go to. In those cases don't close the buffer. @@ -1697,33 +1683,23 @@ void set_curbuf(buf_T *buf, int action, bool update_jumplist) if (prevbuf == curwin->w_buffer) { reset_synblock(curwin); } - // autocommands may have opened a new window - // with prevbuf, grr - if (unload - || (prev_nwindows <= 1 && last_winid != get_last_winid() - && action == DOBUF_GOTO && !buf_hide(prevbuf))) { + if (unload) { close_windows(prevbuf, false); } if (bufref_valid(&prevbufref) && !aborting()) { - win_T *previouswin = curwin; - // Do not sync when in Insert mode and the buffer is open in // another window, might be a timer doing something in another // window. if (prevbuf == curbuf && ((State & MODE_INSERT) == 0 || curbuf->b_nwindows <= 1)) { u_sync(false); } - close_buffer(prevbuf == curwin->w_buffer ? curwin : NULL, + close_buffer(curwin, prevbuf, unload ? action : (action == DOBUF_GOTO && !buf_hide(prevbuf) && !bufIsChanged(prevbuf)) ? DOBUF_UNLOAD : 0, - false, false); - if (curwin != previouswin && win_valid(previouswin)) { - // autocommands changed curwin, Grr! - curwin = previouswin; - } + false, false, true); } } // An autocommand may have deleted "buf", already entered it (e.g., when @@ -1731,11 +1707,6 @@ void set_curbuf(buf_T *buf, int action, bool update_jumplist) // If curwin->w_buffer is null, enter_buffer() will make it valid again bool valid = buf_valid(buf); if ((valid && buf != curbuf && !aborting()) || curwin->w_buffer == NULL) { - // autocommands changed curbuf and we will move to another - // buffer soon, so decrement curbuf->b_nwindows - if (curbuf != NULL && prevbuf != curbuf) { - curbuf->b_nwindows--; - } // If the buffer is not valid but curwin->w_buffer is NULL we must // enter some buffer. Using the last one is hopefully OK. enter_buffer(valid ? buf : lastbuf); @@ -1764,6 +1735,10 @@ static void enter_buffer(buf_T *buf) end_visual_mode(); } + if (curwin->w_buffer != NULL) { + curwin->w_buffer->b_nwindows--; + } + // Get the buffer in the current window. curwin->w_buffer = buf; curbuf = buf; @@ -3091,7 +3066,7 @@ int setfname(buf_T *buf, char *ffname_arg, char *sfname_arg, bool message) return FAIL; } // delete from the list - close_buffer(NULL, obuf, DOBUF_WIPE, false, false); + close_buffer(NULL, obuf, DOBUF_WIPE, false, false, false); } sfname = xstrdup(sfname); #ifdef CASE_INSENSITIVE_FILENAME @@ -4189,7 +4164,7 @@ void wipe_buffer(buf_T *buf, bool aucmd) // Don't trigger BufDelete autocommands here. block_autocmds(); } - close_buffer(NULL, buf, DOBUF_WIPE, false, true); + close_buffer(NULL, buf, DOBUF_WIPE, false, true, false); if (!aucmd) { unblock_autocmds(); } diff --git a/src/nvim/ex_cmds.c b/src/nvim/ex_cmds.c index d983f05629..875650eafa 100644 --- a/src/nvim/ex_cmds.c +++ b/src/nvim/ex_cmds.c @@ -2498,12 +2498,6 @@ int do_ecmd(int fnum, char *ffname, char *sfname, exarg_T *eap, linenr_T newlnum // autocommands try to edit a closing buffer, which like splitting, can // result in more windows displaying it; abort if (buf->b_locked_split) { - // window was split, but not editing the new buffer, reset b_nwindows again - if (oldwin == NULL - && curwin->w_buffer != NULL - && curwin->w_buffer->b_nwindows > 1) { - curwin->w_buffer->b_nwindows--; - } emsg(_(e_cannot_switch_to_a_closing_buffer)); goto theend; } @@ -2591,7 +2585,6 @@ int do_ecmd(int fnum, char *ffname, char *sfname, exarg_T *eap, linenr_T newlnum auto_buf = true; } else { win_T *the_curwin = curwin; - buf_T *was_curbuf = curbuf; // Set w_locked to avoid that autocommands close the window. // Set b_locked for the same reason. @@ -2602,15 +2595,14 @@ int do_ecmd(int fnum, char *ffname, char *sfname, exarg_T *eap, linenr_T newlnum buf_copy_options(buf, BCO_ENTER); } - // Close the link to the current buffer. This will set - // oldwin->w_buffer to NULL. + // Close the link to the current buffer. This may set + // curwin->w_buffer to NULL. u_sync(false); - const bool did_decrement - = close_buffer(oldwin, curbuf, - (flags & ECMD_HIDE) - || (curbuf->terminal && terminal_running(curbuf->terminal)) - ? 0 : DOBUF_UNLOAD, - false, false); + close_buffer(curwin, curbuf, + (flags & ECMD_HIDE) + || (curbuf->terminal && terminal_running(curbuf->terminal)) + ? 0 : DOBUF_UNLOAD, + false, false, oldwin != NULL); // Autocommands may have closed the window. if (win_valid(the_curwin)) { @@ -2626,20 +2618,17 @@ int do_ecmd(int fnum, char *ffname, char *sfname, exarg_T *eap, linenr_T newlnum } // Be careful again, like above. if (!bufref_valid(&au_new_curbuf)) { - // New buffer has been deleted. - delbuf_msg(new_name); // Frees new_name. - au_new_curbuf = save_au_new_curbuf; - goto theend; + // New buffer was deleted. If curwin->w_buffer is NULL, we + // must enter some buffer. Hopefully the last one is OK. + if (curwin->w_buffer == NULL) { + buf = lastbuf; + } else { + delbuf_msg(new_name); // Frees new_name. + au_new_curbuf = save_au_new_curbuf; + goto theend; + } } if (buf == curbuf) { // already in new buffer - // close_buffer() has decremented the window count, - // increment it again here and restore w_buffer. - if (did_decrement && buf_valid(was_curbuf)) { - was_curbuf->b_nwindows++; - } - if (win_valid_any_tab(oldwin) && oldwin->w_buffer == NULL) { - oldwin->w_buffer = was_curbuf; - } auto_buf = true; } else { // We could instead free the synblock @@ -2649,6 +2638,10 @@ int do_ecmd(int fnum, char *ffname, char *sfname, exarg_T *eap, linenr_T newlnum curwin->w_s = &(buf->b_s); } + if (curwin->w_buffer != NULL) { + curwin->w_buffer->b_nwindows--; + } + curwin->w_buffer = buf; curbuf = buf; curbuf->b_nwindows++; diff --git a/src/nvim/ex_getln.c b/src/nvim/ex_getln.c index 05d424ceb2..ce61832dd9 100644 --- a/src/nvim/ex_getln.c +++ b/src/nvim/ex_getln.c @@ -4611,7 +4611,7 @@ static int open_cmdwin(void) } // win_close() autocommands may have already deleted the buffer. if (newbuf_status == OK && bufref_valid(&bufref) && bufref.br_buf != curbuf) { - close_buffer(NULL, bufref.br_buf, DOBUF_WIPE, false, false); + close_buffer(NULL, bufref.br_buf, DOBUF_WIPE, false, false, false); } cmdwin_type = 0; @@ -4796,7 +4796,7 @@ static int open_cmdwin(void) // win_close() may have already wiped the buffer when 'bh' is // set to 'wipe', autocommands may have closed other windows if (bufref_valid(&bufref) && bufref.br_buf != curbuf) { - close_buffer(NULL, bufref.br_buf, DOBUF_WIPE, false, false); + close_buffer(NULL, bufref.br_buf, DOBUF_WIPE, false, false, false); } // Restore window sizes. diff --git a/src/nvim/memory.c b/src/nvim/memory.c index 50062122e8..cc222f4259 100644 --- a/src/nvim/memory.c +++ b/src/nvim/memory.c @@ -957,7 +957,7 @@ void free_all_mem(void) // callbacks are called, so free them before closing the buffer. buf_free_callbacks(buf); - close_buffer(NULL, buf, DOBUF_WIPE, false, false); + close_buffer(NULL, buf, DOBUF_WIPE, false, false, false); // Didn't work, try next one. buf = bufref_valid(&bufref) ? nextbuf : firstbuf; } diff --git a/src/nvim/quickfix.c b/src/nvim/quickfix.c index f7eae6f825..fbbe439455 100644 --- a/src/nvim/quickfix.c +++ b/src/nvim/quickfix.c @@ -1777,11 +1777,11 @@ static void wipe_qf_buffer(qf_info_T *qi) buf_T *const qfbuf = buflist_findnr(qi->qf_bufnr); if (qfbuf != NULL && qfbuf->b_nwindows == 0) { bool buf_was_null = false; - // can happen when curwin is going to be closed e.g. curwin->w_buffer - // was already closed in win_close(), and we are now closing the - // window related location list buffer from win_free_mem() - // but close_buffer() calls CHECK_CURBUF() macro and requires - // curwin->w_buffer == curbuf + // Can happen when curwin is closing (e.g: w_buffer was unloaded in + // win_close()) and we are now closing the window-related location list + // buffer from win_free(). close_buffer() calls CHECK_CURBUF() and + // requires curwin->w_buffer == curbuf. Should be OK to not increment + // b_nwindows, especially as autocmds are blocked in win_free(). if (curwin->w_buffer == NULL) { curwin->w_buffer = curbuf; buf_was_null = true; @@ -1789,7 +1789,7 @@ static void wipe_qf_buffer(qf_info_T *qi) // If the quickfix buffer is not loaded in any window, then // wipe the buffer. - close_buffer(NULL, qfbuf, DOBUF_WIPE, false, false); + close_buffer(NULL, qfbuf, DOBUF_WIPE, false, false, false); qi->qf_bufnr = INVALID_QFBUFNR; if (buf_was_null) { curwin->w_buffer = NULL; @@ -6079,7 +6079,7 @@ static void unload_dummy_buffer(buf_T *buf, char *dirname_start) return; } - close_buffer(NULL, buf, DOBUF_UNLOAD, false, true); + close_buffer(NULL, buf, DOBUF_UNLOAD, false, true, false); // When autocommands/'autochdir' option changed directory: go back. restore_start_dir(dirname_start); diff --git a/src/nvim/window.c b/src/nvim/window.c index 4df00525e1..bc19c82d86 100644 --- a/src/nvim/window.c +++ b/src/nvim/window.c @@ -2757,7 +2757,7 @@ static bool close_last_window_tabpage(win_T *win, bool free_buf, tabpage_T *prev /// "abort_if_last" is passed to close_buffer(): abort closing if all other /// windows are closed. /// -/// @return whether close_buffer() decremented b_nwindows +/// @return @see close_buffer(). static bool win_close_buffer(win_T *win, int action, bool abort_if_last) FUNC_ATTR_NONNULL_ALL { @@ -2779,7 +2779,7 @@ static bool win_close_buffer(win_T *win, int action, bool abort_if_last) bufref_T bufref; set_bufref(&bufref, curbuf); win->w_locked = true; - retval = close_buffer(win, win->w_buffer, action, abort_if_last, true); + retval = close_buffer(win, win->w_buffer, action, abort_if_last, true, true); if (win_valid_any_tab(win)) { win->w_locked = false; } @@ -2798,8 +2798,7 @@ static bool win_close_buffer(win_T *win, int action, bool abort_if_last) /// call this to make the window have a buffer again. /// /// @param bufref reference to win->w_buffer before calling close_buffer() -/// @param did_decrement whether close_buffer() decremented b_nwindows -static void win_unclose_buffer(win_T *win, bufref_T *bufref, bool did_decrement) +static void win_unclose_buffer(win_T *win, bufref_T *bufref) { if (win->w_buffer == NULL) { // If the buffer was removed from the window we have to give it any buffer. @@ -2809,10 +2808,6 @@ static void win_unclose_buffer(win_T *win, bufref_T *bufref, bool did_decrement) curbuf = curwin->w_buffer; } win_init_empty(win); - } else if (did_decrement && win->w_buffer == bufref->br_buf && bufref_valid(bufref)) { - // close_buffer() decremented the window count, but we're keeping the window. - // As the window is still viewing the buffer, increment the count. - win->w_buffer->b_nwindows++; } } @@ -2950,7 +2945,7 @@ int win_close(win_T *win, bool free_buf, bool force) bufref_T bufref; set_bufref(&bufref, win->w_buffer); - bool did_decrement = win_close_buffer(win, free_buf ? DOBUF_UNLOAD : 0, true); + win_close_buffer(win, free_buf ? DOBUF_UNLOAD : 0, true); if (win_valid(win) && win->w_buffer == NULL && !win->w_floating && last_window(win)) { @@ -2978,7 +2973,7 @@ int win_close(win_T *win, bool free_buf, bool force) if (first_tabpage->tp_next != NULL) { emsg(e_floatonly); } - win_unclose_buffer(win, &bufref, did_decrement); + win_unclose_buffer(win, &bufref); return FAIL; } if (close_last_window_tabpage(win, free_buf, prev_curtab)) { @@ -3012,6 +3007,10 @@ int win_close(win_T *win, bool free_buf, bool force) // which may have changed since the last set_bufref. (e.g: close_buffer autocmds) set_bufref(&bufref, win->w_buffer); + if (win->w_buffer != NULL) { + win->w_buffer->b_nwindows--; + } + // Free the memory used for the window and get the window that received // the screen space. int dir; @@ -3189,7 +3188,6 @@ bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force) FUNC_ATTR_NONNULL_ALL { assert(tp != curtab); - bool did_decrement = false; // Commands that may call win_close_othertab() already check this, but // check here again just in case. @@ -3251,7 +3249,7 @@ bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force) if (win->w_buffer != NULL) { // Close the link to the buffer. - did_decrement = close_buffer(win, win->w_buffer, free_buf ? DOBUF_UNLOAD : 0, false, true); + close_buffer(win, win->w_buffer, free_buf ? DOBUF_UNLOAD : 0, false, true, true); } // Careful: Autocommands may have closed the tab page or made it the @@ -3302,6 +3300,10 @@ bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force) // which may have changed since the last set_bufref. (e.g: close_buffer autocmds) set_bufref(&bufref, win->w_buffer); + if (win->w_buffer != NULL) { + win->w_buffer->b_nwindows--; + } + // Free the memory used for the window. int dir; win_free_mem(win, &dir, tp); @@ -3323,7 +3325,7 @@ bool win_close_othertab(win_T *win, int free_buf, tabpage_T *tp, bool force) leave_open: if (win_valid_any_tab(win)) { - win_unclose_buffer(win, &bufref, did_decrement); + win_unclose_buffer(win, &bufref); } return false; } diff --git a/test/functional/ex_cmds/dict_notifications_spec.lua b/test/functional/ex_cmds/dict_notifications_spec.lua index 30d066a058..076b9a6542 100644 --- a/test/functional/ex_cmds/dict_notifications_spec.lua +++ b/test/functional/ex_cmds/dict_notifications_spec.lua @@ -6,10 +6,12 @@ local clear, source = n.clear, n.source local api = n.api local insert = n.insert local eq, next_msg = t.eq, n.next_msg +local matches = t.matches local exc_exec = n.exc_exec local exec_lua = n.exec_lua local command = n.command local eval = n.eval +local pcall_err = t.pcall_err describe('Vimscript dictionary notifications', function() local channel @@ -433,6 +435,13 @@ describe('Vimscript dictionary notifications', function() insert('t') eq('E937: Attempt to delete a buffer that is in use: [No Name]', api.nvim_get_vvar('errmsg')) assert_alive() + + command([[enew | set modified | call dictwatcheradd(b:, 'changedtick', {-> execute('split')})]]) + -- Used to instead leave a window open to a NULL buffer. + matches( + 'E565: Not allowed to change text or change window: split$', + pcall_err(command, 'bdelete!') + ) end) it('does not cause use-after-free when unletting from callback', function() diff --git a/test/old/testdir/test_autocmd.vim b/test/old/testdir/test_autocmd.vim index c6bc608204..2c4808434b 100644 --- a/test/old/testdir/test_autocmd.vim +++ b/test/old/testdir/test_autocmd.vim @@ -4318,10 +4318,11 @@ func Test_autocmd_creates_new_window_on_bufleave() setlocal bufhidden=wipe autocmd BufLeave diffsplit c.txt bn - call assert_equal(1, winnr('$')) + " curbuf set for the new split opened for c.txt, due to BufLeave + call assert_equal(2, winnr('$')) call assert_equal('a.txt', bufname('%')) - bw a.txt - bw c.txt + call assert_equal('b.txt', bufname('#')) + %bw! endfunc " Ensure `expected` was just recently written as a Vim session @@ -4620,6 +4621,32 @@ func Test_autocmd_BufWinLeave_with_vsp() exe "bw! " .. dummy endfunc +func Test_autocmd_BufWinLeave_with_vsp2() + edit Xfoo + split Xbar + split + let s:fired = 0 + augroup testing + autocmd! + autocmd BufWinLeave Xfoo ++once ++nested + \ execute 'autocmd WinEnter * ++once let s:fired = 1' + \ .. '| call assert_equal(3, win_findbuf(bufnr(''Xbar''))->len())' + \ .. '| quit' + \| call assert_fails('vsplit Xfoo', 'E1546:') + augroup END + bw Xfoo + call assert_equal(1, s:fired) + " After 9.1.0764, Xbar's b_nwindows would be 0 if autocmds closed the new + " split before E1546, causing it to be unloaded despite being in a window. + call assert_equal(0, bufexists('Xfoo')) + call assert_equal(1, win_findbuf(bufnr('Xbar'))->len()) + call assert_equal(1, bufloaded('Xbar')) + + call CleanUpTestAuGroup() + unlet! s:fired + %bw! +endfunc + func Test_OptionSet_cmdheight() set mouse=a laststatus=2 au OptionSet cmdheight :let &l:ch = v:option_new @@ -5203,4 +5230,104 @@ func Test_win_tabclose_autocmd() bw! endfunc +func Test_buffer_b_nwindows() + " In these cases, b_nwindows of the Xbars was 1 despite being in no windows. + " Would cause weird failures in other tests, as they would be un-deletable. + edit Xfoo1 + augroup testing + autocmd! + autocmd BufUnload * ++once edit Xbar1 + augroup END + bdelete + call assert_equal([], win_findbuf(bufnr('Xfoo1'))) + call assert_equal([], win_findbuf(bufnr('Xbar1'))) + call assert_equal(1, bufexists('Xfoo1')) + call assert_equal(1, bufexists('Xbar1')) + %bw! + call assert_equal(0, bufexists('Xfoo1')) + call assert_equal(0, bufexists('Xbar1')) + + split Xbar2 + enew + augroup testing + autocmd! + autocmd BufWinLeave * ++once buffer Xbar2 + augroup END + quit + call assert_equal([], win_findbuf(bufnr('Xbar2'))) + call assert_equal(1, bufexists('Xbar2')) + %bw! + call assert_equal(0, bufexists('Xbar2')) + + edit Xbar3 + enew + setlocal bufhidden=hide + let s:win = win_getid() + tabnew + augroup testing + autocmd! + autocmd BufHidden * ++once call win_execute(s:win, 'buffer Xbar3') + augroup END + tabonly + call assert_equal([], win_findbuf(bufnr('Xbar3'))) + call assert_equal(1, bufexists('Xbar3')) + %bw! + call assert_equal(0, bufexists('Xbar3')) + unlet! s:win + + edit Xbar4 + split Xfoo4 + augroup testing + autocmd! + autocmd BufWinLeave * ++once call assert_equal('Xfoo4', bufname()) + \| edit Xbar4 + augroup END + edit Xbar4 + call assert_equal(0, bufloaded('Xfoo4')) + call assert_equal(1, bufexists('Xfoo4')) + " After 8.2.2354, Xfoo4 wrongly had b_nwindows of 1, so couldn't be wiped. + call assert_equal([], win_findbuf('Xfoo4')) + %bw! + call assert_equal(0, bufexists('Xfoo4')) + + call CleanUpTestAuGroup() + %bw! +endfunc + +" Test that an autocmd triggered by v:swapchoice == 'q' that switches buffers +" doesn't cause b_nwindows to be wrong. +func Test_SwapExists_b_nwindows() + let lines =<< trim END + set nocompatible directory=. + + let g:buf = bufnr() + new + + func SwapExists() + let v:swapchoice = 'q' + autocmd BufWinLeave * ++nested ++once buffer Xfoo + endfunc + + func SafeState() + edit Xfoo + edit