mirrors/neovim
1
0
Fork 0
mirror of https://github.com/neovim/neovim.git synced 2026-04-02 05:39:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

vim-patch:9.2.0253: various issues with wrong b_nwindows after closing buffers

Browse Source 
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)

(endless yapping omitted)

related: vim/vim#19728

bf21df1c7b

b_nwindows = 0 change for free_all_mem() was already ported.

Originally Nvim returned true when b_nwindows was decremented before the end was
reached (to better indicate the decrement). That's not needed anymore, so just
return true only at the end, like Vim. (retval isn't used anywhere now anyways)

Set textlock for dict watchers at the end of close_buffer() to prevent them from
switching windows, as that can leave a window with a NULL buffer. (possible
before this PR, but the new assert catches it; added a test)

Despite textlock, things still aren't ideal, as watchers may observe the buffer
as unloaded and hidden (b_nwindows was decremented), yet still in a window...
Likewise, for Nvim, wipe_qf_buffer()'s comment may not be entirely accurate;
autocmds are blocked, but on_detach callbacks (textlocked) and dict watchers may
still run. Might be problematic, but those aren't new issues.

Co-authored-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>
This commit is contained in:
Sean Dewar
2026-03-24 18:25:03 +00:00
parent d539d6ac6e
commit 6617f85b76
10 changed files with 294 additions and 168 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
test/unit/buffer_spec.lua
View File

@@ -13,8 +13,8 @@ describe('buffer functions', function()
return buffer.buflist_new(c_file, c_file, 1, flags)
end
local close_buffer = function(win, buf, action, abort_if_last, ignore_abort)
return buffer.close_buffer(win, buf, action, abort_if_last, ignore_abort)
local close_buffer = function(...)
return buffer.close_buffer(...)
end
local path1 = 'test_file_path'
@@ -48,7 +48,7 @@ describe('buffer functions', function()
itp('should view a closed and hidden buffer as valid', function()
local buf = buflist_new(path1, buffer.BLN_LISTED)
close_buffer(NULL, buf, 0, 0, 0)
close_buffer(NULL, buf, 0, 0, 0, 0)
eq(true, buffer.buf_valid(buf))
end)
@@ -56,7 +56,7 @@ describe('buffer functions', function()
itp('should view a closed and unloaded buffer as valid', function()
local buf = buflist_new(path1, buffer.BLN_LISTED)
close_buffer(NULL, buf, buffer.DOBUF_UNLOAD, 0, 0)
close_buffer(NULL, buf, buffer.DOBUF_UNLOAD, 0, 0, 0)
eq(true, buffer.buf_valid(buf))
end)
@@ -64,7 +64,7 @@ describe('buffer functions', function()
itp('should view a closed and wiped buffer as invalid', function()
local buf = buflist_new(path1, buffer.BLN_LISTED)
close_buffer(NULL, buf, buffer.DOBUF_WIPE, 0, 0)
close_buffer(NULL, buf, buffer.DOBUF_WIPE, 0, 0, 0)
eq(false, buffer.buf_valid(buf))
end)
@@ -83,7 +83,7 @@ describe('buffer functions', function()
eq(buf.handle, buflist_findpat(path1, ONLY_LISTED))
close_buffer(NULL, buf, buffer.DOBUF_WIPE, 0, 0)
close_buffer(NULL, buf, buffer.DOBUF_WIPE, 0, 0, 0)
end)
itp('should prefer to match the start of a file path', function()
@@ -95,9 +95,9 @@ describe('buffer functions', function()
eq(buf2.handle, buflist_findpat('file', ONLY_LISTED))
eq(buf3.handle, buflist_findpat('path', ONLY_LISTED))
close_buffer(NULL, buf1, buffer.DOBUF_WIPE, 0, 0)
close_buffer(NULL, buf2, buffer.DOBUF_WIPE, 0, 0)
close_buffer(NULL, buf3, buffer.DOBUF_WIPE, 0, 0)
close_buffer(NULL, buf1, buffer.DOBUF_WIPE, 0, 0, 0)
close_buffer(NULL, buf2, buffer.DOBUF_WIPE, 0, 0, 0)
close_buffer(NULL, buf3, buffer.DOBUF_WIPE, 0, 0, 0)
end)
itp('should prefer to match the end of a file over the middle', function()
@@ -111,7 +111,7 @@ describe('buffer functions', function()
--}
--{ When: We close buf2
close_buffer(NULL, buf2, buffer.DOBUF_WIPE, 0, 0)
close_buffer(NULL, buf2, buffer.DOBUF_WIPE, 0, 0, 0)
-- And: Open buf1, which has 'file' in the middle of its name
local buf1 = buflist_new(path1, buffer.BLN_LISTED)
@@ -120,8 +120,8 @@ describe('buffer functions', function()
eq(buf3.handle, buflist_findpat('file', ONLY_LISTED))
--}
close_buffer(NULL, buf1, buffer.DOBUF_WIPE, 0, 0)
close_buffer(NULL, buf3, buffer.DOBUF_WIPE, 0, 0)
close_buffer(NULL, buf1, buffer.DOBUF_WIPE, 0, 0, 0)
close_buffer(NULL, buf3, buffer.DOBUF_WIPE, 0, 0, 0)
end)
itp('should match a unique fragment of a file path', function()
@@ -131,9 +131,9 @@ describe('buffer functions', function()
eq(buf3.handle, buflist_findpat('_test_', ONLY_LISTED))
close_buffer(NULL, buf1, buffer.DOBUF_WIPE, 0, 0)
close_buffer(NULL, buf2, buffer.DOBUF_WIPE, 0, 0)
close_buffer(NULL, buf3, buffer.DOBUF_WIPE, 0, 0)
close_buffer(NULL, buf1, buffer.DOBUF_WIPE, 0, 0, 0)
close_buffer(NULL, buf2, buffer.DOBUF_WIPE, 0, 0, 0)
close_buffer(NULL, buf3, buffer.DOBUF_WIPE, 0, 0, 0)
end)
itp('should include / ignore unlisted buffers based on the flag.', function()
@@ -145,7 +145,7 @@ describe('buffer functions', function()
--}
--{ When: We unlist the buffer
close_buffer(NULL, buf3, buffer.DOBUF_DEL, 0, 0)
close_buffer(NULL, buf3, buffer.DOBUF_DEL, 0, 0, 0)
-- Then: It should not find the buffer when searching only listed buffers
eq(-1, buflist_findpat('_test_', ONLY_LISTED))
@@ -155,7 +155,7 @@ describe('buffer functions', function()
--}
--{ When: We wipe the buffer
close_buffer(NULL, buf3, buffer.DOBUF_WIPE, 0, 0)
close_buffer(NULL, buf3, buffer.DOBUF_WIPE, 0, 0, 0)
-- Then: It should not find the buffer at all
eq(-1, buflist_findpat('_test_', ONLY_LISTED))
@@ -173,7 +173,7 @@ describe('buffer functions', function()
--}
--{ When: The first buffer is unlisted
close_buffer(NULL, buf1, buffer.DOBUF_DEL, 0, 0)
close_buffer(NULL, buf1, buffer.DOBUF_DEL, 0, 0, 0)
-- Then: The second buffer is preferred because
-- unlisted buffers are not allowed
@@ -187,7 +187,7 @@ describe('buffer functions', function()
--}
--{ When: We unlist the second buffer
close_buffer(NULL, buf2, buffer.DOBUF_DEL, 0, 0)
close_buffer(NULL, buf2, buffer.DOBUF_DEL, 0, 0, 0)
-- Then: The first buffer is preferred again
-- because buf1 matches better which takes precedence
@@ -198,8 +198,8 @@ describe('buffer functions', function()
eq(-1, buflist_findpat('test', ONLY_LISTED))
--}
close_buffer(NULL, buf1, buffer.DOBUF_WIPE, 0, 0)
close_buffer(NULL, buf2, buffer.DOBUF_WIPE, 0, 0)
close_buffer(NULL, buf1, buffer.DOBUF_WIPE, 0, 0, 0)
close_buffer(NULL, buf2, buffer.DOBUF_WIPE, 0, 0, 0)
end)
end)
end)