vim-patch:8.1.1756: autocommand that splits window messes up window layout

Problem:    Autocommand that splits window messes up window layout.
Solution:   Disallow splitting a window while closing one.  In ":all" give an
            error when moving a window will not work.
https://github.com/vim/vim/commit/1417c766f55e5959b31da488417b7d9b141404af

Expected error number was changed to E242 in Vim in patch 8.2.1183, and
patch 8.2.2420 (which has already been ported) made the test no longer
throw E249 in Vim, so just use E242 in the test.

4 files changed, 64 insertions, 14 deletions

diff --git a/src/nvim/buffer.c b/src/nvim/buffer.c
index 4d914acea4..78426568b4 100644
--- a/src/nvim/buffer.c
+++ b/src/nvim/buffer.c
@@ -4855,6 +4855,10 @@ void do_arg_all(int count, int forceit, int keep_tabs)
             if (keep_tabs) {
               new_curwin = wp;
               new_curtab = curtab;
+            } else if (wp->w_frame->fr_parent != curwin->w_frame->fr_parent) {
+              emsg(_("E249: window layout changed unexpectedly"));
+              i = count;
+              break;
             } else {
               win_move_after(wp, curwin);
             }
diff --git a/src/nvim/testdir/test_window_cmd.vim b/src/nvim/testdir/test_window_cmd.vim
index ef6dec580f..798122dc5d 100644
--- a/src/nvim/testdir/test_window_cmd.vim
+++ b/src/nvim/testdir/test_window_cmd.vim
@@ -513,14 +513,15 @@ func Test_window_colon_command()
 endfunc
 
 func Test_access_freed_mem()
+  call assert_equal(&columns, winwidth(0))
   " This was accessing freed memory (but with what events?)
   au BufEnter,BufLeave,WinEnter,WinLeave 0 vs xxx
   arg 0
   argadd
-  all
-  all
+  call assert_fails("all", "E242:")
   au!
   bwipe xxx
+  call assert_equal(&columns, winwidth(0))
 endfunc
 
 func Test_visual_cleared_after_window_split()
diff --git a/src/nvim/window.c b/src/nvim/window.c
index 2ca5128445..d1cc5f245a 100644
--- a/src/nvim/window.c
+++ b/src/nvim/window.c
@@ -74,6 +74,35 @@ typedef enum {
 
 static char *m_onlyone = N_("Already only one window");
 
+/// When non-zero splitting a window is forbidden.  Used to avoid that nasty
+/// autocommands mess up the window structure.
+static int split_disallowed = 0;
+
+// #define WIN_DEBUG
+#ifdef WIN_DEBUG
+/// Call this method to log the current window layout.
+static void log_frame_layout(frame_T *frame)
+{
+  DLOG("layout %s, wi: %d, he: %d, wwi: %d, whe: %d, id: %d",
+       frame->fr_layout == FR_LEAF ? "LEAF" : frame->fr_layout == FR_ROW ? "ROW" : "COL",
+       frame->fr_width,
+       frame->fr_height,
+       frame->fr_win == NULL ? -1 : frame->fr_win->w_width,
+       frame->fr_win == NULL ? -1 : frame->fr_win->w_height,
+       frame->fr_win == NULL ? -1 : frame->fr_win->w_id);
+  if (frame->fr_child != NULL) {
+    DLOG("children");
+    log_frame_layout(frame->fr_child);
+    if (frame->fr_next != NULL) {
+      DLOG("END of children");
+    }
+  }
+  if (frame->fr_next != NULL) {
+    log_frame_layout(frame->fr_next);
+  }
+}
+#endif
+
 /// @return the current window, unless in the cmdline window and "prevwin" is
 /// set, then return "prevwin".
 win_T *prevwin_curwin(void)
@@ -909,6 +938,17 @@ void ui_ext_win_viewport(win_T *wp)
   }
 }
 
+/// If "split_disallowed" is set given an error and return FAIL.
+/// Otherwise return OK.
+static int check_split_disallowed(void)
+{
+  if (split_disallowed > 0) {
+    emsg(_("E242: Can't split a window while closing another"));
+    return FAIL;
+  }
+  return OK;
+}
+
 /*
  * split the current window, implements CTRL-W s and :split
  *
@@ -937,6 +977,9 @@ int win_split(int size, int flags)
     emsg(_("E442: Can't split topleft and botright at the same time"));
     return FAIL;
   }
+  if (check_split_disallowed() == FAIL) {
+    return FAIL;
+  }
 
   // When creating the help window make a snapshot of the window layout.
   // Otherwise clear the snapshot, it's now invalid.
@@ -1886,6 +1929,9 @@ static void win_totop(int size, int flags)
   if (curwin == aucmd_win) {
     return;
   }
+  if (check_split_disallowed() == FAIL) {
+    return;
+  }
 
   if (curwin->w_floating) {
     ui_comp_remove_grid(&curwin->w_grid_alloc);
@@ -1929,6 +1975,11 @@ void win_move_after(win_T *win1, win_T *win2)
 
   // check if there is something to do
   if (win2->w_next != win1) {
+    if (win1->w_frame->fr_parent != win2->w_frame->fr_parent) {
+      iemsg("INTERNAL: trying to move a window into another frame");
+      return;
+    }
+
     // may need move the status line, horizontal or vertical separator of the last window
     if (win1 == lastwin) {
       height = win1->w_prev->w_status_height;
@@ -2742,6 +2793,10 @@ int win_close(win_T *win, bool free_buf, bool force)
     return FAIL;
   }
 
+  // Now we are really going to close the window.  Disallow any autocommand
+  // to split a window to avoid trouble.
+  split_disallowed++;
+
   // let terminal buffers know that this window dimensions may be ignored
   win->w_closing = true;
 
@@ -2809,6 +2864,8 @@ int win_close(win_T *win, bool free_buf, bool force)
     }
   }
 
+  split_disallowed--;
+
   /*
    * If last window has a status line now and we don't want one,
    * remove the status line.
diff --git a/test/functional/autocmd/autocmd_spec.lua b/test/functional/autocmd/autocmd_spec.lua
index 6111654b5e..b8d2c9ec1d 100644
--- a/test/functional/autocmd/autocmd_spec.lua
+++ b/test/functional/autocmd/autocmd_spec.lua
@@ -548,18 +548,6 @@ describe('autocmd', function()
       neq({}, meths.get_autocmds { group = "filetypedetect" })
     end)
 
-    it('should not access freed mem', function()
-      source [[
-        au BufEnter,BufLeave,WinEnter,WinLeave 0 vs xxx
-        arg 0
-        argadd
-        all
-        all
-        au!
-        bwipe xxx
-      ]]
-    end)
-
     it('should allow comma-separated patterns', function()
       source [[
         augroup TestingPatterns