Skip to content

Commit cee5220

Browse files
committed
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)
1 parent e49b4bb commit cee5220

6 files changed

Lines changed: 53 additions & 2 deletions

File tree

src/buffer.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,7 @@ close_buffer(
508508
int wipe_buf = (action == DOBUF_WIPE || action == DOBUF_WIPE_REUSE);
509509
int del_buf = (action == DOBUF_DEL || wipe_buf);
510510

511+
CHECK_CURBUF;
511512
/*
512513
* Force unloading or deleting when 'bufhidden' says so.
513514
* The caller must take care of NOT deleting/freeing when 'bufhidden' is
@@ -530,6 +531,7 @@ close_buffer(
530531
#ifdef FEAT_TERMINAL
531532
if (bt_terminal(buf) && (buf->b_nwindows == 1 || del_buf))
532533
{
534+
CHECK_CURBUF;
533535
if (term_job_running(buf->b_term))
534536
{
535537
if (wipe_buf || unload_buf)
@@ -554,6 +556,7 @@ close_buffer(
554556
unload_buf = TRUE;
555557
wipe_buf = TRUE;
556558
}
559+
CHECK_CURBUF;
557560
}
558561
#endif
559562

@@ -743,6 +746,7 @@ close_buffer(
743746
if (del_buf)
744747
buf->b_p_bl = FALSE;
745748
}
749+
// NOTE: at this point "curbuf" may be invalid!
746750
}
747751

748752
/*
@@ -933,7 +937,11 @@ free_buffer(buf_T *buf)
933937
au_pending_free_buf = buf;
934938
}
935939
else
940+
{
936941
vim_free(buf);
942+
if (curbuf == buf)
943+
curbuf = NULL; // make clear it's not to be used
944+
}
937945
}
938946

939947
/*

src/macros.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,8 +364,11 @@
364364
# define ESTACK_CHECK_SETUP estack_len_before = exestack.ga_len;
365365
# define ESTACK_CHECK_NOW if (estack_len_before != exestack.ga_len) \
366366
siemsg("Exestack length expected: %d, actual: %d", estack_len_before, exestack.ga_len);
367+
# define CHECK_CURBUF if (curwin != NULL && curwin->w_buffer != curbuf) \
368+
iemsg("curbuf != curwin->w_buffer")
367369
#else
368370
# define ESTACK_CHECK_DECLARATION
369371
# define ESTACK_CHECK_SETUP
370372
# define ESTACK_CHECK_NOW
373+
# define CHECK_CURBUF
371374
#endif

src/popupwin.c

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2135,7 +2135,7 @@ popup_close_and_callback(win_T *wp, typval_T *arg)
21352135
break;
21362136
if (owp != NULL)
21372137
win_enter(owp, FALSE);
2138-
else if (win_valid(prevwin))
2138+
else if (win_valid(prevwin) && wp != prevwin)
21392139
win_enter(prevwin, FALSE);
21402140
else
21412141
win_enter(firstwin, FALSE);
@@ -2147,11 +2147,13 @@ popup_close_and_callback(win_T *wp, typval_T *arg)
21472147
if (wp == curwin && ERROR_IF_POPUP_WINDOW)
21482148
return;
21492149

2150+
CHECK_CURBUF;
21502151
if (wp->w_close_cb.cb_name != NULL)
21512152
// Careful: This may make "wp" invalid.
21522153
invoke_popup_callback(wp, arg);
21532154

21542155
popup_close(id);
2156+
CHECK_CURBUF;
21552157
}
21562158

21572159
void
@@ -2505,6 +2507,11 @@ popup_close(int id)
25052507
for (wp = first_popupwin; wp != NULL; prev = wp, wp = wp->w_next)
25062508
if (wp->w_id == id)
25072509
{
2510+
if (wp == curwin)
2511+
{
2512+
ERROR_IF_ANY_POPUP_WINDOW;
2513+
return;
2514+
}
25082515
if (prev == NULL)
25092516
first_popupwin = wp->w_next;
25102517
else
@@ -2531,6 +2538,11 @@ popup_close_tabpage(tabpage_T *tp, int id)
25312538
for (wp = *root; wp != NULL; prev = wp, wp = wp->w_next)
25322539
if (wp->w_id == id)
25332540
{
2541+
if (wp == curwin)
2542+
{
2543+
ERROR_IF_ANY_POPUP_WINDOW;
2544+
return;
2545+
}
25342546
if (prev == NULL)
25352547
*root = wp->w_next;
25362548
else
@@ -2881,10 +2893,11 @@ error_if_popup_window(int also_with_term UNUSED)
28812893
{
28822894
// win_execute() may set "curwin" to a popup window temporarily, but many
28832895
// commands are disallowed then. When a terminal runs in the popup most
2884-
// things are allowed.
2896+
// things are allowed. When a terminal is finished it can be closed.
28852897
if (WIN_IS_POPUP(curwin)
28862898
# ifdef FEAT_TERMINAL
28872899
&& (also_with_term || curbuf->b_term == NULL)
2900+
&& !term_is_finished(curbuf)
28882901
# endif
28892902
)
28902903
{

src/terminal.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,7 @@ term_close_buffer(buf_T *buf, buf_T *old_curbuf)
382382
curwin->w_buffer = curbuf;
383383
++curbuf->b_nwindows;
384384
}
385+
CHECK_CURBUF;
385386

386387
// Wiping out the buffer will also close the window and call
387388
// free_terminal().

src/testdir/test_terminal.vim

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2430,3 +2430,27 @@ func Test_hidden_terminal()
24302430
call assert_equal('', bufname('^$'))
24312431
call StopShellInTerminal(buf)
24322432
endfunc
2433+
2434+
func Test_term_nasty_callback()
2435+
func OpenTerms()
2436+
set hidden
2437+
let g:buf0 = term_start('sh', #{hidden: 1})
2438+
call popup_create(g:buf0, {})
2439+
let g:buf1 = term_start('sh', #{hidden: 1, term_finish: 'close'})
2440+
call popup_create(g:buf1, {})
2441+
let g:buf2 = term_start(['sh', '-c'], #{curwin: 1, exit_cb: function('TermExit')})
2442+
sleep 100m
2443+
call popup_close(win_getid())
2444+
endfunc
2445+
func TermExit(...)
2446+
call term_sendkeys(bufnr('#'), "exit\<CR>")
2447+
call popup_close(win_getid())
2448+
endfu
2449+
call OpenTerms()
2450+
2451+
call term_sendkeys(g:buf0, "exit\<CR>")
2452+
sleep 50m
2453+
exe g:buf0 .. 'bwipe'
2454+
set hidden&
2455+
endfunc
2456+

src/version.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,8 @@ static char *(features[]) =
738738

739739
static int included_patches[] =
740740
{ /* Add new patch number below this line */
741+
/**/
742+
371,
741743
/**/
742744
370,
743745
/**/

0 commit comments

Comments
 (0)