Here are 3 patches that fix 2 particular problems with closing windows while executing an autocommand.
The first one is about Neovim issue 6308 (https://github.com/neovim/neovim/issues/6308) which is about a crash on wipeing out buffers on SessionLoadPost autocommand. I believe the reason is, that the autocommand removed one window in another tabpage when executing the autocommand (and left the cursor in that other tabpage). And in aucmd_restbuf we are left with a NULL curwin, causing the segfault later on. While looking into it, I also noticed that when trying to close the autocommand window in the autocommand and provoking E814, this will lead to running into an endless loop, because in close_windows() Vim loops over all windows and after calling win_close() it starts from the beginning: ,---- | for (wp = firstwin; wp != NULL && !ONE_WINDOW; ) | { | if (wp->w_buffer == buf && (!keep_curwin || wp != curwin) | #ifdef FEAT_AUTOCMD | && !(wp->w_closing || wp->w_buffer->b_locked > 0) | #endif | ) | { | win_close(wp, FALSE); | | /* Start all over, autocommands may change the window layout. */ | wp = firstwin; `---- However, if win_close() returns an error and can't close the window the loop will never return. So fix this as well. And finally, the third patch adds a test for both bugs. Note if you run the test with an unpatched Vim, the endless loop bug will prevent it from terminating. Best, Christian -- Mit Absicht handeln ist das, was den Menschen über geringere Geschöpfe erhebt. -- Gotthold Ephraim Lessing -- -- You received this message from the "vim_dev" maillist. Do not top-post! Type your reply below the text you are replying to. For more information, visit http://www.vim.org/maillist.php --- You received this message because you are subscribed to the Google Groups "vim_dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
From 16e4e3c858363d5204c17ab242d558fe98cabafa Mon Sep 17 00:00:00 2001 From: Christian Brabandt <[email protected]> Date: Sun, 19 Mar 2017 10:08:07 +0100 Subject: [PATCH 1/3] Do not segfault on SessionLoadPost when in a SessionLoadPost autocommand all windows are wiped out Vim will segfault, because in aucmd_restbuf we might end up with a tabpage containing no valid window (since it closes the autocommand window it created earlier). So make sure, that the current tabpage is still valid and if not, close it properly. --- src/fileio.c | 5 +++++ src/proto/window.pro | 2 ++ src/window.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+) diff --git a/src/fileio.c b/src/fileio.c index 32b2059f0..5004850ce 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -9033,6 +9033,11 @@ win_found: win_remove(curwin, NULL); aucmd_win_used = FALSE; last_status(FALSE); /* may need to remove last status line */ + + /* no valid window in current tabpage */ + if (!valid_tabpage_win(curtab)) + close_tabpage(curtab); + restore_snapshot(SNAP_AUCMD_IDX, FALSE); (void)win_comp_pos(); /* recompute window positions */ unblock_autocmds(); diff --git a/src/proto/window.pro b/src/proto/window.pro index 8b649dbf1..e53123c55 100644 --- a/src/proto/window.pro +++ b/src/proto/window.pro @@ -26,6 +26,8 @@ int win_new_tabpage(int after); int may_open_tabpage(void); int make_tabpages(int maxcount); int valid_tabpage(tabpage_T *tpc); +int valid_tabpage_win(tabpage_T *tpc); +void close_tabpage(tabpage_T *tpc); tabpage_T *find_tabpage(int n); int tabpage_index(tabpage_T *ftp); void goto_tabpage(int n); diff --git a/src/window.c b/src/window.c index 43c9ed2c0..a570a6fec 100644 --- a/src/window.c +++ b/src/window.c @@ -3755,6 +3755,57 @@ valid_tabpage(tabpage_T *tpc) } /* + * Return TRUE when "tpc" points to a valid tab page and at least one window is + * valid + */ + int +valid_tabpage_win(tabpage_T *tpc) +{ + tabpage_T *tp; + win_T *wp; + + FOR_ALL_TABPAGES(tp) + { + if (tp == tpc) + { + FOR_ALL_WINDOWS_IN_TAB(tp, wp) + { + if (win_valid(wp)) + return TRUE; + } + return FALSE; + } + } + /* shouldn't happen */ + return FALSE; +} + +/* close tabpage and all windows in it */ + void +close_tabpage(tabpage_T *tab) +{ + tabpage_T *ptp; + win_T *wp; + int dir; + + if (tab == first_tabpage) + first_tabpage = tab->tp_next; + else + { + for (ptp = first_tabpage; ptp != NULL && ptp->tp_next != tab; + ptp = ptp->tp_next) + ; + ptp->tp_next = tab->tp_next; + } + + /* close all remaining windows */ + FOR_ALL_WINDOWS_IN_TAB(tab, wp) + win_free_mem(wp, &dir, tab); + goto_tabpage_tp(ptp, FALSE, FALSE); + free_tabpage(tab); +} + +/* * Find tab page "n" (first one is 1). Returns NULL when not found. */ tabpage_T * -- 2.11.0
From b6dd3e0415b92e9e16fecd5d63a6541269dc5d58 Mon Sep 17 00:00:00 2001 From: Christian Brabandt <[email protected]> Date: Sun, 19 Mar 2017 10:33:29 +0100 Subject: [PATCH 2/3] Make sure, closing Windows won't endless loop If from an autocommand win_close() is called from close_windows() and win_close() fails, we will run in an endless loop, and keep on outputting e.g. E813 and not return anymore. Make sure, the loop terminates correctly. --- src/window.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/window.c b/src/window.c index a570a6fec..24940e743 100644 --- a/src/window.c +++ b/src/window.c @@ -2131,7 +2131,8 @@ close_windows( #endif ) { - win_close(wp, FALSE); + if (win_close(wp, FALSE) == FAIL) + break; /* Start all over, autocommands may change the window layout. */ wp = firstwin; -- 2.11.0
From eda62af2688e9895f03a42974de9b97755b11657 Mon Sep 17 00:00:00 2001 From: Christian Brabandt <[email protected]> Date: Sun, 19 Mar 2017 14:11:55 +0100 Subject: [PATCH 3/3] Add test for fixes --- src/testdir/test_autocmd.vim | 63 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/src/testdir/test_autocmd.vim b/src/testdir/test_autocmd.vim index 566a07c6f..ccafc3015 100644 --- a/src/testdir/test_autocmd.vim +++ b/src/testdir/test_autocmd.vim @@ -341,3 +341,66 @@ func Test_BufEnter() call delete('Xdir', 'd') au! BufEnter endfunc + +" Closing a window might cause an endless loop +" E814 for older Vims +function Test_autocmd_bufwipe_in_SessLoadPost() + tabnew + set noswapfile + let g:bufnr=bufnr('%') + mksession! + + let content=['set nocp noswapfile', + \ 'let v:swapchoice="e"', + \ 'augroup test_autocmd_sessionload', + \ 'autocmd!', + \ 'autocmd SessionLoadPost * 4bw!', + \ 'augroup END' + \ ] + call writefile(content, 'Xvimrc') + let a=system(v:progpath. ' -u Xvimrc --noplugins -S Session.vim') + call assert_match('E814', a) + + unlet! g:bufnr + set swapfile + for file in ['Session.vim', 'Xvimrc'] + call delete(file) + endfor +endfunc + +" SEGV occurs in older versions. +function Test_autocmd_bufwipe_in_SessLoadPost2() + tabnew + set noswapfile + let g:bufnr=bufnr('%') + mksession! + + let content = ['set nocp noswapfile', + \ 'function! DeleteInactiveBufs()', + \ ' tabfirst', + \ ' let tabblist = []', + \ ' for i in range(1, tabpagenr(''$''))', + \ ' call extend(tabblist, tabpagebuflist(i))', + \ ' endfor', + \ ' for b in range(1, bufnr(''$''))', + \ ' if bufexists(b) && buflisted(b) && (index(tabblist, b) == -1 || bufname(b) =~# ''^$'')', + \ ' exec ''bwipeout '' . b', + \ ' endif', + \ ' endfor', + \ 'call append("1", "SessionLoadPost DONE")', + \ 'endfunction', + \ 'au SessionLoadPost * call DeleteInactiveBufs()'] + call writefile(content, 'Xvimrc') + let a=system(v:progpath. ' -u Xvimrc --noplugins -S Session.vim') + " this probably only matches on unix + if has("unix") + call assert_notmatch('Caught deadly signal SEGV', a) + endif + call assert_match('SessionLoadPost DONE', a) + + unlet! g:bufnr + set swapfile + for file in ['Session.vim', 'Xvimrc'] + call delete(file) + endfor +endfunc -- 2.11.0
