fix(jobwait): always drain process event queues #15402

Problem: jobwait() returns early if the job was stopped, but the job might have pending callbacks on its event queue which are required to complete its teardown. State such as term->closed might not be updated yet (by the pending callbacks), so codepaths such as :bdelete think the job is still running. Solution: Always flush the job's event queue before returning from jobwait(). ref #15349
neovim · Aug 31, 2021 · 3c081d0 · 3c081d0
1 parent 55defa1
commit 3c081d0
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 14 deletions.
diff --git a/runtime/doc/nvim_terminal_emulator.txt b/runtime/doc/nvim_terminal_emulator.txt
@@ -13,7 +13,7 @@ from the connected program.
 Terminal buffers behave like normal buffers, except:
 - With 'modifiable', lines can be edited but not deleted.
 - 'scrollback' controls how many lines are kept.
-- Output is followed if the cursor is on the last line.
+- Output is followed ("tailed") if cursor is on the last line.
 - 'modified' is the default. You can set 'nomodified' to avoid a warning when
   closing the terminal buffer.
 - 'bufhidden' defaults to "hide".

diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c
@@ -5404,14 +5404,19 @@ static void f_jobwait(typval_T *argvars, typval_T *rettv, FunPtr fptr)
   TV_LIST_ITER_CONST(args, arg, {
     Channel *chan = NULL;
     if (TV_LIST_ITEM_TV(arg)->v_type != VAR_NUMBER
-        || !(chan = find_job(TV_LIST_ITEM_TV(arg)->vval.v_number, false))) {
+        || !(chan = find_channel(TV_LIST_ITEM_TV(arg)->vval.v_number))
+        || chan->streamtype != kChannelStreamProc) {
+      jobs[i] = NULL;  // Invalid job.
+    } else if (process_is_stopped(&chan->stream.proc)) {
+      // Job is stopped but not fully destroyed.
+      // Ensure all callbacks on its event queue are executed. #15402
+      process_wait(&chan->stream.proc, -1, NULL);
       jobs[i] = NULL;  // Invalid job.
     } else {
       jobs[i] = chan;
       channel_incref(chan);
       if (chan->stream.proc.status < 0) {
-        // Process any pending events on the job's queue before temporarily
-        // replacing it.
+        // Flush any events in the job's queue before temporarily replacing it.
         multiqueue_process_events(chan->events);
         multiqueue_replace_parent(chan->events, waiting_jobs);
       }

diff --git a/test/functional/terminal/buffer_spec.lua b/test/functional/terminal/buffer_spec.lua
@@ -7,12 +7,7 @@ local eq, neq = helpers.eq, helpers.neq
 local write_file = helpers.write_file
 local command= helpers.command
 local exc_exec = helpers.exc_exec
-local retry = helpers.retry
-local funcs = helpers.funcs
-local pesc = helpers.pesc
 local matches = helpers.matches
-local nvim_dir = helpers.nvim_dir
-local iswin = helpers.iswin
 
 describe(':terminal buffer', function()
   local screen
@@ -261,16 +256,25 @@ describe(':terminal buffer', function()
     command('bdelete!')
   end)
 
-  it("requires bang (!) to close a running job", function()
-    local cwd = funcs.fnamemodify('.', ':p:~'):gsub([[[\/]*$]], '')
-    local ext_pat = iswin() and '%.EXE' or ''
+  it('requires bang (!) to close a running job #15402', function()
     eq('Vim(wqall):E948: Job still running', exc_exec('wqall'))
-    matches('^Vim%(bdelete%):E89: term://'..pesc(cwd)..'//%d+:'..nvim_dir..'/tty%-test'..ext_pat..' will be killed %(add %! to override%)$', exc_exec('bdelete'))
+    for _, cmd in ipairs({ 'bdelete', '%bdelete', 'bwipeout', 'bunload' }) do
+      matches('^Vim%('..cmd:gsub('%%', '')..'%):E89: term://.*tty%-test.* will be killed %(add %! to override%)$',
+        exc_exec(cmd))
+    end
     command('call jobstop(&channel)')
-    retry(nil, nil, function() assert(0 >= eval('jobwait([&channel], 1000)[0]')) end)
+    assert(0 >= eval('jobwait([&channel], 1000)[0]'))
     command('bdelete')
   end)
 
+  it('stops running jobs with :quit', function()
+    -- Open in a new window to avoid terminating the nvim instance
+    command('split')
+    command('terminal')
+    command('set nohidden')
+    command('quit')
+  end)
+
   it('does not segfault when pasting empty buffer #13955', function()
     feed_command('terminal')
     feed('<c-\\><c-n>')