vim-patch:9.2.{0252,0253,0254}: I want to get off `b_nwindows`' wild ride by seandewar · Pull Request #38473 · neovim/neovim

seandewar · 2026-03-24T18:47:02Z

Ref: vim/vim#19728

seandewar · 2026-03-25T09:42:44Z

Tests seem to pass. Just had to update unit tests with the new close_buffer() signature, fix a small bug in the port of v9.1.0678, and reset b_nwindows in free_all_mem() (which also seems to have fixed all the existing X lua references were leaked! warnings on the CI?)

Pretty cool. I'll still probably spend more time combing over stuff because I'm paranoid. 😁

Problem: - Small error in port of v9.1.0678, causing :ball to check w_locked for the wrong window. - After neovim#27439, free_all_mem() may not wipe out buffers that were open in more than one window before windows were freed. Solution: - Check win_locked() for wp in ex_buffer_all(), not curwin. - Set b_nwindows to 0 in free_all_mem() before calling close_buffer(). Ref: neovim#38473 (comment) No need to block these fixes on that. free_all_mem() change also looks like it fixed the "N lua references were leaked!" warnings on the CI in that PR.

Problem: - Small error in port of v9.1.0678, causing :ball to check w_locked for the wrong window. - After #27439, free_all_mem() may not wipe out buffers that were open in more than one window before windows were freed. Solution: - Check win_locked() for wp in ex_buffer_all(), not curwin. - Set b_nwindows to 0 in free_all_mem() before calling close_buffer(). Ref: #38473 (comment) No need to block these fixes on that. free_all_mem() change also looks like it fixed the existing "N lua references were leaked!" warnings on the CI.

seandewar · 2026-03-27T20:52:25Z

Some remarks in the commit messages.

I think it's safest to wait until after the release with these fixes, in case I screwed something up. (But I think it's solid 🙏; I'll add revisit-at-release as an advisory label anyway)

Also, maybe 9.2.0252 should be considered a partial patch? I 'unno.

…loaded Problem: if close_buffer() in set_curbuf() unloads curbuf, NULL pointer accesses may occur from enter_buffer() calling end_visual_mode(), as curbuf is already abandoned and possibly unloaded. Also, selection registers may not contain the selection with clipboard+=autoselect(plus). Solution: Move close_buffer()'s end_visual_mode() call to buf_freeall(), after any autocmds that may restart it, but just before freeing anything (Sean Dewar) related: vim/vim#19728 vim/vim@a8fdfd4 Maybe this should be considered partial? clipboard+=autoselect isn't implemented. If it is, we may need to update these comments to mention TextYankPost being possible, and unskip its test. Co-authored-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>

…g 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 vim/vim@bf21df1 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>

Problem: w_locked can be bypassed when recursively set if not restored to its prior value. Solution: Rather than save/restore everywhere, just make it a count, like other locks (Sean Dewar) Requires the previous commit, otherwise b_nwindows will be wrong in tests, which causes a bunch of weird failures. closes: vim/vim#19728 vim/vim@7cb43f2 Co-authored-by: Sean Dewar <6256228+seandewar@users.noreply.github.com>

The NULL checks added in neovim#35679 are no longer needed after neovim#38473. _____________________________________________________________________________________________ *** CID 645258: (REVERSE_INULL) /src/nvim/buffer.c: 645 in close_buffer() 639 // Autocommands deleted the buffer. 640 emsg(_(e_auabort)); 641 return false; 642 } 643 buf->b_locked--; 644 buf->b_locked_split--; >>> CID 645258: (REVERSE_INULL) >>> Null-checking "win" suggests that it may be null, but it has already been dereferenced on all paths leading to the check. 645 if (abort_if_last && win != NULL && one_window(win, NULL)) { 646 // Autocommands made this the only window. 647 emsg(_(e_auabort)); 648 return false; 649 } 650 } /src/nvim/buffer.c: 626 in close_buffer() 620 // Autocommands deleted the buffer. 621 emsg(_(e_auabort)); 622 return false; 623 } 624 buf->b_locked--; 625 buf->b_locked_split--; >>> CID 645258: (REVERSE_INULL) >>> Null-checking "win" suggests that it may be null, but it has already been dereferenced on all paths leading to the check. 626 if (abort_if_last && win != NULL && one_window(win, NULL)) { 627 // Autocommands made this the only window. 628 emsg(_(e_auabort)); 629 return false; 630 } 631

The NULL checks added in #35679 are no longer needed after #38473. _____________________________________________________________________________________________ *** CID 645258: (REVERSE_INULL) /src/nvim/buffer.c: 645 in close_buffer() 639 // Autocommands deleted the buffer. 640 emsg(_(e_auabort)); 641 return false; 642 } 643 buf->b_locked--; 644 buf->b_locked_split--; >>> CID 645258: (REVERSE_INULL) >>> Null-checking "win" suggests that it may be null, but it has already been dereferenced on all paths leading to the check. 645 if (abort_if_last && win != NULL && one_window(win, NULL)) { 646 // Autocommands made this the only window. 647 emsg(_(e_auabort)); 648 return false; 649 } 650 } /src/nvim/buffer.c: 626 in close_buffer() 620 // Autocommands deleted the buffer. 621 emsg(_(e_auabort)); 622 return false; 623 } 624 buf->b_locked--; 625 buf->b_locked_split--; >>> CID 645258: (REVERSE_INULL) >>> Null-checking "win" suggests that it may be null, but it has already been dereferenced on all paths leading to the check. 626 if (abort_if_last && win != NULL && one_window(win, NULL)) { 627 // Autocommands made this the only window. 628 emsg(_(e_auabort)); 629 return false; 630 } 631

The NULL checks added in neovim#35679 are no longer needed after neovim#38473. _____________________________________________________________________________________________ *** CID 645258: (REVERSE_INULL) /src/nvim/buffer.c: 645 in close_buffer() 639 // Autocommands deleted the buffer. 640 emsg(_(e_auabort)); 641 return false; 642 } 643 buf->b_locked--; 644 buf->b_locked_split--; >>> CID 645258: (REVERSE_INULL) >>> Null-checking "win" suggests that it may be null, but it has already been dereferenced on all paths leading to the check. 645 if (abort_if_last && win != NULL && one_window(win, NULL)) { 646 // Autocommands made this the only window. 647 emsg(_(e_auabort)); 648 return false; 649 } 650 } /src/nvim/buffer.c: 626 in close_buffer() 620 // Autocommands deleted the buffer. 621 emsg(_(e_auabort)); 622 return false; 623 } 624 buf->b_locked--; 625 buf->b_locked_split--; >>> CID 645258: (REVERSE_INULL) >>> Null-checking "win" suggests that it may be null, but it has already been dereferenced on all paths leading to the check. 626 if (abort_if_last && win != NULL && one_window(win, NULL)) { 627 // Autocommands made this the only window. 628 emsg(_(e_auabort)); 629 return false; 630 } 631

seandewar added the DO NOT MERGE Nothing to see here; move along label Mar 24, 2026

seandewar force-pushed the i-hate-b_nwindows-all-my-homies-hate-b_nwindows branch 4 times, most recently from 94c4562 to 8df8695 Compare March 25, 2026 08:50

seandewar mentioned this pull request Mar 25, 2026

fix: :ball w_locked check, reset b_nwindows in free_all_mem() #38484

Merged

seandewar mentioned this pull request Mar 26, 2026

Fix out-of-sync b_nwindows, other uncovered issues vim/vim#19728

Closed

seandewar force-pushed the i-hate-b_nwindows-all-my-homies-hate-b_nwindows branch 2 times, most recently from 4c298ef to 640a0ff Compare March 27, 2026 20:26

seandewar changed the title ~~I hate b_nwindows all my homies hate b_nwindows~~ vim-patch:9.2.{0252,0253,0254}: I want to get off b_nwindows' wild ride Mar 27, 2026

seandewar added vim-patch See https://neovim.io/doc/user/dev_vimpatch.html and removed DO NOT MERGE Nothing to see here; move along labels Mar 27, 2026

seandewar force-pushed the i-hate-b_nwindows-all-my-homies-hate-b_nwindows branch from 640a0ff to 31bc432 Compare March 27, 2026 20:47

seandewar marked this pull request as ready for review March 27, 2026 20:47

github-actions Bot requested a review from zeertzjq March 27, 2026 20:50

seandewar added the revisit-at-release label Mar 27, 2026

zeertzjq approved these changes Mar 27, 2026

View reviewed changes

seandewar added 3 commits March 29, 2026 23:26

seandewar force-pushed the i-hate-b_nwindows-all-my-homies-hate-b_nwindows branch from 31bc432 to 6b0532c Compare March 29, 2026 22:30

seandewar enabled auto-merge (rebase) March 29, 2026 22:30

seandewar removed the revisit-at-release label Mar 29, 2026

seandewar merged commit 8579946 into neovim:master Mar 29, 2026
43 checks passed

seandewar deleted the i-hate-b_nwindows-all-my-homies-hate-b_nwindows branch March 29, 2026 22:50

zeertzjq mentioned this pull request Mar 30, 2026

refactor(buffer.c): fix coverity warning #38589

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

vim-patch:9.2.{0252,0253,0254}: I want to get off `b_nwindows`' wild ride#38473

vim-patch:9.2.{0252,0253,0254}: I want to get off `b_nwindows`' wild ride#38473
seandewar merged 3 commits intoneovim:masterfrom
seandewar:i-hate-b_nwindows-all-my-homies-hate-b_nwindows

seandewar commented Mar 24, 2026 •

edited

Loading

Uh oh!

seandewar commented Mar 25, 2026 •

edited

Loading

Uh oh!

seandewar commented Mar 27, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

seandewar commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seandewar commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seandewar commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

seandewar commented Mar 24, 2026 •

edited

Loading

seandewar commented Mar 25, 2026 •

edited

Loading

seandewar commented Mar 27, 2026 •

edited

Loading