vim-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)

(endless yapping omitted)

related: vim/vim#19728

https://github.com/vim/vim/commit/bf21df1c7bc772e3a29961c961d0821584d50ee0

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>

1 files changed, 21 insertions, 21 deletions

diff --git a/test/unit/buffer_spec.lua b/test/unit/buffer_spec.lua
index a7c672322b..76cca1242e 100644
--- a/test/unit/buffer_spec.lua
+++ b/test/unit/buffer_spec.lua
@@ -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)