Bug in patch 9.0.0907 causes E1312 in autocmd

[Gary Johnson]

I've had an autocommand in my vimrc for quite some time that
I noticed recently was causing the following error message when
activated:

Error detected while processing BufEnter Autocommands for "<buffer=3>":
E1312: Not allowed to change the window layout in this autocmd

I performed a git bisect on the Vim source and found that the bug
was introduced here:

d63a85592cef0ee4f0fec5efe2f8d66b31f01f05 is the first bad commit
commit d63a85592cef0ee4f0fec5efe2f8d66b31f01f05
Author: Bram Moolenaar <Br...@vim.org>
Date: Sat Nov 19 11:41:30 2022 +0000

patch 9.0.0907: restoring window after WinScrolled may fail

Problem: Restoring window after WinScrolled may fail.
Solution: Lock the window layout when triggering WinScrolled.

src/errors.h | 2 ++
src/ex_docmd.c | 18 ++++++++++++-----
src/proto/window.pro | 1 +
src/version.c | 2 ++
src/window.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++---
5 files changed, 71 insertions(+), 8 deletions(-)

This is the autocommand in my vimrc and the reason for it being
there:

" If the Calendar window is open when the last non-Calendar
" window is closed, Vim continues to run with the Calendar
" window occupying the full Vim window. Fix this,
" automatically close the Calendar window if it is the only
" window.
"
autocmd BufWinEnter __Calendar
\ autocmd BufEnter <buffer> if winnr("$")==1 | quit | endif

Steps to reproduce

1. Start Vim with the following command (one line):

$ vim -N -u NONE -i NONE -c tabnew -c vnew -c 'autocmd BufEnter <buffer> if winnr("$")==1 | quit | endif' -c 'wincmd w'

This opens a new tab, opens a new window in that tab, creates
a BufEnter autocommand in that buffer, and moves the cursor to
the right window.

2. Execute

:q

The error message appears.

3. Hit ENTER to continue. The right window has closed, but the
left window (now the only window on that tab page) remains open.

Expected behavior

At step 2, there should be no error message and both windows on the
second tab page should have closed, leaving only the first tab
page.

The right window was closed by :q. That caused the cursor to enter
the remaining window in that tab, triggering the autocommand and
closing that window and the tab.

Version of Vim

Observed on version 9.0.1034. Found to have been introduced at
version 9.0.0907.

Environment

Operating system: Ubuntu 22.04
Terminal: XTerm(373)
Value of $TERM: xterm-256color
Shell: bash

Regards,
Gary

[Bram Moolenaar]

So you have a BufEnter autocommand that closes the buffer (with some
condition). The BufEnter event can be triggered in various places, and
they are most likely not prepared for the buffer disappearing.

There already is quite a lot of code to handle side effects of what an
autocmd may do, but it's getting very complicated. In some cases it's
better to just not allow certain actions. For a BufEnter event it seems
quite natural to disallow deleting that buffer. Why would you delete a
buffer you just entered?

Since this stuff is so complicated we keep finding new problems. So
once in a while something that was allowed before is no longer allowed,
because there are situations where it causes a crash.

The situation you describe seems quite rare, thus I think we just have
to accept this no longer works. You can probably find another way.


--
The psychic said, "God bless you." I said, "I didn't sneeze." She
looked deep into my eyes and said, "You will, eventually." And, damn
if she wasn't right. Two days later, I sneezed. --Ellen Degeneres

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[Gary Johnson]

The autocommand is defined only when the buffer name is "__Calendar"
and the "<buffer>" pattern is used so that that is the only buffer
in which the command is triggered. The event can be triggered in
only one place and that place is prepared for it.

> There already is quite a lot of code to handle side effects of what an
> autocmd may do, but it's getting very complicated. In some cases it's
> better to just not allow certain actions. For a BufEnter event it seems
> quite natural to disallow deleting that buffer. Why would you delete a
> buffer you just entered?

In this case, I want to delete a buffer if it is the only buffer
displayed in the tab. I never use that buffer alone, only alongside
another buffer. When I close that other buffer, I want the
__Calendar buffer to be deleted, too, because I'm done with it.
I got tired of typing ":q<Enter>" twice, so I created the
autocommand to save some typing and time, i.e., to make the scenario
"just work" as Vim usually gives me the tools to do.

The BufEnter event is used because that's what happens when a window
is closed: some other window is entered. And with the test for
'winnr("$")==1', it isn't closed every time it is entered, but only
when it is the last open window in the tab.

> Since this stuff is so complicated we keep finding new problems. So
> once in a while something that was allowed before is no longer allowed,
> because there are situations where it causes a crash.
>
> The situation you describe seems quite rare, thus I think we just have
> to accept this no longer works. You can probably find another way.

I'll buy that the code is getting very complicated and that some use
cases aren't worth supporting. It's just a shame when useful tools
are prevented from working because they don't work in every case,
and frustrating when things that used to work stop working.

I don't know what other event is triggered when a window/buffer
becomes the last one open in a tab. I suppose I could try having
a BufDelete or WinClosed event check whether the buffer/window being
closed is the next-to-last one in a tab, and if the other buffer is
__Calendar, then also close that window somehow. (I know my use of
"window" and "buffer" above is a little loose.) I just hope those
commands haven't required the same protections as BufEnter.

Thanks for taking the time to explain the situation.

Regards,
Gary

[Bram Moolenaar]

That is your specific situation. Other users may have very different
situations. And we need to deal with all of them...

> > There already is quite a lot of code to handle side effects of what an
> > autocmd may do, but it's getting very complicated. In some cases it's
> > better to just not allow certain actions. For a BufEnter event it seems
> > quite natural to disallow deleting that buffer. Why would you delete a
> > buffer you just entered?
>
> In this case, I want to delete a buffer if it is the only buffer
> displayed in the tab. I never use that buffer alone, only alongside
> another buffer. When I close that other buffer, I want the
> __Calendar buffer to be deleted, too, because I'm done with it.
> I got tired of typing ":q<Enter>" twice, so I created the
> autocommand to save some typing and time, i.e., to make the scenario
> "just work" as Vim usually gives me the tools to do.
>
> The BufEnter event is used because that's what happens when a window
> is closed: some other window is entered. And with the test for
> 'winnr("$")==1', it isn't closed every time it is entered, but only
> when it is the last open window in the tab.

The idea of BufEnter is that you can set some options specifically for a
buffer, setup mappings or insert some template text. Deleting the
buffer isn't really anticipated.

> > Since this stuff is so complicated we keep finding new problems. So
> > once in a while something that was allowed before is no longer allowed,
> > because there are situations where it causes a crash.
> >
> > The situation you describe seems quite rare, thus I think we just have
> > to accept this no longer works. You can probably find another way.
>
> I'll buy that the code is getting very complicated and that some use
> cases aren't worth supporting. It's just a shame when useful tools
> are prevented from working because they don't work in every case,
> and frustrating when things that used to work stop working.

At some point I thought I should drop auto commands completely, because
it's just getting too complicated and too many crashes have had to be
fixed. And there are probably a few more that we haven't found yet.

But users, and especialy plugin writers, depend on auto commands, and
there is no good replacement, thus they are still here. But let it be
clear that supporting auto commands is almost infeasible, thus you can
expect some limitations. I currently tend to prevent things from going
bad rather than deal with the problems caused by them. E.g. disallow
closing and splitting windows rather than dealing with a window
disappearing "under our fingers". Sometimes it's not at all clear what
to do then.

> I don't know what other event is triggered when a window/buffer
> becomes the last one open in a tab. I suppose I could try having
> a BufDelete or WinClosed event check whether the buffer/window being
> closed is the next-to-last one in a tab, and if the other buffer is
> __Calendar, then also close that window somehow. (I know my use of
> "window" and "buffer" above is a little loose.) I just hope those
> commands haven't required the same protections as BufEnter.

Whatever auto command event you use, deleting the buffer will always be
problematic. Maybe you find one where it works now, but after some
crashing bug it may stop working.

There is one exception: the SafeState event. It was specifically added
for when Vim isn't doing anythng, thus deleting the buffer then is
probably OK. Give it a try.

--
CONCORDE: Message for you, sir.
He falls forward revealing the arrow with the note.
"Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD

[Gary Johnson]

On 2022-12-14, Bram Moolenaar wrote:

Yes, autocommands are wonderful! There are so many situations where
when "this" happens you want "that" to happen, too. Autocommands
are pretty much the only hooks we have. They do so much to make Vim
work just the way I want it. And I do appreciate that they also
allow users to shoot themselves (ourselves) in the foot. I'm sorry
they're a PITA for you. Thanks for maintaining them.

> > I don't know what other event is triggered when a window/buffer
> > becomes the last one open in a tab. I suppose I could try having
> > a BufDelete or WinClosed event check whether the buffer/window being
> > closed is the next-to-last one in a tab, and if the other buffer is
> > __Calendar, then also close that window somehow. (I know my use of
> > "window" and "buffer" above is a little loose.) I just hope those
> > commands haven't required the same protections as BufEnter.
>
> Whatever auto command event you use, deleting the buffer will always be
> problematic. Maybe you find one where it works now, but after some
> crashing bug it may stop working.
>
> There is one exception: the SafeState event. It was specifically added
> for when Vim isn't doing anythng, thus deleting the buffer then is
> probably OK. Give it a try.

Thanks for the tip. I made a note of that near the current code and
I'll try that if what I'm doing now has problems.

I found another solution using feedkeys(), which allows the :quit
command to be executed outside of an autocommand environment.

Regards,
Gary

[Matt Martini]

Gary,

I (and others) are having the same issue with vim-nerdtree-tabs.

It is fully described here: issue #102 

There is a function that when a tab is closed, checks if the last buffer is NERDTree.

and if it is it closes/quits. 

It was using similar code to yours:

if exists("t:NERDTreeBufName") && bufwinnr(t:NERDTreeBufName) != -1 && winnr("$") == 1

or the variants

if (winnr("$") == 1 && exists("b:NERDTreeType") && b:NERDTreeType == "primary"):

if (winnr("$") == 1 && exists("b:NERDTree") && b:NERDTree.isTabTree())

The code was working until 9.0.900+ (I don't know the exact patch).

Unfortunately, the vim-nerdtree-tabs plugin is no longer maintained, so we must find a solution on our own.

Were you able to find a workaround or another method of solving your issue?

Matt

[Gary Johnson]

On 2023-01-28, Matt Martini wrote:
> Gary,
>
> I (and others) are having the same issue with vim-nerdtree-tabs.
> It is fully described here: issue #102 
>
> There is a function that when a tab is closed, checks if the last buffer is
> NERDTree.
> and if it is it closes/quits. 
>
> It was using similar code to yours:
>
> if exists("t:NERDTreeBufName") && bufwinnr(t:NERDTreeBufName) != -1 && winnr
> ("$") == 1
>
> or the variants
>
> if (winnr("$") == 1 && exists("b:NERDTreeType") && b:NERDTreeType ==
> "primary"):
>
> if (winnr("$") == 1 && exists("b:NERDTree") && b:NERDTree.isTabTree())
>
> The code was working until 9.0.900+ (I don't know the exact patch).
>
> Unfortunately, the vim-nerdtree-tabs plugin is no longer maintained, so we must
> find a solution on our own.
>
> Were you able to find a workaround or another method of solving your issue?
>
> Matt

Matt,

The solution I finally adopted was to use feedkeys(), like this.

" Note the ending ":\<BS>". This clears the "quit" from the command line.
"
autocmd BufWinEnter __Calendar
\ augroup MyCalendar
\ | autocmd!
\ | autocmd BufEnter <buffer> if winnr("$") == 1 | call feedkeys(":quit\<CR>:\<BS>") | endif
\ | augroup END

This isn't all that different from the original code that failed
after patch 9.0.907.

autocmd BufWinEnter __Calendar
\ autocmd BufEnter <buffer> if winnr("$")==1 | quit | endif

Sometime later, I encountered the same error while using the linediff
plugin (https://badge.fury.io/gh/andrewradev/linediff.vim). I fixed
that one temporarily with this change, then reported it to the
author.

diff --git a/autoload/linediff/differ.vim b/autoload/linediff/differ.vim
index 23a69ce..023c716 100644
--- a/autoload/linediff/differ.vim
+++ b/autoload/linediff/differ.vim
@@ -211,7 +211,11 @@ endfunction
function! linediff#differ#CloseDiffBuffer(force) dict
if bufexists(self.diff_buffer)
let bang = a:force ? '!' : ''
- exe "bdelete".bang." ".self.diff_buffer
+ " [GAJ 2022-12-19] Change direct :bdelete command to a feedkeys()
+ " operation to work around the new behavior of patch 9.0.907 that
+ " prohibits certain operations such as buffer deletions in autocommand
+ " contexts. The ":\<BS>" erases the command from the command line.
+ call feedkeys(":bdelete".bang." ".self.diff_buffer."\<CR>:\<BS>")
endif
endfunction

He thought there might be a race condition (he may have actually
observed one--I forget) when using feedkeys() and preferred the
following solution.

diff --git a/autoload/linediff/differ.vim b/autoload/linediff/differ.vim
index 23a69ce..b9f3d12 100644
--- a/autoload/linediff/differ.vim
+++ b/autoload/linediff/differ.vim
@@ -211,7 +211,16 @@ endfunction
function! linediff#differ#CloseDiffBuffer(force) dict
if bufexists(self.diff_buffer)
let bang = a:force ? '!' : ''
- exe "bdelete".bang." ".self.diff_buffer
+ let diff_buffer = self.diff_buffer
+
+ if has('patch-9.0.907')
+ " Vim forbids closing the window inside an autocommand, so let's do it
+ " afterwards.
+ " Ref: https://github.com/AndrewRadev/linediff.vim/issues/36
+ call timer_start(1, {-> execute('bdelete'.bang.' '.diff_buffer) })
+ else
+ exe 'bdelete'.bang.' '.diff_buffer
+ endif
endif
endfunction

Regards,
Gary