feat(aucmd_win): allow crazy things with hidden buffers (#21250)

Problem:    Crash when doing crazy things with hidden buffers.
Solution:   Dynamically allocate the list of autocommand windows.

src/nvim/autocmd.c

@@ -633,11 +633,6 @@ void do_augroup(char *arg, int del_group)
   }
 }
 
-void autocmd_init(void)
-{
-  CLEAR_FIELD(aucmd_win);
-}
-
 #if defined(EXITFREE)
 void free_all_autocmds(void)
 {
@@ -1328,13 +1323,6 @@ void ex_doautoall(exarg_T *eap)
 
     // Find a window for this buffer and save some values.
     aucmd_prepbuf(&aco, buf);
-    if (curbuf != buf) {
-      // Failed to find a window for this buffer.  Better not execute
-      // autocommands then.
-      retval = FAIL;
-      break;
-    }
-
     set_bufref(&bufref, buf);
 
     // execute the autocommands for this buffer
@@ -1385,7 +1373,6 @@ bool check_nomodeline(char **argp)
 /// If the current buffer is not in any visible window, put it in a temporary
 /// floating window using an entry in `aucmd_win[]`.
 /// Set `curbuf` and `curwin` to match `buf`.
-/// When this fails `curbuf` is not equal `buf`.
 ///
 /// @param aco  structure to save values in
 /// @param buf  new curbuf
@@ -1413,23 +1400,23 @@ void aucmd_prepbuf(aco_save_T *aco, buf_T *buf)
   if (win == NULL) {
     for (auc_idx = 0; auc_idx < AUCMD_WIN_COUNT; auc_idx++) {
       if (!aucmd_win[auc_idx].auc_win_used) {
-        if (aucmd_win[auc_idx].auc_win == NULL) {
-          win_alloc_aucmd_win(auc_idx);
-          need_append = false;
-        }
-        auc_win = aucmd_win[auc_idx].auc_win;
-        aucmd_win[auc_idx].auc_win_used = true;
         break;
       }
     }
 
-    // If this fails (using all AUCMD_WIN_COUNT entries)
+    if (auc_idx == AUCMD_WIN_COUNT) {
-    // then we can't reliably execute the autocmd,
+      kv_push(aucmd_win_vec, ((aucmdwin_T){
-    // return with "curbuf" unequal "buf".
+        .auc_win = NULL,
-    if (auc_win == NULL) {
+        .auc_win_used = false,
-      assert(curbuf != buf);
+      }));
-      return;
     }
+
+    if (aucmd_win[auc_idx].auc_win == NULL) {
+      win_alloc_aucmd_win(auc_idx);
+      need_append = false;
+    }
+    auc_win = aucmd_win[auc_idx].auc_win;
+    aucmd_win[auc_idx].auc_win_used = true;
   }
 
   aco->save_curwin_handle = curwin->handle;

src/nvim/buffer.c

@@ -181,13 +181,11 @@ void buffer_ensure_loaded(buf_T *buf)
   if (buf->b_ml.ml_mfp == NULL) {
     aco_save_T aco;
 
-    // Make sure the buffer is in a window.  If not then skip it.
+    // Make sure the buffer is in a window.
     aucmd_prepbuf(&aco, buf);
-    if (curbuf == buf) {
+    swap_exists_action = SEA_NONE;
-      swap_exists_action = SEA_NONE;
+    open_buffer(false, NULL, 0);
-      open_buffer(false, NULL, 0);
+    aucmd_restbuf(&aco);
-      aucmd_restbuf(&aco);
-    }
   }
 }
 
@@ -369,20 +367,17 @@ int open_buffer(int read_stdin, exarg_T *eap, int flags_arg)
     aco_save_T aco;
 
     // Go to the buffer that was opened, make sure it is in a window.
-    // If not then skip it.
     aucmd_prepbuf(&aco, old_curbuf.br_buf);
-    if (curbuf == old_curbuf.br_buf) {
+    do_modelines(0);
-      do_modelines(0);
+    curbuf->b_flags &= ~(BF_CHECK_RO | BF_NEVERLOADED);
-      curbuf->b_flags &= ~(BF_CHECK_RO | BF_NEVERLOADED);
 
-      if ((flags & READ_NOWINENTER) == 0) {
+    if ((flags & READ_NOWINENTER) == 0) {
-        apply_autocmds_retval(EVENT_BUFWINENTER, NULL, NULL, false, curbuf,
+      apply_autocmds_retval(EVENT_BUFWINENTER, NULL, NULL, false, curbuf,
-                              &retval);
+                            &retval);
-      }
-
-      // restore curwin/curbuf and a few other things
-      aucmd_restbuf(&aco);
     }
+
+    // restore curwin/curbuf and a few other things
+    aucmd_restbuf(&aco);
   }
 
   return retval;
@@ -4177,11 +4172,6 @@ bool buf_contents_changed(buf_T *buf)
   // Set curwin/curbuf to buf and save a few things.
   aco_save_T aco;
   aucmd_prepbuf(&aco, newbuf);
-  if (curbuf != newbuf) {
-    // Failed to find a window for "newbuf".
-    wipe_buffer(newbuf, false);
-    return true;
-  }
 
   if (ml_open(curbuf) == OK
       && readfile(buf->b_ffname, buf->b_fname,

src/nvim/diff.c

@@ -2944,11 +2944,6 @@ void ex_diffgetput(exarg_T *eap)
 
     // Set curwin/curbuf to buf and save a few things.
     aucmd_prepbuf(&aco, curtab->tp_diffbuf[idx_other]);
-    if (curbuf != curtab->tp_diffbuf[idx_other]) {
-      // Could not find a window for this buffer, the rest is likely to
-      // fail.
-      goto theend;
-    }
   }
 
   const int idx_from = eap->cmdidx == CMD_diffget ? idx_other : idx_cur;

src/nvim/eval/buffer.c

@@ -86,8 +86,6 @@ static void find_win_for_curbuf(void)
 ///
 /// Information is saved in "cob" and MUST be restored by calling
 /// change_other_buffer_restore().
-///
-/// If this fails then "curbuf" will not be equal to "buf".
 static void change_other_buffer_prepare(cob_T *cob, buf_T *buf)
 {
   CLEAR_POINTER(cob);
@@ -105,9 +103,7 @@ static void change_other_buffer_prepare(cob_T *cob, buf_T *buf)
     // curwin->w_buffer differ from "curbuf", use the autocmd window.
     curbuf = curwin->w_buffer;
     aucmd_prepbuf(&cob->cob_aco, buf);
-    if (curbuf == buf) {
+    cob->cob_using_aco = true;
-      cob->cob_using_aco = true;
-    }
   }
 }
 

src/nvim/eval/vars.c

@@ -1802,13 +1802,11 @@ void f_setbufvar(typval_T *argvars, typval_T *rettv, EvalFuncData fptr)
 
       // Set curbuf to be our buf, temporarily.
       aucmd_prepbuf(&aco, buf);
-      if (curbuf == buf) {
-        // Only when it worked to set "curbuf".
-        set_option_from_tv(varname + 1, varp);
 
-        // reset notion of buffer
+      set_option_from_tv(varname + 1, varp);
-        aucmd_restbuf(&aco);
+
-      }
+      // reset notion of buffer
+      aucmd_restbuf(&aco);
     } else {
       const size_t varname_len = strlen(varname);
       char *const bufvarname = xmalloc(varname_len + 3);

src/nvim/ex_cmds2.c

@@ -683,10 +683,8 @@ void ex_listdo(exarg_T *eap)
                          curbuf);
         } else {
           aucmd_prepbuf(&aco, buf);
-          if (curbuf == buf) {
+          apply_autocmds(EVENT_SYNTAX, buf->b_p_syn, buf->b_fname, true, buf);
-            apply_autocmds(EVENT_SYNTAX, buf->b_p_syn, buf->b_fname, true, buf);
+          aucmd_restbuf(&aco);
-            aucmd_restbuf(&aco);
-          }
         }
 
         // start over, in case autocommands messed things up.

src/nvim/fileio.c

@@ -2310,12 +2310,6 @@ int buf_write(buf_T *buf, char *fname, char *sfname, linenr_T start, linenr_T en
 
     // Set curwin/curbuf to buf and save a few things.
     aucmd_prepbuf(&aco, buf);
-    if (curbuf != buf) {
-      // Could not find a window for "buf".  Doing more might cause
-      // problems, better bail out.
-      return FAIL;
-    }
-
     set_bufref(&bufref, buf);
 
     if (append) {
@@ -3618,27 +3612,25 @@ nofail:
 
     // Apply POST autocommands.
     // Careful: The autocommands may call buf_write() recursively!
-    // Only do this when a window was found for "buf".
     aucmd_prepbuf(&aco, buf);
-    if (curbuf == buf) {
-      if (append) {
-        apply_autocmds_exarg(EVENT_FILEAPPENDPOST, fname, fname,
-                             false, curbuf, eap);
-      } else if (filtering) {
-        apply_autocmds_exarg(EVENT_FILTERWRITEPOST, NULL, fname,
-                             false, curbuf, eap);
-      } else if (reset_changed && whole) {
-        apply_autocmds_exarg(EVENT_BUFWRITEPOST, fname, fname,
-                             false, curbuf, eap);
-      } else {
-        apply_autocmds_exarg(EVENT_FILEWRITEPOST, fname, fname,
-                             false, curbuf, eap);
-      }
 
-      // restore curwin/curbuf and a few other things
+    if (append) {
-      aucmd_restbuf(&aco);
+      apply_autocmds_exarg(EVENT_FILEAPPENDPOST, fname, fname,
+                           false, curbuf, eap);
+    } else if (filtering) {
+      apply_autocmds_exarg(EVENT_FILTERWRITEPOST, NULL, fname,
+                           false, curbuf, eap);
+    } else if (reset_changed && whole) {
+      apply_autocmds_exarg(EVENT_BUFWRITEPOST, fname, fname,
+                           false, curbuf, eap);
+    } else {
+      apply_autocmds_exarg(EVENT_FILEWRITEPOST, fname, fname,
+                           false, curbuf, eap);
     }
 
+    // restore curwin/curbuf and a few other things
+    aucmd_restbuf(&aco);
+
     if (aborting()) {       // autocmds may abort script processing
       retval = false;
     }
@@ -5013,11 +5005,6 @@ void buf_reload(buf_T *buf, int orig_mode, bool reload_options)
 
   // Set curwin/curbuf for "buf" and save some things.
   aucmd_prepbuf(&aco, buf);
-  if (curbuf != buf) {
-    // Failed to find a window for "buf", it is dangerous to continue,
-    // better bail out.
-    return;
-  }
 
   // Unless reload_options is set, we only want to read the text from the
   // file, not reset the syntax highlighting, clear marks, diff status, etc.

src/nvim/globals.h

@@ -422,19 +422,18 @@ EXTERN win_T *prevwin INIT(= NULL);  // previous window
 
 EXTERN win_T *curwin;        // currently active window
 
-/// When executing autocommands for a buffer that is not in any window, a
-/// special window is created to handle the side effects.  When autocommands
-/// nest we may need more than one.  Allow for up to five, if more are needed
-/// something crazy is happening.
-enum { AUCMD_WIN_COUNT = 5, };
-
 typedef struct {
   win_T *auc_win;     ///< Window used in aucmd_prepbuf().  When not NULL the
                       ///< window has been allocated.
   bool auc_win_used;  ///< This auc_win is being used.
 } aucmdwin_T;
 
-EXTERN aucmdwin_T aucmd_win[AUCMD_WIN_COUNT];
+/// When executing autocommands for a buffer that is not in any window, a
+/// special window is created to handle the side effects.  When autocommands
+/// nest we may need more than one.
+EXTERN kvec_t(aucmdwin_T) aucmd_win_vec INIT(= KV_INITIAL_VALUE);
+#define aucmd_win (aucmd_win_vec.items)
+#define AUCMD_WIN_COUNT ((int)aucmd_win_vec.size)
 
 // The window layout is kept in a tree of frames.  topframe points to the top
 // of the tree.

src/nvim/main.c

@@ -251,8 +251,6 @@ int main(int argc, char **argv)
   // `argc` and `argv` are also copied, so that they can be changed.
   init_params(&params, argc, argv);
 
-  autocmd_init();
-
   // Since os_open is called during the init_startuptime, we need to call
   // fs_init before it.
   fs_init();

src/nvim/quickfix.c

@@ -3902,27 +3902,21 @@ static void qf_update_buffer(qf_info_T *qi, qfline_T *old_last)
 
     aco_save_T aco;
 
-    bool do_fill = true;
     if (old_last == NULL) {
       // set curwin/curbuf to buf and save a few things
       aucmd_prepbuf(&aco, buf);
-      if (curbuf != buf) {
-        do_fill = false;  // failed to find a window for "buf"
-      }
     }
 
-    if (do_fill) {
+    qf_update_win_titlevar(qi);
-      qf_update_win_titlevar(qi);
 
-      qf_fill_buffer(qf_get_curlist(qi), buf, old_last, qf_winid);
+    qf_fill_buffer(qf_get_curlist(qi), buf, old_last, qf_winid);
-      buf_inc_changedtick(buf);
+    buf_inc_changedtick(buf);
 
-      if (old_last == NULL) {
+    if (old_last == NULL) {
-        (void)qf_win_pos_update(qi, 0);
+      (void)qf_win_pos_update(qi, 0);
 
-        // restore curwin/curbuf and a few other things
+      // restore curwin/curbuf and a few other things
-        aucmd_restbuf(&aco);
+      aucmd_restbuf(&aco);
-      }
     }
 
     // Only redraw when added lines are visible.  This avoids flickering when
@@ -5469,11 +5463,9 @@ static int vgr_process_files(win_T *wp, qf_info_T *qi, vgr_args_T *cmd_args, boo
           // options!
           aco_save_T aco;
           aucmd_prepbuf(&aco, buf);
-          if (curbuf == buf) {
+          apply_autocmds(EVENT_FILETYPE, buf->b_p_ft, buf->b_fname, true, buf);
-            apply_autocmds(EVENT_FILETYPE, buf->b_p_ft, buf->b_fname, true, buf);
+          do_modelines(OPT_NOWIN);
-            do_modelines(OPT_NOWIN);
+          aucmd_restbuf(&aco);
-            aucmd_restbuf(&aco);
-          }
         }
       }
     }
@@ -5631,43 +5623,42 @@ static buf_T *load_dummy_buffer(char *fname, char *dirname_start, char *resultin
     // set curwin/curbuf to buf and save a few things
     aco_save_T aco;
     aucmd_prepbuf(&aco, newbuf);
-    if (curbuf == newbuf) {
-      // Need to set the filename for autocommands.
-      (void)setfname(curbuf, fname, NULL, false);
 
-      // Create swap file now to avoid the ATTENTION message.
+    // Need to set the filename for autocommands.
-      check_need_swap(true);
+    (void)setfname(curbuf, fname, NULL, false);
 
-      // Remove the "dummy" flag, otherwise autocommands may not
+    // Create swap file now to avoid the ATTENTION message.
-      // work.
+    check_need_swap(true);
-      curbuf->b_flags &= ~BF_DUMMY;
 
-      bufref_T newbuf_to_wipe;
+    // Remove the "dummy" flag, otherwise autocommands may not
-      newbuf_to_wipe.br_buf = NULL;
+    // work.
-      int readfile_result = readfile(fname, NULL, (linenr_T)0, (linenr_T)0,
+    curbuf->b_flags &= ~BF_DUMMY;
-                                     (linenr_T)MAXLNUM, NULL,
+
-                                     READ_NEW | READ_DUMMY, false);
+    bufref_T newbuf_to_wipe;
-      newbuf->b_locked--;
+    newbuf_to_wipe.br_buf = NULL;
-      if (readfile_result == OK
+    int readfile_result = readfile(fname, NULL, (linenr_T)0, (linenr_T)0,
-          && !got_int
+                                   (linenr_T)MAXLNUM, NULL,
-          && !(curbuf->b_flags & BF_NEW)) {
+                                   READ_NEW | READ_DUMMY, false);
-        failed = false;
+    newbuf->b_locked--;
-        if (curbuf != newbuf) {
+    if (readfile_result == OK
-          // Bloody autocommands changed the buffer!  Can happen when
+        && !got_int
-          // using netrw and editing a remote file.  Use the current
+        && !(curbuf->b_flags & BF_NEW)) {
-          // buffer instead, delete the dummy one after restoring the
+      failed = false;
-          // window stuff.
+      if (curbuf != newbuf) {
-          set_bufref(&newbuf_to_wipe, newbuf);
+        // Bloody autocommands changed the buffer!  Can happen when
-          newbuf = curbuf;
+        // using netrw and editing a remote file.  Use the current
-        }
+        // buffer instead, delete the dummy one after restoring the
+        // window stuff.
+        set_bufref(&newbuf_to_wipe, newbuf);
+        newbuf = curbuf;
       }
+    }
 
-      // Restore curwin/curbuf and a few other things.
+    // Restore curwin/curbuf and a few other things.
-      aucmd_restbuf(&aco);
+    aucmd_restbuf(&aco);
 
-      if (newbuf_to_wipe.br_buf != NULL && bufref_valid(&newbuf_to_wipe)) {
+    if (newbuf_to_wipe.br_buf != NULL && bufref_valid(&newbuf_to_wipe)) {
-        wipe_buffer(newbuf_to_wipe.br_buf, false);
+      wipe_buffer(newbuf_to_wipe.br_buf, false);
-      }
     }
 
     // Add back the "dummy" flag, otherwise buflist_findname_file_id()

src/nvim/window.c

@@ -3122,6 +3122,8 @@ void win_free_all(void)
     }
   }
 
+  kv_destroy(aucmd_win_vec);
+
   while (firstwin != NULL) {
     int dummy;
     (void)win_free_mem(firstwin, &dummy, NULL);

test/functional/lua/vim_spec.lua

@@ -2679,6 +2679,46 @@ describe('lua stdlib', function()
         a.nvim_buf_call(a.nvim_create_buf(false, true), function() vim.cmd "redraw" end)
       ]]
     end)
+
+    it('can be nested crazily with hidden buffers', function()
+      eq(true, exec_lua([[
+        local function scratch_buf_call(fn)
+          local buf = vim.api.nvim_create_buf(false, true)
+          vim.api.nvim_buf_set_option(buf, 'cindent', true)
+          return vim.api.nvim_buf_call(buf, function()
+            return vim.api.nvim_get_current_buf() == buf
+              and vim.api.nvim_buf_get_option(buf, 'cindent')
+              and fn()
+          end) and vim.api.nvim_buf_delete(buf, {}) == nil
+        end
+
+        return scratch_buf_call(function()
+          return scratch_buf_call(function()
+            return scratch_buf_call(function()
+              return scratch_buf_call(function()
+                return scratch_buf_call(function()
+                  return scratch_buf_call(function()
+                    return scratch_buf_call(function()
+                      return scratch_buf_call(function()
+                        return scratch_buf_call(function()
+                          return scratch_buf_call(function()
+                            return scratch_buf_call(function()
+                              return scratch_buf_call(function()
+                                return true
+                              end)
+                            end)
+                          end)
+                        end)
+                      end)
+                    end)
+                  end)
+                end)
+              end)
+            end)
+          end)
+        end)
+      ]]))
+    end)
   end)
 
   describe('vim.api.nvim_win_call', function()