From cee52204ca030ce7814844e4dab8b4ed897ba3cc Mon Sep 17 00:00:00 2001 From: Bram Moolenaar Date: Wed, 11 Mar 2020 14:19:58 +0100 Subject: 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) --- src/buffer.c | 8 ++++++++ src/macros.h | 3 +++ src/popupwin.c | 17 +++++++++++++++-- src/terminal.c | 1 + src/testdir/test_terminal.vim | 24 ++++++++++++++++++++++++ src/version.c | 2 ++ 6 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/buffer.c b/src/buffer.c index 25620afb5..2dcbae2ae 100644 --- 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 a7964d850..803b89115 100644 --- 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 3c85e3d56..1b56c1c4b 100644 --- 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 d4c605ed8..34316d843 100644 --- 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 8cbd6a665..7bdabb892 100644 --- 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\") + call popup_close(win_getid()) + endfu + call OpenTerms() + + call term_sendkeys(g:buf0, "exit\") + sleep 50m + exe g:buf0 .. 'bwipe' + set hidden& +endfunc + diff --git a/src/version.c b/src/version.c index 34eb68fb2..eb76880dc 100644 --- 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, /**/ -- cgit v1.2.1