[vim/vim] `b_nwindows` wrong after `:edit` in `BufUnload` (Issue #19116)

[Sean Dewar]

seandewar created an issue (vim/vim#19116)

Steps to reproduce

vim -u repro.vim

repro.vim:

set noswf
edit foo
autocmd BufUnload * ++once edit bar
bdelete

" after this:
" "foo"   buffer 1: b_nwindows 0, actual: 0
" no name buffer 2: b_nwindows 1, actual: 1
" "bar"   buffer 3: b_nwindows 1, actual: 0 (!)

(doesn't seem related to #19096 (comment))

Expected behaviour

b_nwindows of "bar" should be 0.

Version of Vim

9.1.2058

Environment

OS: Fedora Workstation 43
Terminal: ghostty 1.2.3, tmux 3.5a,
$TERM: tmux-256color
shell: bash 5.3.0

Logs and stack traces

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/19116@github.com>

[Paul Ollis]

paul-ollis left a comment (vim/vim#19116)

The Vim help for BufUnload says:

Don't change to another buffer or window, it will cause problems!

So using edit is invalid.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/19116/3801270615@github.com>

[Sean Dewar]

seandewar left a comment (vim/vim#19116)

This is the thread that resulted in that addition. Doesn't seem like Bram got around to making it an error.

Since then buf_freeall and surrounding code has become a bit better at dealing with unexpected buffer/window changes. (as far back as 7.4.2006, maybe further) Assuming it's not possible to fix b_nwindows here, I'd worry the alternative of disallowing buffer/window changes completely during those events would be too heavy-handed...

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/19116/3801788554@github.com>

[Paul Ollis]

paul-ollis left a comment (vim/vim#19116)

@seandewar Wow that was some while ago!

I do not think would be consider to heavy-handled to block buffer/window changes, but I suspect that it would not be an easy change (I have not actually looked). I think it would be a good change, but probably quite low priority.

As a general rule, it is best to be treat autocmd and some other "callback" mechanisms as interrupts and keep the "interrupt code" conservative. Using a timer of zero seconds (timer_start) is a good way to execute non-trivial actions (such as editing a new file), because timer callbacks are invoked by Vim when most actions are safe and permitted.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/19116/3801996040@github.com>

[Sean Dewar]

seandewar left a comment (vim/vim#19116)

My concern is that aggressive curbuf_locked/textlock-like restrictions for these rather well-used events after such a long time can be unexpectedly breaking, and I'm not convinced it's generally warranted now; additional checks such as b_locked_split (see my related patches) and changes like 9.1.2087 have already improved things greatly since then.

As a general rule, it is best to be treat autocmd and some other "callback" mechanisms as interrupts and keep the "interrupt code" conservative.

I'm (unfortunately) well aware -- autocommand bugs drive me mad! 🙈 In Nvim land, we've started to consider deferring newer events for this reason, provided it doesn't hurt flexibility too much.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <vim/vim/issues/19116/3802232433@github.com>