Mercurial > vim

changeset 19629:804322d6c6ba v8.2.0371

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
patch 8.2.0371: crash with combination of terminal popup and autocmd Commit: https://github.com/vim/vim/commit/cee52204ca030ce7814844e4dab8b4ed897ba3cc Author: Bram Moolenaar <Bram@vim.org> Date: Wed Mar 11 14:19:58 2020 +0100 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)
author Bram Moolenaar <Bram@vim.org>
date Wed, 11 Mar 2020 14:30:04 +0100
parents a9fb9a329b79
children 522158d28042
files src/buffer.c src/macros.h src/popupwin.c src/terminal.c src/testdir/test_terminal.vim src/version.c
diffstat 6 files changed, 53 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- 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
+    }
 }
 
 /*
--- 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
--- a/src/popupwin.c
+++ b/src/popupwin.c
@@ -2135,7 +2135,7 @@ popup_close_and_callback(win_T *wp, typv
 		    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, typv
     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 i
     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
 {
     // 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
 	    )
     {
--- a/src/terminal.c
+++ b/src/terminal.c
@@ -382,6 +382,7 @@ term_close_buffer(buf_T *buf, buf_T *old
 	curwin->w_buffer = curbuf;
 	++curbuf->b_nwindows;
     }
+    CHECK_CURBUF;
 
     // Wiping out the buffer will also close the window and call
     // free_terminal().
--- 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
+
--- a/src/version.c
+++ b/src/version.c
@@ -739,6 +739,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    371,
+/**/
     370,
 /**/
     369,