[vim/vim] BufDelete called without preceding BufNew (#5694)

[Paul Jolly]

Describe the bug

Similar to #5656, but in this case we see a BufDelete for a buffer before it even exists (i.e. no preceding BufNew).

To Reproduce

Using repro.vim:

set nocompatible
function s:EchoEvent(msg)
  redir >> /tmp/listener.log | echo a:msg | redir END
endfunction

au BufNewFile * call s:EchoEvent("BufNewFile")
au BufReadPre * call s:EchoEvent("BufReadPre")
au BufRead * call s:EchoEvent("BufRead")
au BufReadPost * call s:EchoEvent("BufReadPost")
au BufWrite * call s:EchoEvent("BufWrite")
au BufWritePre * call s:EchoEvent("BufWritePre")
au BufWritePost * call s:EchoEvent("BufWritePost")
au BufAdd * call s:EchoEvent("BufAdd ".expand("<abuf>")." ".expand("<afile>"))
au BufCreate * call s:EchoEvent("BufCreate ".expand("<abuf>")." ".expand("<afile>"))
au BufDelete * call s:EchoEvent("BufDelete ".expand("<abuf>")." ".expand("<afile>")." ".bufnr())
au BufWipeout * call s:EchoEvent("BufWipeout ".expand("<abuf>")." ".expand("<afile>"))
au BufFilePre * call s:EchoEvent("BufFilePre")
au BufFilePost * call s:EchoEvent("BufFilePost")
au BufEnter * call s:EchoEvent("BufEnter ".bufnr()." ".bufname())
au BufLeave * call s:EchoEvent("BufLeave ".expand("<abuf>")." ".expand("<afile>"))
au BufWinEnter * call s:EchoEvent("BufWinEnter ".bufnr()." ".bufname())
au BufWinLeave * call s:EchoEvent("BufWinLeave ".expand("<abuf>")." ".expand("<afile>"))
au BufUnload * call s:EchoEvent("BufUnload ".expand("<abuf>")." ".expand("<afile>"))
au BufHidden * call s:EchoEvent("BufHidden")
au BufNew * call s:EchoEvent("BufNew ".expand("<abuf>")." ".expand("<afile>"))
au VimEnter * call s:EchoEvent("VimEnter")

Run:

vim -u repro.vim

Execute:

:echo map(getbufinfo(), {_, v -> [v.bufnr, v.name]})

The output should be:

[[1, '']]

Now in another terminal:

tail -f /tmp/listener.log

Back in the original terminal, execute:

:tabe .

Notice the output of tail is:

BufNew 2 /home/myitcv/repros/vim
BufAdd 2 /home/myitcv/repros/vim
BufCreate 2 /home/myitcv/repros/vim
BufLeave 1
BufEnter 2 .
BufDelete 3 /home/myitcv/repros/vim 3
BufAdd 3 /home/myitcv/repros/vim
BufCreate 3 /home/myitcv/repros/vim
BufDelete 3 /home/myitcv/repros/vim 3

Buffer 2 has a BufNew fired, but notice the extraneous BufDelete for buffer 3 before it even exists. Then notice there is no subsequent BufNew for buffer 3, only a BufAdd. Finally there is a BufDelete for buffer 3.

If we now execute:

:tabe .

again we see the tail output:

BufAdd 3 /home/myitcv/repros/vim
BufCreate 3 /home/myitcv/repros/vim
BufEnter 3 /home/myitcv/repros/vim
BufDelete 3 /home/myitcv/repros/vim 3
BufAdd 3 /home/myitcv/repros/vim
BufCreate 3 /home/myitcv/repros/vim
BufDelete 3 /home/myitcv/repros/vim 3
BufWinEnter 3 /home/myitcv/repros/vim

Ignoring the fact we haven't seen a BufNew for buffer 3, we get a sequence of BufAdd, BufCreate and BufDelete that look plausible, except that this sequence then ends with BufWinEnter for buffer 3, despite us just having deleted that buffer (according to the line before).

Expected behavior

BufNew to be called to create a buffer, and subsequent buffer events to consistently reflect the buffer lifecycle.

Screenshots

n/a

Environment (please complete the following information):

• Vim version

VIM - Vi IMproved 8.2 (2019 Dec 12, compiled Feb 24 2020 06:34:01)
Included patches: 1-313
Compiled by myitcv@myitcv
Huge version with GTK2 GUI.  Features included (+) or not (-):
+acl               -farsi             -mouse_sysmouse    -tag_old_static
+arabic            +file_in_path      +mouse_urxvt       -tag_any_white
+autocmd           +find_in_path      +mouse_xterm       -tcl
+autochdir         +float             +multi_byte        +termguicolors
-autoservername    +folding           +multi_lang        +terminal
+balloon_eval      -footer            -mzscheme          +terminfo
+balloon_eval_term +fork()            -netbeans_intg     +termresponse
+browse            +gettext           +num64             +textobjects
++builtin_terms    -hangul_input      +packages          +textprop
+byte_offset       +iconv             +path_extra        +timers
+channel           +insert_expand     -perl              +title
+cindent           +job               +persistent_undo   +toolbar
+clientserver      +jumplist          +popupwin          +user_commands
+clipboard         +keymap            +postscript        +vartabs
+cmdline_compl     +lambda            +printer           +vertsplit
+cmdline_hist      +langmap           +profile           +virtualedit
+cmdline_info      +libcall           +python/dyn        +visual
+comments          +linebreak         +python3/dyn       +visualextra
+conceal           +lispindent        +quickfix          +viminfo
+cryptv            +listcmds          +reltime           +vreplace
+cscope            +localmap          +rightleft         +wildignore
+cursorbind        +lua               -ruby              +wildmenu
+cursorshape       +menu              +scrollbind        +windows
+dialog_con_gui    +mksession         +signs             +writebackup
+diff              +modify_fname      +smartindent       +X11
+digraphs          +mouse             -sound             -xfontset
+dnd               +mouseshape        +spell             +xim
-ebcdic            +mouse_dec         +startuptime       +xpm
+emacs_tags        -mouse_gpm         +statusline        +xsmp_interact
+eval              -mouse_jsbterm     -sun_workshop      +xterm_clipboard
+ex_extra          +mouse_netterm     +syntax            -xterm_save
+extra_search      +mouse_sgr         +tag_binary        
   system vimrc file: "$VIM/vimrc"
     user vimrc file: "$HOME/.vimrc"
 2nd user vimrc file: "~/.vim/vimrc"
      user exrc file: "$HOME/.exrc"
  system gvimrc file: "$VIM/gvimrc"
    user gvimrc file: "$HOME/.gvimrc"
2nd user gvimrc file: "~/.vim/gvimrc"
       defaults file: "$VIMRUNTIME/defaults.vim"
    system menu file: "$VIMRUNTIME/menu.vim"
  fall-back for $VIM: "/home/myitcv/usr/vim/share/vim"
Compilation: gcc -c -I. -Iproto -DHAVE_CONFIG_H -DFEAT_GUI_GTK  -pthread -I/usr/include/gtk-2.0 -I/usr/lib/x86_64-linux-gnu/gtk-2.0/include -I/usr/include/pango-1.0 -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/pango-1.0 -I/usr/include/harfbuzz -I/usr/include/pango-1.0 -I/usr/include/fribidi -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libpng16   -g -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1       
Linking: gcc   -L/usr/local/lib -Wl,--as-needed -o vim   -lgtk-x11-2.0 -lgdk-x11-2.0 -lpangocairo-1.0 -latk-1.0 -lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lpangoft2-1.0 -lpango-1.0 -lgobject-2.0 -lglib-2.0 -lfontconfig -lfreetype -lSM -lICE -lXpm -lXt -lX11 -lXdmcp -lSM -lICE  -lm -ltinfo -lnsl    -ldl  -L/usr/lib/x86_64-linux-gnu -lluajit-5.1         

• OS: Ubuntu 19.10
• Terminal: XTerm(348)

Additional context

n/a

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

[Paul Jolly]

@brammool - I guess one of the challenges with fixing any bugs related to these events is that existing plugins etc that depend on the current behaviour then break.

Is one solution here therefore to introduce new events that precisely track the buffer lifecycle? Those events could (should?) be very limited in what they permit during a callback too. For example, no action should be permitted that results in a new buffer lifecycle event (the most we do is call listener_add)

[Paul Jolly]

Another variation on this using the same repro.vim is the following (with thanks to @midnightrun):

vim -u repro.vim

Execute:

:echo map(getbufinfo(), {_, v -> [v.bufnr, v.name]})

gives:

[[1, '']]

Execute:

:Explore

shows the following trace:

BufDelete 1  1
BufWipeout 1
BufUnload 1
BufNew 1
BufAdd 1
BufCreate 1
BufEnter 1
BufWinEnter 1
BufFilePre
BufFilePost
BufDelete 1 /home/myitcv/repros/vim 1
BufAdd 1 /home/myitcv/repros/vim
BufCreate 1 /home/myitcv/repros/vim
BufDelete 1 /home/myitcv/repros/vim 1

Yet despite the last line of this trace apparently deleting buffer 1, the current buffer is 1:

:echo bufnr()

gives:

1

[Bram Moolenaar]

Paul Jolly wrote:

> @brammool - I guess one of the challenges with fixing any bugs related
> to these events is that existing plugins etc that depend on the
> current behaviour then break.

Yes. Although the delete event happening for a buffer that didn't have
any "new" event does smell like a bug.

> Is one solution here therefore to introduce new events that precisely
> track the buffer lifecycle? Those events could (should?) be very
> limited in what they permit during a callback too. For example, no
> action should be permitted that results in a new buffer lifecycle

> event (the most we do is call `listener_add`)

I hesitate to add more events, without having a good overview of where
we are heading. We might use callbacks instead, that should be much
quicker, especially if the callback is compiled.

But the main problems with any of these solutions is that it's so easy
to break the flow. Especially when an autocommand does something with
buffers or windows.

--
hundred-and-one symptoms of being an internet addict:
119. You are reading a book and look for the scroll bar to get to
the next page.

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

[Paul Jolly]

We might use callbacks instead, that should be much quicker, especially if the callback is compiled.

Agreed. I'm not fixed on any particular solution here, just raising the problems we're seeing.

But the main problems with any of these solutions is that it's so easy to break the flow. Especially when an autocommand does something with buffers or windows.

Indeed, which is why I suggested a very limited sort of event/callback above. i.e. the most we will do in such a callback is call listener_add or listener_remove. The main goal we're trying to solve for here is keeping the remote plugin in sync with the state of buffers in Vim, so that when events happen in buffer N we know about it.

I'm just about to experiment with a different approach which might serve as a useful proof of concept. I plan to subscribe to all buffer events and have them call the following function:

let s:bufferState = []

function s:bufferStateCheck()
  let newState = map(getbufinfo(), { _, v -> {'bufnr': v.bufnr, 'name': v.name, 'loaded': v.loaded}})
  if s:bufferState != newState
    let s:bufferState = newState
    if s:govim_status == "initcomplete"
      doautoall User BufferStateChange
    endif
  endif
endfunction

That way, govim can subscribe to this autocommand and evaluate the value of s:bufferState when the event fires and handle the deltas accordingly in the order of:

• new/removed buffers
• buffer name changes
• buffer loaded state changes

Any thoughts on this approach, as a workaround?

[Bram Moolenaar]

Paul Jolly wrote:

> > We might use callbacks instead, that should be much quicker,
> > especially if the callback is compiled.
>
> Agreed. I'm not fixed on any particular solution here, just raising
> the problems we're seeing.
>
> > But the main problems with any of these solutions is that it's so
> > easy to break the flow. Especially when an autocommand does
> > something with buffers or windows.
>
> Indeed, which is why I suggested a very limited sort of event/callback
> above. i.e. the most we will do in such a callback is call

> `listener_add` or `listener_remove`. The main goal we're trying to

> solve for here is keeping the remote plugin in sync with the state of
> buffers in Vim, so that when events happen in buffer N we know about
> it.
>
> I'm just about to experiment with a different approach which might
> serve as a useful proof of concept. I plan to subscribe to all buffer
> events and have them call the following function:
>

> ```vim

> let s:bufferState = []
>
> function s:bufferStateCheck()
> let newState = map(getbufinfo(), { _, v -> {'bufnr': v.bufnr, 'name': v.name, 'loaded': v.loaded}})
> if s:bufferState != newState
> let s:bufferState = newState
> if s:govim_status == "initcomplete"
> doautoall User BufferStateChange
> endif
> endif
> endfunction
> ```
>
> That way, `govim` can subscribe to this autocommand and evaluate the
> value of `s:bufferState` when the event fires and handle the deltas
> accordingly in the order of:
>

> * new/removed buffers
> * buffer name changes
> * buffer loaded state changes

>
> Any thoughts on this approach, as a workaround?

Perhaps you are really only interested in loaded buffers?
There is the BufUnload event, but there is no BufLoad event. It could
be triggered after putting text in the buffer. Or before that, but I
suppose it's most useful when the text is there. BufReadPost comes
close, but is not triggered for creating a new buffer.

--
"Computers in the future may weigh no more than 1.5 tons."
Popular Mechanics, 1949

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

—

[Paul Jolly]

@brammool - having experimented for the last week or so I think I've exhausted options for working around this. Perhaps this should have been obvious to me at the start of this experiment, but all workarounds suffer because if any buffer (or other) autocommand event is out of sequence it ultimately affects the workaround too in such a way that I can't detect/recover from.

But the experiment has been useful in the following respect: I now have a much clearer idea of the events I need to detect:

• new buffer/buffer wiped out - govim tracks details about buffers in Vim, so it makes sense to accurately follow the lifecycle of buffers from beginning to end
• buffer loaded/unloaded - when a buffer is loaded we call listener_add, and similarly we call listener_remove when a buffer is unloaded
• when a buffer is "ready". For an empty unnamed buffer (e.g. run vim) this will fire after the buffer is loaded. Similarly for an empty named buffer (e.g. run vim blah.go where blah.go does not exist) this will fire after the buffer is loaded. But for a non-empty named buffer (e.g. run vim main.go where main.go does exist) this would fire after the buffer contents have been updated from the file, i.e. post BufRead
• when a buffer's content is updated outside of a listener_add callback, e.g. when a buffer's contents are set from a file that is being opened (e.g. BufRead). Happens when you e! to re-read a file from disk, for example
• when a buffer is renamed - e.g. when an unnamed buffer (with content) is saved via w file.go. Such a transition then makes that buffer of interest by virtue of the name the buffer now has ending in .go

I think this points towards a fairly low-level callback that accurately fires such that govim (and other remote plugins) can accurately keep track of buffer state. The callback would include the pre and post state of the buffer(s) in question.

Would something like this be possible?

[Paul Jolly]

Just to clarify one aspect of my previous comment: these low-level callbacks would need to fire after the event has happened. e.g. immediately after a new buffer has been created, after a buffer has been wiped out etc.

[Paul Jolly]

@brammool - any thoughts on the rough outline of a solution above?

[Bram Moolenaar]

Haven't had time to look into the details.
One problem comes to mind with buffer autocommands: if we allow just about any change, it can break in many ways. e.g. a BufNew autocommand might add another buffer and make that the current buffer, which makes Vim crash if we're not careful. How about making the new autocommands not trigger immediately, but only later, in between this and the next command.

[Paul Jolly]

My revised suggestion is actually built on your suggestion of callbacks: #5694 (comment). i.e. don't use autocommands.

if we allow just about any change, it can break in many ways. e.g. a BufNew autocommand might add another buffer and make that the current buffer, which makes Vim crash if we're not careful

I think you just disallow this, or simply say "you can't do this - the results are undefined"

[Bram Moolenaar]

Paul Jolly wrote:

> My revised suggestion is actually built on your suggestion of callbacks: https://github.com/vim/vim/issues/5694#issuecomment-594525215. i.e. don't use autocommands.

>
> > if we allow just about any change, it can break in many ways. e.g. a BufNew autocommand might add another buffer and make that the current buffer, which makes Vim crash if we're not careful
>
> I think you just disallow this, or simply say "you can't do this - the
> results are undefined"

I don't like "undefined behavior", because people will depend on it
anyway. We could add new "BufIs" autocommands that disallow any kind of
changes to the buffer list:
BufIsCreated
BufIsLoaded
BufIsUnloaded
BufIsWiped

Do we also need events for a buffer being renamed?

--
hundred-and-one symptoms of being an internet addict:

244. You use more than 20 passwords.

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

—

[Paul Jolly]

I don't like "undefined behavior", because people will depend on it
anyway. We could add new "BufIs" autocommands that disallow any kind of
changes to the buffer list:
BufIsCreated
BufIsLoaded
BufIsUnloaded
BufIsWiped

Do we also need events for a buffer being renamed?

Apologies, I've been busy with other things lately. Thank you for replying - I haven't forgotten about this and will reply ASAP.

Paul

[vim-dev ML]