patch 8.1.0892: failure when closing a window when location list is in usev8.1.0892

Problem:    Failure when closing a window when location list is in use.
Solution:   Handle the situation gracefully. Make sure memory for 'switchbuf'
            is not freed at the wrong time. (Yegappan Lakshmanan,
            closes #3928)

7 files changed, 95 insertions, 36 deletions

diff --git a/src/eval.c b/src/eval.c
index 08603cad6a..c30c5d83c0 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -8587,7 +8587,7 @@ find_win_by_nr_or_id(typval_T *vp)
     int	nr = (int)tv_get_number_chk(vp, NULL);
 
     if (nr >= LOWEST_WIN_ID)
-	return win_id2wp(vp);
+	return win_id2wp(tv_get_number(vp));
     return find_win_by_nr(vp, NULL);
 }
 
diff --git a/src/evalfunc.c b/src/evalfunc.c
index eb082b771b..c18ab31251 100644
--- a/src/evalfunc.c
+++ b/src/evalfunc.c
@@ -5800,7 +5800,7 @@ f_getwininfo(typval_T *argvars, typval_T *rettv)
 
     if (argvars[0].v_type != VAR_UNKNOWN)
     {
-	wparg = win_id2wp(argvars);
+	wparg = win_id2wp(tv_get_number(&argvars[0]));
 	if (wparg == NULL)
 	    return;
     }
diff --git a/src/proto/window.pro b/src/proto/window.pro
index 4527a37f42..27f77c4758 100644
--- a/src/proto/window.pro
+++ b/src/proto/window.pro
@@ -91,7 +91,7 @@ int get_tab_number(tabpage_T *tp);
 int win_getid(typval_T *argvars);
 int win_gotoid(typval_T *argvars);
 void win_id2tabwin(typval_T *argvars, list_T *list);
-win_T *win_id2wp(typval_T *argvars);
+win_T *win_id2wp(int id);
 int win_id2win(typval_T *argvars);
 void win_findbuf(typval_T *argvars, list_T *list);
 void get_framelayout(frame_T *fr, list_T *l, int outer);
diff --git a/src/quickfix.c b/src/quickfix.c
index dba6b63bbe..ea2e8c9d18 100644
--- a/src/quickfix.c
+++ b/src/quickfix.c
@@ -1899,23 +1899,24 @@ ll_free_all(qf_info_T **pqi)
 	return;
     *pqi = NULL;	// Remove reference to this list
 
+    // If the location list is still in use, then queue the delete request
+    // to be processed later.
+    if (quickfix_busy > 0)
+    {
+	locstack_queue_delreq(qi);
+	return;
+    }
+
     qi->qf_refcount--;
     if (qi->qf_refcount < 1)
     {
 	// No references to this location list.
-	// If the location list is still in use, then queue the delete request
-	// to be processed later.
-	if (quickfix_busy > 0)
-	    locstack_queue_delreq(qi);
-	else
-	{
-	    // If the quickfix window buffer is loaded, then wipe it
-	    wipe_qf_buffer(qi);
+	// If the quickfix window buffer is loaded, then wipe it
+	wipe_qf_buffer(qi);
 
-	    for (i = 0; i < qi->qf_listcount; ++i)
-		qf_free(&qi->qf_lists[i]);
-	    vim_free(qi);
-	}
+	for (i = 0; i < qi->qf_listcount; ++i)
+	    qf_free(&qi->qf_lists[i]);
+	vim_free(qi);
     }
 }
 
@@ -3018,7 +3019,7 @@ qf_jump_edit_buffer(
 	qf_info_T	*qi,
 	qfline_T	*qf_ptr,
 	int		forceit,
-	win_T		*oldwin,
+	int		prev_winid,
 	int		*opened_window)
 {
     qf_list_T	*qfl = &qi->qf_lists[qi->qf_curlist];
@@ -3039,7 +3040,7 @@ qf_jump_edit_buffer(
 
 	retval = do_ecmd(qf_ptr->qf_fnum, NULL, NULL, NULL, (linenr_T)1,
 		ECMD_HIDE + ECMD_SET_HELP,
-		oldwin == curwin ? curwin : NULL);
+		prev_winid == curwin->w_id ? curwin : NULL);
     }
     else
 	retval = buflist_getfile(qf_ptr->qf_fnum,
@@ -3047,11 +3048,15 @@ qf_jump_edit_buffer(
 
     // If a location list, check whether the associated window is still
     // present.
-    if (qfl_type == QFLT_LOCATION && !win_valid_any_tab(oldwin))
+    if (qfl_type == QFLT_LOCATION)
     {
-	emsg(_("E924: Current window was closed"));
-	*opened_window = FALSE;
-	return NOTDONE;
+	win_T	*wp = win_id2wp(prev_winid);
+	if (wp == NULL && curwin->w_llist != qi)
+	{
+	    emsg(_("E924: Current window was closed"));
+	    *opened_window = FALSE;
+	    return NOTDONE;
+	}
     }
 
     if (qfl_type == QFLT_QUICKFIX && !qflist_valid(NULL, save_qfid))
@@ -3211,7 +3216,7 @@ qf_jump_to_buffer(
 	int		qf_index,
 	qfline_T	*qf_ptr,
 	int		forceit,
-	win_T		*oldwin,
+	int		prev_winid,
 	int		*opened_window,
 	int		openfold,
 	int		print_message)
@@ -3227,7 +3232,7 @@ qf_jump_to_buffer(
 
     if (qf_ptr->qf_fnum != 0)
     {
-	retval = qf_jump_edit_buffer(qi, qf_ptr, forceit, oldwin,
+	retval = qf_jump_edit_buffer(qi, qf_ptr, forceit, prev_winid,
 						opened_window);
 	if (retval != OK)
 	    return retval;
@@ -3287,8 +3292,8 @@ qf_jump_newwin(qf_info_T	*qi,
     int			old_qf_index;
     char_u		*old_swb = p_swb;
     unsigned		old_swb_flags = swb_flags;
+    int			prev_winid;
     int			opened_window = FALSE;
-    win_T		*oldwin = curwin;
     int			print_message = TRUE;
 #ifdef FEAT_FOLDING
     int			old_KeyTyped = KeyTyped; // getting file may reset it
@@ -3304,6 +3309,8 @@ qf_jump_newwin(qf_info_T	*qi,
 	return;
     }
 
+    incr_quickfix_busy();
+
     qfl = &qi->qf_lists[qi->qf_curlist];
 
     qf_ptr = qfl->qf_ptr;
@@ -3325,13 +3332,15 @@ qf_jump_newwin(qf_info_T	*qi,
 	// window
 	print_message = FALSE;
 
+    prev_winid = curwin->w_id;
+
     retval = qf_jump_open_window(qi, qf_ptr, newwin, &opened_window);
     if (retval == FAIL)
 	goto failed;
     if (retval == NOTDONE)
 	goto theend;
 
-    retval = qf_jump_to_buffer(qi, qf_index, qf_ptr, forceit, oldwin,
+    retval = qf_jump_to_buffer(qi, qf_index, qf_ptr, forceit, prev_winid,
 	    &opened_window, old_KeyTyped, print_message);
     if (retval == NOTDONE)
     {
@@ -3359,7 +3368,7 @@ theend:
 	qfl->qf_ptr = qf_ptr;
 	qfl->qf_index = qf_index;
     }
-    if (p_swb != old_swb && opened_window)
+    if (p_swb != old_swb)
     {
 	// Restore old 'switchbuf' value, but not when an autocommand or
 	// modeline has changed the value.
@@ -3371,6 +3380,7 @@ theend:
 	else
 	    free_string_option(old_swb);
     }
+    decr_quickfix_busy();
 }
 
 // Highlight attributes used for displaying entries from the quickfix list.
@@ -4004,9 +4014,9 @@ qf_open_new_cwindow(qf_info_T *qi, int height)
     if (IS_LL_STACK(qi))
     {
 	// For the location list window, create a reference to the
-	// location list from the window 'win'.
-	curwin->w_llist_ref = win->w_llist;
-	win->w_llist->qf_refcount++;
+	// location list stack from the window 'win'.
+	curwin->w_llist_ref = qi;
+	qi->qf_refcount++;
     }
 
     if (oldwin != curwin)
diff --git a/src/testdir/test_quickfix.vim b/src/testdir/test_quickfix.vim
index aa22508255..d31bd5233f 100644
--- a/src/testdir/test_quickfix.vim
+++ b/src/testdir/test_quickfix.vim
@@ -1,4 +1,4 @@
-" Test for the quickfix commands.
+" Test for the quickfix feature.
 
 if !has('quickfix')
   finish
@@ -1419,7 +1419,7 @@ func XquickfixSetListWithAct(cchar)
           \    {'filename': 'fnameD', 'text': 'D'},
           \    {'filename': 'fnameE', 'text': 'E'}]
 
-  " {action} is unspecified.  Same as specifing ' '.
+  " {action} is unspecified.  Same as specifying ' '.
   new | only
   silent! Xnewer 99
   call g:Xsetlist(list1)
@@ -2348,7 +2348,7 @@ func Test_cwindow_jump()
   " Open a new window and create a location list
   " Open the location list window and close the other window
   " Jump to an entry.
-  " Should create a new window and jump to the entry. The scrtach buffer
+  " Should create a new window and jump to the entry. The scratch buffer
   " should not be used.
   enew | only
   set buftype=nofile
@@ -3831,7 +3831,7 @@ func Test_splitview()
   new | only
 
   " When split opening files from a helpgrep location list window, a new help
-  " window should be opend with a copy of the location list.
+  " window should be opened with a copy of the location list.
   lhelpgrep window
   let locid = getloclist(0, {'id' : 0}).id
   lwindow
@@ -3933,8 +3933,30 @@ func Xqfbuf_test(cchar)
     call assert_match(qfbnum . '  h-  "\[Location List]"', execute('ls'))
     call assert_true(bufloaded(qfbnum))
 
+    " When the location list is cleared for the window, the buffer should be
+    " removed
+    call setloclist(0, [], 'f')
+    call assert_false(bufexists(qfbnum))
+
+    " When the location list is freed with the location list window open, the
+    " location list buffer should not be lost. It should be reused when the
+    " location list is again populated.
+    lexpr "F1:10:Line10"
+    lopen
+    let wid = win_getid()
+    let qfbnum = bufnr('')
+    wincmd p
+    call setloclist(0, [], 'f')
+    lexpr "F1:10:Line10"
+    lopen
+    call assert_equal(wid, win_getid())
+    call assert_equal(qfbnum, bufnr(''))
+    lclose
+
+    " When the window with the location list is closed, the buffer should be
+    " removed
     new | only
-    call assert_false(bufloaded(qfbnum))
+    call assert_false(bufexists(qfbnum))
   endif
 endfunc
 
@@ -3942,3 +3964,29 @@ func Test_qfbuf()
   call Xqfbuf_test('c')
   call Xqfbuf_test('l')
 endfunc
+
+" If there is an autocmd to use only one window, then opening the location
+" list window used to crash Vim.
+func Test_winonly_autocmd()
+  call s:create_test_file('Xtest1')
+  " Autocmd to show only one Vim window at a time
+  autocmd WinEnter * only
+  new
+  " Load the location list
+  lexpr "Xtest1:5:Line5\nXtest1:10:Line10\nXtest1:15:Line15"
+  let loclistid = getloclist(0, {'id' : 0}).id
+  " Open the location list window. Only this window will be shown and the file
+  " window is closed.
+  lopen
+  call assert_equal(loclistid, getloclist(0, {'id' : 0}).id)
+  " Jump to an entry in the location list and make sure that the cursor is
+  " positioned correctly.
+  ll 3
+  call assert_equal(loclistid, getloclist(0, {'id' : 0}).id)
+  call assert_equal('Xtest1', bufname(''))
+  call assert_equal(15, line('.'))
+  " Cleanup
+  autocmd! WinEnter
+  new | only
+  call delete('Xtest1')
+endfunc
diff --git a/src/version.c b/src/version.c
index 7de8a0ab7e..3305e22d2e 100644
--- a/src/version.c
+++ b/src/version.c
@@ -784,6 +784,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    892,
+/**/
     891,
 /**/
     890,
diff --git a/src/window.c b/src/window.c
index f78fccafe6..f6e611535a 100644
--- a/src/window.c
+++ b/src/window.c
@@ -7193,11 +7193,10 @@ win_id2tabwin(typval_T *argvars, list_T *list)
 }
 
     win_T *
-win_id2wp(typval_T *argvars)
+win_id2wp(int id)
 {
     win_T	*wp;
     tabpage_T   *tp;
-    int		id = tv_get_number(&argvars[0]);
 
     FOR_ALL_TAB_WINDOWS(tp, wp)
 	if (wp->w_id == id)