patch 8.2.0371: crash with combination of terminal popup and autocmd

Problem:    Crash with combination of terminal popup and autocmd.
Solution:   Disallow closing a popup that is the current window.  Add a check
            that the current buffer is valid. (closes #5754)

diff --git a/src/buffer.c b/src/buffer.c
index 25620afb5dcb416d29eef3b7cd545773074c2b02..2dcbae2aea02542eea3730b39390b0d920174f6b 100644 (file)
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -508,6 +508,7 @@ close_buffer(
     int                wipe_buf = (action == DOBUF_WIPE || action == DOBUF_WIPE_REUSE);
     int                del_buf = (action == DOBUF_DEL || wipe_buf);
 
+    CHECK_CURBUF;
     /*
      * Force unloading or deleting when 'bufhidden' says so.
      * The caller must take care of NOT deleting/freeing when 'bufhidden' is
@@ -530,6 +531,7 @@ close_buffer(
 #ifdef FEAT_TERMINAL
     if (bt_terminal(buf) && (buf->b_nwindows == 1 || del_buf))
     {
+       CHECK_CURBUF;
        if (term_job_running(buf->b_term))
        {
            if (wipe_buf || unload_buf)
@@ -554,6 +556,7 @@ close_buffer(
            unload_buf = TRUE;
            wipe_buf = TRUE;
        }
+       CHECK_CURBUF;
     }
 #endif
 
@@ -743,6 +746,7 @@ aucmd_abort:
        if (del_buf)
            buf->b_p_bl = FALSE;
     }
+    // NOTE: at this point "curbuf" may be invalid!
 }
 
 /*
@@ -933,7 +937,11 @@ free_buffer(buf_T *buf)
        au_pending_free_buf = buf;
     }
     else
+    {
        vim_free(buf);
+       if (curbuf == buf)
+           curbuf = NULL;  // make clear it's not to be used
+    }
 }
 
 /*

diff --git a/src/macros.h b/src/macros.h
index a7964d850a143fb338c832c208ea7cca6565661f..803b89115f0aea93a28360642d2ee8c3ee50c296 100644 (file)
--- a/src/macros.h
+++ b/src/macros.h
@@ -364,8 +364,11 @@
 # define ESTACK_CHECK_SETUP estack_len_before = exestack.ga_len;
 # define ESTACK_CHECK_NOW if (estack_len_before != exestack.ga_len) \
        siemsg("Exestack length expected: %d, actual: %d", estack_len_before, exestack.ga_len);
+# define CHECK_CURBUF if (curwin != NULL && curwin->w_buffer != curbuf) \
+               iemsg("curbuf != curwin->w_buffer")
 #else
 # define ESTACK_CHECK_DECLARATION
 # define ESTACK_CHECK_SETUP
 # define ESTACK_CHECK_NOW
+# define CHECK_CURBUF
 #endif

diff --git a/src/popupwin.c b/src/popupwin.c
index 3c85e3d567bbea825678948e7d9459103cfe7c49..1b56c1c4bede7f846502c0e632c7a044083c8e65 100644 (file)
--- a/src/popupwin.c
+++ b/src/popupwin.c
@@ -2135,7 +2135,7 @@ popup_close_and_callback(win_T *wp, typval_T *arg)
                    break;
            if (owp != NULL)
                win_enter(owp, FALSE);
-           else if (win_valid(prevwin))
+           else if (win_valid(prevwin) && wp != prevwin)
                win_enter(prevwin, FALSE);
            else
                win_enter(firstwin, FALSE);
@@ -2147,11 +2147,13 @@ popup_close_and_callback(win_T *wp, typval_T *arg)
     if (wp == curwin && ERROR_IF_POPUP_WINDOW)
        return;
 
+    CHECK_CURBUF;
     if (wp->w_close_cb.cb_name != NULL)
        // Careful: This may make "wp" invalid.
        invoke_popup_callback(wp, arg);
 
     popup_close(id);
+    CHECK_CURBUF;
 }
 
     void
@@ -2505,6 +2507,11 @@ popup_close(int id)
     for (wp = first_popupwin; wp != NULL; prev = wp, wp = wp->w_next)
        if (wp->w_id == id)
        {
+           if (wp == curwin)
+           {
+               ERROR_IF_ANY_POPUP_WINDOW;
+               return;
+           }
            if (prev == NULL)
                first_popupwin = wp->w_next;
            else
@@ -2531,6 +2538,11 @@ popup_close_tabpage(tabpage_T *tp, int id)
     for (wp = *root; wp != NULL; prev = wp, wp = wp->w_next)
        if (wp->w_id == id)
        {
+           if (wp == curwin)
+           {
+               ERROR_IF_ANY_POPUP_WINDOW;
+               return;
+           }
            if (prev == NULL)
                *root = wp->w_next;
            else
@@ -2881,10 +2893,11 @@ error_if_popup_window(int also_with_term UNUSED)
 {
     // win_execute() may set "curwin" to a popup window temporarily, but many
     // commands are disallowed then.  When a terminal runs in the popup most
-    // things are allowed.
+    // things are allowed.  When a terminal is finished it can be closed.
     if (WIN_IS_POPUP(curwin)
 # ifdef FEAT_TERMINAL
            && (also_with_term || curbuf->b_term == NULL)
+           && !term_is_finished(curbuf)
 # endif
            )
     {

diff --git a/src/terminal.c b/src/terminal.c
index d4c605ed8fbd93a7f7f68e2f2cee1849041296a7..34316d8439320d01e162d1dfdd7b20c3ff47b602 100644 (file)
--- a/src/terminal.c
+++ b/src/terminal.c
@@ -382,6 +382,7 @@ term_close_buffer(buf_T *buf, buf_T *old_curbuf)
        curwin->w_buffer = curbuf;
        ++curbuf->b_nwindows;
     }
+    CHECK_CURBUF;
 
     // Wiping out the buffer will also close the window and call
     // free_terminal().

diff --git a/src/testdir/test_terminal.vim b/src/testdir/test_terminal.vim
index 8cbd6a665a56dd94000bbfdb49c30559b2a50602..7bdabb892971e7cf6365435e38e9d39bb16291ee 100644 (file)
--- a/src/testdir/test_terminal.vim
+++ b/src/testdir/test_terminal.vim
@@ -2430,3 +2430,27 @@ func Test_hidden_terminal()
   call assert_equal('', bufname('^$'))
   call StopShellInTerminal(buf)
 endfunc
+
+func Test_term_nasty_callback()
+  func OpenTerms()
+    set hidden
+    let g:buf0 = term_start('sh', #{hidden: 1})
+    call popup_create(g:buf0, {})
+    let g:buf1 = term_start('sh', #{hidden: 1, term_finish: 'close'})
+    call popup_create(g:buf1, {})
+    let g:buf2 = term_start(['sh', '-c'], #{curwin: 1, exit_cb: function('TermExit')})
+    sleep 100m
+    call popup_close(win_getid())
+  endfunc
+  func TermExit(...)
+    call term_sendkeys(bufnr('#'), "exit\<CR>")
+    call popup_close(win_getid())
+  endfu
+  call OpenTerms()
+
+  call term_sendkeys(g:buf0, "exit\<CR>")
+  sleep 50m
+  exe g:buf0 .. 'bwipe'
+  set hidden&
+endfunc
+

diff --git a/src/version.c b/src/version.c
index 34eb68fb253683617f07a7749f4b759977f59157..eb76880dc92edd5028c36f5a7189f549f5177f55 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -738,6 +738,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    371,
 /**/
     370,
 /**/