vim-patch:8.1.0892: failure when closing a window when location list is in use

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 vim/vim#3928)
eeb1b9c7ed

src/nvim/eval.c

@@ -6436,7 +6436,7 @@ win_T *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);

src/nvim/eval/funcs.c

@@ -2197,12 +2197,13 @@ static void f_execute(typval_T *argvars, typval_T *rettv, FunPtr fptr)
 // "win_execute(win_id, command)" function
 static void f_win_execute(typval_T *argvars, typval_T *rettv, FunPtr fptr)
 {
-  tabpage_T *tp;
-  win_T *wp = win_id2wp_tp(argvars, &tp);
   // Return an empty string if something fails.
   rettv->v_type = VAR_STRING;
   rettv->vval.v_string = NULL;
 
+  int id = tv_get_number(argvars);
+  tabpage_T *tp;
+  win_T *wp = win_id2wp_tp(id, &tp);
   if (wp != NULL && tp != NULL) {
     WIN_EXECUTE(wp, tp, execute_common(argvars, rettv, fptr, 1));
   }
@@ -4130,7 +4131,7 @@ static void f_getwininfo(typval_T *argvars, typval_T *rettv, FunPtr fptr)
   tv_list_alloc_ret(rettv, kListLenMayKnow);
 
   if (argvars[0].v_type != VAR_UNKNOWN) {
-    wparg = win_id2wp(argvars);
+    wparg = win_id2wp(tv_get_number(&argvars[0]));
     if (wparg == NULL) {
       return;
     }
@@ -5917,10 +5918,10 @@ static void f_line(typval_T *argvars, typval_T *rettv, FunPtr fptr)
   int fnum;
 
   if (argvars[1].v_type != VAR_UNKNOWN) {
-    tabpage_T *tp;
-
     // use window specified in the second argument
-    win_T *wp = win_id2wp_tp(&argvars[1], &tp);
+    int id = (int)tv_get_number(&argvars[1]);
+    tabpage_T *tp;
+    win_T *wp = win_id2wp_tp(id, &tp);
     if (wp != NULL && tp != NULL) {
       switchwin_T switchwin;
       if (switch_win_noblock(&switchwin, wp, tp, true) == OK) {

src/nvim/quickfix.c

@@ -1739,22 +1739,23 @@ static void ll_free_all(qf_info_T **pqi)
   }
   *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
+    // If the quickfix window buffer is loaded, then wipe it
-    // to be processed later.
+    wipe_qf_buffer(qi);
-    if (quickfix_busy > 0) {
-      locstack_queue_delreq(qi);
-    } else {
-      // If the quickfix window buffer is loaded, then wipe it
-      wipe_qf_buffer(qi);
 
-      for (i = 0; i < qi->qf_listcount; i++) {
+    for (i = 0; i < qi->qf_listcount; i++) {
-        qf_free(qf_get_list(qi, i));
+      qf_free(qf_get_list(qi, i));
-      }
-      xfree(qi);
     }
+    xfree(qi);
   }
 }
 
@@ -2750,7 +2751,7 @@ static int qf_jump_to_usable_window(int qf_fnum, bool newwin, int *opened_window
 }
 
 /// Edit the selected file or help file.
-static int qf_jump_edit_buffer(qf_info_T *qi, qfline_T *qf_ptr, int forceit, win_T *oldwin,
+static int qf_jump_edit_buffer(qf_info_T *qi, qfline_T *qf_ptr, int forceit, int prev_winid,
                                int *opened_window)
 {
   qf_list_T *qfl = qf_get_curlist(qi);
@@ -2769,7 +2770,7 @@ static int qf_jump_edit_buffer(qf_info_T *qi, qfline_T *qf_ptr, int forceit, win
     } else {
       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->handle ? curwin : NULL);
     }
   } else {
     retval = buflist_getfile(qf_ptr->qf_fnum, (linenr_T)1,
@@ -2777,10 +2778,13 @@ static int qf_jump_edit_buffer(qf_info_T *qi, qfline_T *qf_ptr, int forceit, win
   }
   // 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"));
+    win_T *wp = win_id2wp(prev_winid);
-    *opened_window = false;
+    if (wp == NULL && curwin->w_llist != qi) {
-    return NOTDONE;
+      emsg(_("E924: Current window was closed"));
+      *opened_window = false;
+      return NOTDONE;
+    }
   }
 
   if (qfl_type == QFLT_QUICKFIX && !qflist_valid(NULL, save_qfid)) {
@@ -2935,7 +2939,7 @@ static int qf_jump_open_window(qf_info_T *qi, qfline_T *qf_ptr, bool newwin, int
 /// NOTDONE if the quickfix/location list is freed by an autocmd when opening
 /// the file.
 static int qf_jump_to_buffer(qf_info_T *qi, int qf_index, qfline_T *qf_ptr, int forceit,
-                             win_T *oldwin, int *opened_window, int openfold, int print_message)
+                             int prev_winid, int *opened_window, int openfold, int print_message)
 {
   buf_T *old_curbuf;
   linenr_T old_lnum;
@@ -2947,7 +2951,7 @@ static int qf_jump_to_buffer(qf_info_T *qi, int qf_index, qfline_T *qf_ptr, int
   old_lnum = curwin->w_cursor.lnum;
 
   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;
@@ -2996,8 +3000,8 @@ static void qf_jump_newwin(qf_info_T *qi, int dir, int errornr, int forceit, boo
   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;
   const bool old_KeyTyped = KeyTyped;           // getting file may reset it
   int retval = OK;
@@ -3011,6 +3015,8 @@ static void qf_jump_newwin(qf_info_T *qi, int dir, int errornr, int forceit, boo
     return;
   }
 
+  incr_quickfix_busy();
+
   qfl = qf_get_curlist(qi);
 
   qf_ptr = qfl->qf_ptr;
@@ -3033,6 +3039,8 @@ static void qf_jump_newwin(qf_info_T *qi, int dir, int errornr, int forceit, boo
     print_message = false;
   }
 
+  prev_winid = curwin->handle;
+
   retval = qf_jump_open_window(qi, qf_ptr, newwin, &opened_window);
   if (retval == FAIL) {
     goto failed;
@@ -3041,7 +3049,7 @@ static void qf_jump_newwin(qf_info_T *qi, int dir, int errornr, int forceit, boo
     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) {
     // Quickfix/location list is freed by an autocmd
@@ -3066,12 +3074,13 @@ theend:
     qfl->qf_ptr = qf_ptr;
     qfl->qf_index = qf_index;
   }
-  if (p_swb != old_swb && p_swb == empty_option && opened_window) {
+  if (p_swb != old_swb && p_swb == empty_option) {
     // Restore old 'switchbuf' value, but not when an autocommand or
     // modeline has changed the value.
     p_swb = old_swb;
     swb_flags = old_swb_flags;
   }
+  decr_quickfix_busy();
 }
 
 
@@ -3679,9 +3688,9 @@ static int 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'.
+    // location list stack from the window 'win'.
-    curwin->w_llist_ref = win->w_llist;
+    curwin->w_llist_ref = qi;
-    win->w_llist->qf_refcount++;
+    qi->qf_refcount++;
   }
 
   if (oldwin != curwin) {

src/nvim/testdir/test_quickfix.vim

@@ -1,4 +1,4 @@
-" Test for the quickfix commands.
+" Test for the quickfix feature.
 
 source check.vim
 CheckFeature quickfix
@@ -1655,7 +1655,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)
@@ -2706,7 +2706,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
@@ -4360,7 +4360,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
@@ -4519,6 +4519,32 @@ func Test_qfbuf()
   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
+
 " Test to make sure that an empty quickfix buffer is not reused for loading
 " a normal buffer.
 func Test_empty_qfbuf()

src/nvim/window.c

@@ -7195,16 +7195,14 @@ void win_id2tabwin(typval_T *const argvars, typval_T *const rettv)
   tv_list_append_number(list, winnr);
 }
 
-win_T *win_id2wp(typval_T *argvars)
+win_T *win_id2wp(int id)
 {
-  return win_id2wp_tp(argvars, NULL);
+  return win_id2wp_tp(id, NULL);
 }
 
 // Return the window and tab pointer of window "id".
-win_T *win_id2wp_tp(typval_T *argvars, tabpage_T **tpp)
+win_T *win_id2wp_tp(int id, tabpage_T **tpp)
 {
-  int id = tv_get_number(&argvars[0]);
-
   FOR_ALL_TAB_WINDOWS(tp, wp) {
     if (wp->handle == id) {
       if (tpp != NULL) {