patch 9.2.0253: various issues with wrong b_nwindows after closing buffers
Commit:
https://github.com/vim/vim/commit/bf21df1c7bc772e3a29961c961d0821584d50ee0
Author: Sean Dewar <[email protected]>
Date: Thu Mar 26 20:16:09 2026 +0000
patch 9.2.0253: various issues with wrong b_nwindows after closing buffers
Problem: close_buffer() callers incorrectly handle b_nwindows,
especially after nasty autocmds, allowing it to go
out-of-sync. May lead to buffers that can't be unloaded, or
buffers that are prematurely freed whilst displayed.
Solution: Modify close_buffer() and review its callers; let them
decrement b_nwindows if it didn't unload the buffer. Remove
some now unneeded workarounds like 8.2.2354, 9.1.0143,
9.1.0764, which didn't always work (Sean Dewar)
close_buffer() now doesn't decrement b_nwindows when not unloading buf, or
when
buf isn't w_buffer after autocmds (they would've already decremented it).
Callers are now expected to decrement b_nwindows if w_buffer is not NULL
after
close_buffer(), and when still intending to switch buffers or close win,
for two
reasons:
- close_buffer() autocmds may have switched buffers. The new w_buffer's
b_nwindows would also need decrementing.
- After close_buffer(), callers may opt to not switch w_buffer or close win.
b_nwindows would need to be incremented again. (unless w_buffer is NULL
from
being unloaded; callers are already forced to find a new buffer then)
These were the main causes of b_nwindows bugs, as these cases could not be
reliably detected, and went largely unhandled.
NOTE: if close_buffer() autocmds switch buffers, close_buffer() is not
called
for that new buffer before decrementing b_nwindows. This may skip
side-effects
like from 'bufhidden', but I think it's mostly harmless, and was already
happening in other places.
Let's see how this goes... Other details: (I have lots to say!)
It's OK to pass a win to close_buffer() that isn't showing buf (used by
set_curbuf()). In that case, we skip some side-effects and don't decrement
b_nwindows, but may still unload buf if hidden.
buf_freeall() now returns whether it freed anything. Removes some repeated
checks in close_buffer().
Preserve close_buffer()'s behaviour when called by win_free_popup() after
its
popup was already removed from the window list. This made
win_valid_any_tab()
return FALSE, so we skip things that originally checked it in that case.
Add "set_context" to close_buffer() to preserve do_ecmd()'s behaviour of
only
setting b_last_cursor and/or calling buflist_setfpos() when not splitting
(see 7.2.041:
https://groups.google.com/g/vim_dev/c/ZGgNvaylNzI/m/WHxjhnuxqB0J)
Without this, Test_marks_cmd() fails from its ' mark differing. Don't use
oldwin though; it's not always the window with the closed buf, especially
after BufLeave autocmds in do_ecmd(). Also, only set context if win is
really
displaying buf.
Don't bail in do_ecmd() if buf was deleted but curwin->w_buffer is NULL;
that
leaves curwin open to a NULL buffer! Use lastbuf instead, like
set_curbuf().
I don't think it's possible for buf to be deleted by close_buffer() anyway,
as
b_locked was set (which I can't see a way to bypass, unlike b_locked_split).
Maybe such checks can be removed, but I'd rather not risk that here.
Don't set curwin to previouswin in set_curbuf(); shouldn't be needed,
otherwise
may lead to curbuf != curwin->w_buffer if autocmds switch to a window
showing
buf, as that skips enter_buffer()? Was introduced back in 7.3.557 to avoid
cases where autocmds switch windows, possibly leaving previouswin with a
NULL
buffer. Since 7.4.2312 and 7.4.2328, close_buffer() and buf_freeall()
already
handles this. I've added an assert() as a sanity check anyway.
In free_all_mem(), set b_nwindows to 0 before close_buffer() so buffers can
be
wiped if still in a window before win_free_all(). Needed as close_buffer()
now
skips unloading buffers that aren't hidden if win is NULL. If it's
possible for
free_all_mem()'s :tabonly! and :only! to not close all windows before
freeing,
then this issue was also previously possible if b_nwindows > 1.
related: #19728
Signed-off-by: Sean Dewar <[email protected]>
Signed-off-by: Christian Brabandt <[email protected]>
diff --git a/src/alloc.c b/src/alloc.c
index 7baf02ecb..0fa59ad6d 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -496,7 +496,9 @@ free_all_mem(void)
set_bufref(&bufref, buf);
nextbuf = buf->b_next;
- close_buffer(NULL, buf, DOBUF_WIPE, FALSE, FALSE);
+ // All windows were freed. Reset b_nwindows so buffers can be wiped.
+ buf->b_nwindows = 0;
+ close_buffer(NULL, buf, DOBUF_WIPE, FALSE, FALSE, FALSE);
if (bufref_valid(&bufref))
buf = nextbuf; // didn't work, try next one
else
diff --git a/src/buffer.c b/src/buffer.c
index dd7285da0..b624abb8a 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -227,7 +227,7 @@ open_buffer(
{
// 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);
FOR_ALL_BUFFERS(curbuf)
if (curbuf->b_ml.ml_mfp != NULL)
break;
@@ -545,36 +545,52 @@ can_unload_buffer(buf_T *buf)
* DOBUF_DEL buffer is unloaded and removed from buffer list
* DOBUF_WIPE buffer is unloaded and really deleted
* DOBUF_WIPE_REUSE idem, and add to buf_reuse list
- * 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.
*
+ * When "win" is 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.
+ *
* When "abort_if_last" is TRUE then do not close the buffer if autocommands
* cause there to be only one window with this buffer. e.g. when ":quit" is
* supposed to close the window but autocommands close all other windows.
*
* When "ignore_abort" is TRUE don't abort even when aborting() returns TRUE.
*
- * Return TRUE when we got to the end and b_nwindows was decremented.
+ * When "set_context" is 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.
+ * Not done for closing popups.
+ *
+ * Return TRUE when we got to the end and unloaded "buf".
*/
int
close_buffer(
- win_T *win, // if not NULL, set b_last_cursor
+ win_T *win,
buf_T *buf,
int action,
int abort_if_last,
- int ignore_abort)
+ int ignore_abort,
+ int set_context)
{
- int is_curbuf;
- int nwindows;
+ int hiding_buf;
bufref_T bufref;
- int is_curwin = (curwin != NULL && curwin->w_buffer == buf);
- win_T *the_curwin = curwin;
+ int is_curwin = (curwin != NULL && curwin == win
+ && curwin->w_buffer == buf);
tabpage_T *the_curtab = curtab;
int unload_buf = (action != 0);
int wipe_buf = (action == DOBUF_WIPE || action ==
DOBUF_WIPE_REUSE);
int del_buf = (action == DOBUF_DEL || wipe_buf);
+ int win_valid = win_valid_any_tab(win);
+ // win_valid is FALSE for closing popups, as they're first removed from the
+ // window list. If so, skip some side-effects, but treat win as valid.
+ // Shouldn't be possible for autocmds to prematurely free win then, as it's
+ // not in the window list.
+ int closed_popup = win && !win_valid && WIN_IS_POPUP(win);
CHECK_CURBUF;
@@ -649,8 +665,7 @@ close_buffer(
if ((del_buf || wipe_buf) && !can_unload_buffer(buf))
return FALSE;
- // check no autocommands closed the window
- if (win != NULL && 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.
@@ -666,7 +681,8 @@ close_buffer(
set_bufref(&bufref, buf);
// When the buffer is no longer in a window, trigger BufWinLeave
- if (buf->b_nwindows == 1)
+ if ((win_valid || closed_popup) && win->w_buffer == buf
+ && buf->b_nwindows == 1)
{
++buf->b_locked;
++buf->b_locked_split;
@@ -707,32 +723,30 @@ aucmd_abort:
if (!ignore_abort && aborting())
return FALSE;
#endif
+ 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();
}
- nwindows = buf->b_nwindows;
-
- // decrease the link count from windows (unless not in any window)
- if (buf->b_nwindows > 0)
- --buf->b_nwindows;
+ // Remember if the buffer may be hidden soon, or is already hidden.
+ hiding_buf = buf->b_nwindows <= 0 || ((win_valid || closed_popup)
+ && win->w_buffer == buf && buf->b_nwindows == 1);
#ifdef FEAT_DIFF
- 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.
#endif
- // Return when a window is displaying the buffer or when it's not
- // unloaded.
- if (buf->b_nwindows > 0 || !unload_buf)
+ // 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.
@@ -741,42 +755,20 @@ aucmd_abort:
// 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.
- is_curbuf = (buf == curbuf);
- buf->b_nwindows = nwindows;
-
- buf_freeall(buf, (del_buf ? BFA_DEL : 0)
+ // 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));
-
- // Autocommands may have deleted the buffer.
- if (!bufref_valid(&bufref))
+ + (ignore_abort ? BFA_IGNORE_ABORT : 0)))
return FALSE;
-#ifdef FEAT_EVAL
- // autocmds may abort script processing
- if (!ignore_abort && aborting())
- return FALSE;
-#endif
-
- // 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)
- return FALSE;
-
- if (win_valid_any_tab(win) && win->w_buffer == buf)
- win->w_buffer = NULL; // make sure we don't use the buffer now
- // 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)
+ win_valid = win_valid && win_valid_any_tab(win);
+ if ((win_valid || closed_popup) && 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;
+ win->w_buffer = NULL; // make sure we don't use the buffer now
+ }
/*
* Remove the buffer from the list.
@@ -839,6 +831,11 @@ aucmd_abort:
buf->b_p_bl = FALSE;
}
// 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;
}
@@ -872,8 +869,10 @@ buf_clear_file(buf_T *buf)
* 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
+ *
+ * Return TRUE when we got to the end.
*/
- void
+ int
buf_freeall(buf_T *buf, int flags)
{
int is_curbuf = (buf == curbuf);
@@ -892,7 +891,7 @@ buf_freeall(buf_T *buf, int flags)
FALSE, buf)
&& !bufref_valid(&bufref))
// autocommands deleted the buffer
- return;
+ return FALSE;
}
if ((flags & BFA_DEL) && buf->b_p_bl)
{
@@ -900,7 +899,7 @@ buf_freeall(buf_T *buf, int flags)
FALSE, buf)
&& !bufref_valid(&bufref))
// autocommands deleted the buffer
- return;
+ return FALSE;
}
if (flags & BFA_WIPE)
{
@@ -908,7 +907,7 @@ buf_freeall(buf_T *buf, int flags)
FALSE, buf)
&& !bufref_valid(&bufref))
// autocommands deleted the buffer
- return;
+ return FALSE;
}
--buf->b_locked;
--buf->b_locked_split;
@@ -926,7 +925,7 @@ buf_freeall(buf_T *buf, int flags)
#ifdef FEAT_EVAL
// autocmds may abort script processing
if ((flags & BFA_IGNORE_ABORT) == 0 && aborting())
- return;
+ return FALSE;
#endif
// It's possible that autocommands change curbuf to the one being deleted.
@@ -934,7 +933,7 @@ 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. May trigger TextYankPost, but with textlock set.
@@ -982,6 +981,7 @@ buf_freeall(buf_T *buf, int flags)
clear_buf_prop_types(buf);
#endif
buf->b_flags &= ~BF_READERR; // a read error is no longer relevant
+ return TRUE;
}
/*
@@ -1230,11 +1230,11 @@ 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, 1L, BLN_CURBUF | BLN_LISTED);
unblock_autocmds();
@@ -1321,7 +1321,7 @@ empty_curbuf(
// 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)
need_fileinfo = FALSE;
else if (retval == OK && !shortmess(SHM_FILEINFO))
@@ -1549,7 +1549,7 @@ do_buffer_ext(
{
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;
}
@@ -1882,7 +1882,6 @@ set_curbuf(buf_T *buf, int action)
bufref_T newbufref;
bufref_T prevbufref;
int valid;
- int last_winid = get_last_winid();
setpcmark();
if ((cmdmod.cmod_flags & CMOD_KEEPALT) == 0)
@@ -1896,7 +1895,6 @@ set_curbuf(buf_T *buf, int action)
prevbuf = curbuf;
set_bufref(&prevbufref, prevbuf);
set_bufref(&newbufref, buf);
- 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.
@@ -1912,11 +1910,7 @@ set_curbuf(buf_T *buf, int action)
if (prevbuf == curwin->w_buffer)
reset_synblock(curwin);
#endif
- // 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 defined(FEAT_EVAL)
if (bufref_valid(&prevbufref) && !aborting())
@@ -1924,22 +1918,17 @@ set_curbuf(buf_T *buf, int action)
if (bufref_valid(&prevbufref))
#endif
{
- 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, prevbuf,
+ 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
@@ -1952,10 +1941,6 @@ set_curbuf(buf_T *buf, int action)
#endif
) || 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.
if (!valid)
@@ -1987,6 +1972,9 @@ 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;
@@ -3665,7 +3653,7 @@ setfname(
return FAIL;
}
// delete from the list
- close_buffer(NULL, obuf, DOBUF_WIPE, FALSE, FALSE);
+ close_buffer(NULL, obuf, DOBUF_WIPE, FALSE, FALSE, FALSE);
}
sfname = vim_strsave(sfname);
if (ffname == NULL || sfname == NULL)
@@ -6438,7 +6426,7 @@ wipe_buffer(
if (!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/ex_cmds.c b/src/ex_cmds.c
index b131697f4..7dfb7176e 100644
--- a/src/ex_cmds.c
+++ b/src/ex_cmds.c
@@ -2984,12 +2984,6 @@ do_ecmd(
// 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;
}
@@ -3085,8 +3079,6 @@ do_ecmd(
else
{
win_T *the_curwin = curwin;
- int did_decrement;
- buf_T *was_curbuf = curbuf;
// Set the w_locked flag to avoid that autocommands close the
// window. And set b_locked for the same reason.
@@ -3096,11 +3088,12 @@ do_ecmd(
if (curbuf == old_curbuf.br_buf)
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);
- did_decrement = close_buffer(oldwin, curbuf,
- (flags & ECMD_HIDE) ? 0 : DOBUF_UNLOAD, FALSE, FALSE);
+ close_buffer(curwin, curbuf,
+ (flags & ECMD_HIDE) ? 0 : DOBUF_UNLOAD, FALSE, FALSE,
+ oldwin != NULL);
// Autocommands may have closed the window.
if (win_valid(the_curwin))
@@ -3119,21 +3112,19 @@ do_ecmd(
// 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
{
#ifdef FEAT_SYN_HL
@@ -3145,6 +3136,9 @@ do_ecmd(
|| curwin->w_s == &(curwin->w_buffer->b_s))
curwin->w_s = &(buf->b_s);
#endif
+ if (curwin->w_buffer != NULL)
+ --curwin->w_buffer->b_nwindows;
+
curwin->w_buffer = buf;
curbuf = buf;
++curbuf->b_nwindows;
diff --git a/src/ex_getln.c b/src/ex_getln.c
index 65bd8ccc8..97cc1601c 100644
--- a/src/ex_getln.c
+++ b/src/ex_getln.c
@@ -4785,7 +4785,7 @@ 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;
cmdwin_win = NULL;
@@ -5000,7 +5000,7 @@ 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.
win_size_restore(&winsizes);
diff --git a/src/proto/buffer.pro b/src/proto/buffer.pro
index db939b381..438ddb1a8 100644
--- a/src/proto/buffer.pro
+++ b/src/proto/buffer.pro
@@ -5,9 +5,9 @@ int open_buffer(int read_stdin, exarg_T *eap, int flags_arg);
void set_bufref(bufref_T *bufref, buf_T *buf);
int bufref_valid(bufref_T *bufref);
int buf_valid(buf_T *buf);
-int close_buffer(win_T *win, buf_T *buf, int action, int abort_if_last, int
ignore_abort);
+int close_buffer(win_T *win, buf_T *buf, int action, int abort_if_last, int
ignore_abort, int set_context);
void buf_clear_file(buf_T *buf);
-void buf_freeall(buf_T *buf, int flags);
+int buf_freeall(buf_T *buf, int flags);
void free_wininfo(wininfo_T *wip);
void goto_buffer(exarg_T *eap, int start, int dir, int count);
void handle_swap_exists(bufref_T *old_curbuf);
diff --git a/src/quickfix.c b/src/quickfix.c
index cffe635ea..ed82c6161 100644
--- a/src/quickfix.c
+++ b/src/quickfix.c
@@ -2071,11 +2071,11 @@ wipe_qf_buffer(qf_info_T *qi)
if (qfbuf != NULL && qfbuf->b_nwindows == 0)
{
int 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;
@@ -2084,7 +2084,7 @@ 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;
@@ -7142,7 +7142,7 @@ unload_dummy_buffer(buf_T *buf, char_u *dirname_start)
if (curbuf == buf) // safety check
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/testdir/test_autocmd.vim b/src/testdir/test_autocmd.vim
index 084dd7a5a..b18e29f05 100644
--- a/src/testdir/test_autocmd.vim
+++ b/src/testdir/test_autocmd.vim
@@ -4828,10 +4828,11 @@ func Test_autocmd_creates_new_window_on_bufleave()
setlocal bufhidden=wipe
autocmd BufLeave <buffer> 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
@@ -5228,6 +5229,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
@@ -5835,4 +5862,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 <script>
+ %bw!
+ call writefile([bufexists('Xfoo')], 'XnwindowsSwapExists.out')
+ qall!
+ endfunc
+
+ autocmd SwapExists * ++nested ++once call SwapExists()
+ autocmd SafeState * ++nested ++once call SafeState()
+ END
+ call writefile(lines, 'XnwindowsSwapExists.vim', 'D')
+
+ new XnwindowsSwapExists.vim
+ if RunVim('', '', ' -S XnwindowsSwapExists.vim')
+ call assert_equal(['0'], readfile('XnwindowsSwapExists.out'))
+ call delete('XnwindowsSwapExists.out')
+ endif
+
+ %bw!
+endfunc
+
" vim: shiftwidth=2 sts=2 expandtab
diff --git a/src/testdir/test_popupwin.vim b/src/testdir/test_popupwin.vim
index 83610a486..1951c06e9 100644
--- a/src/testdir/test_popupwin.vim
+++ b/src/testdir/test_popupwin.vim
@@ -5063,4 +5063,20 @@ func Test_popup_opacity_vsplit()
call StopVimInTerminal(buf)
endfunc
+func Test_popup_close_b_nwindows()
+ edit Xfoo
+ setlocal bufhidden=wipe
+ let &charconvert = 'win_execute(win_getid(1), ''call popup_clear()'') || 1'
+ let popup = popup_create(bufnr(), {})
+ edit Xbar
+ call assert_equal('Xfoo', winbufnr(popup)->bufname())
+ call assert_fails('call win_execute(popup, ''write ++enc=a b'')', ['E937:',
'E513:'])
+
+ set charconvert&
+ call popup_clear()
+ %bw!
+ " b_nwindows used to be 2 here, preventing Xfoo from being wiped.
+ call assert_equal(0, bufexists('Xfoo'))
+endfunc
+
" vim: shiftwidth=2 sts=2
diff --git a/src/testdir/test_visual.vim b/src/testdir/test_visual.vim
index a2bb12588..1ba407f05 100644
--- a/src/testdir/test_visual.vim
+++ b/src/testdir/test_visual.vim
@@ -3017,6 +3017,26 @@ func Test_visual_ended_in_unloaded_buffer()
call assert_equal('n', mode())
call assert_equal(1, s:fired)
+ augroup testing
+ autocmd!
+ autocmd BufHidden Xfoo ++once call assert_equal('Xfoo', bufname())
+ \| execute 'normal! v'
+ \| call assert_equal('v', mode())
+
+ " Check b_nwindows is not decremented too early when Visual mode ends in a
+ " loaded buffer. Buffer should not be considered hidden in TextYankPost,
as
+ " by that point it's still loaded and displayed in the current window.
+ autocmd TextYankPost * ++once let s:fired = 2
+ \| call assert_equal(1, bufloaded(bufnr()))
+ \| call assert_equal(0, getbufinfo(bufnr())[0].hidden)
+ augroup END
+ edit Xbar
+ edit Xfoo
+ only
+ hide buffer #
+ call assert_equal('n', mode())
+ call assert_equal(2, s:fired)
+
autocmd! testing
unlet! s:fired
set clipboard&
diff --git a/src/version.c b/src/version.c
index da34b93b8..5d352a98e 100644
--- a/src/version.c
+++ b/src/version.c
@@ -734,6 +734,8 @@ static char *(features[]) =
static int included_patches[] =
{ /* Add new patch number below this line */
+/**/
+ 253,
/**/
252,
/**/
diff --git a/src/window.c b/src/window.c
index b0f4574b3..0301dd5a6 100644
--- a/src/window.c
+++ b/src/window.c
@@ -2718,7 +2718,7 @@ win_close_buffer(win_T *win, int action, int
abort_if_last)
set_bufref(&bufref, curbuf);
win->w_locked = TRUE;
- close_buffer(win, win->w_buffer, action, abort_if_last, TRUE);
+ close_buffer(win, win->w_buffer, action, abort_if_last, TRUE, TRUE);
if (win_valid_any_tab(win))
win->w_locked = FALSE;
// Make sure curbuf is valid. It can become invalid if 'bufhidden' is
@@ -2902,6 +2902,9 @@ win_close(win_T *win, int free_buf)
++dont_parse_messages;
#endif
+ 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.
wp = win_free_mem(win, &dir, NULL);
@@ -3476,7 +3479,7 @@ win_close_othertab(win_T *win, int free_buf, tabpage_T
*tp)
if (win->w_buffer != NULL)
// Close the link to the buffer.
close_buffer(win, win->w_buffer, free_buf ? DOBUF_UNLOAD : 0,
- FALSE, TRUE);
+ FALSE, TRUE, TRUE);
// Careful: Autocommands may have closed the tab page or made it the
// current tab page.
@@ -3529,6 +3532,9 @@ win_close_othertab(win_T *win, int free_buf, tabpage_T
*tp)
shell_new_rows();
}
+ if (win->w_buffer != NULL)
+ --win->w_buffer->b_nwindows;
+
// Free the memory used for the window.
win_free_mem(win, &dir, tp);
@@ -6167,7 +6173,10 @@ win_free_popup(win_T *win)
if (bt_popup(win->w_buffer))
win_close_buffer(win, DOBUF_WIPE_REUSE, FALSE);
else
- close_buffer(win, win->w_buffer, 0, FALSE, FALSE);
+ close_buffer(win, win->w_buffer, 0, FALSE, FALSE, TRUE);
+
+ if (win->w_buffer != NULL)
+ --win->w_buffer->b_nwindows;
}
# if defined(FEAT_TIMERS)
// the timer may have been cleared, making the pointer invalid
--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php
---
You received this message because you are subscribed to the Google Groups
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion visit
https://groups.google.com/d/msgid/vim_dev/E1w5rLC-007PEH-2L%40256bit.org.
