eval: Fix `jobwait()`

- Properly save job event deferring state for recursive calls
- Disable breakcheck while running. Breakcheck can invoke job callbacks
  in unexpected places.

4 files changed, 72 insertions, 8 deletions

diff --git a/src/nvim/eval.c b/src/nvim/eval.c
index 6c8dd3da32..a3ef24f295 100644
--- a/src/nvim/eval.c
+++ b/src/nvim/eval.c
@@ -472,7 +472,7 @@ typedef struct {
 #define JobEventFreer(x)
 KMEMPOOL_INIT(JobEventPool, JobEvent, JobEventFreer)
 static kmempool_t(JobEventPool) *job_event_pool = NULL;
-static bool defer_job_callbacks = true;
+static int disable_job_defer = 0;
 
 /*
  * Initialize the global and v: variables.
@@ -10922,9 +10922,16 @@ static void f_jobwait(typval_T *argvars, typval_T *rettv)
   list_T *args = argvars[0].vval.v_list;
   list_T *rv = list_alloc();
 
-  // must temporarily disable job event deferring so the callbacks are
-  // processed while waiting.
-  defer_job_callbacks = false;
+  ui_busy_start();
+  // disable breakchecks, which could result in job callbacks being executed
+  // at unexpected places
+  disable_breakcheck++;
+  // disable job event deferring so the callbacks are processed while waiting.
+  if (!disable_job_defer++) {
+    // process any pending job events in the deferred queue, but only do this if
+    // deferred is not disabled(at the top-level `jobwait()` call)
+    event_process();
+  }
   // For each item in the input list append an integer to the output list. -3
   // is used to represent an invalid job id, -2 is for a interrupted job and
   // -1 for jobs that were skipped or timed out.
@@ -10997,8 +11004,9 @@ static void f_jobwait(typval_T *argvars, typval_T *rettv)
     // job exits
     data->status_ptr = NULL;
   }
-  // restore defer flag
-  defer_job_callbacks = true;
+  disable_job_defer--;
+  disable_breakcheck--;
+  ui_busy_stop();
 
   rv->lv_refcount++;
   rettv->v_type = VAR_LIST;
@@ -20176,7 +20184,7 @@ static inline void push_job_event(Job *job, ufunc_T *callback,
   event_push((Event) {
     .handler = on_job_event,
     .data = event_data
-  }, defer_job_callbacks);
+  }, !disable_job_defer);
 }
 
 static void on_job_stdout(RStream *rstream, void *job, bool eof)
diff --git a/src/nvim/globals.h b/src/nvim/globals.h
index 158125c878..9eb70d37df 100644
--- a/src/nvim/globals.h
+++ b/src/nvim/globals.h
@@ -898,6 +898,14 @@ EXTERN FILE     *scriptout INIT(= NULL);    /* stream to write script to */
 /* volatile because it is used in signal handler catch_sigint(). */
 EXTERN volatile int got_int INIT(= FALSE);    /* set to TRUE when interrupt
                                                  signal occurred */
+EXTERN int disable_breakcheck INIT(= 0);    // > 0 if breakchecks should be 
+                                            // ignored. FIXME(tarruda): Hacky
+                                            // way to run functions that would
+                                            // result in *_breakcheck calls
+                                            // while events that would normally
+                                            // be deferred are being processed
+                                            // immediately. Ref:
+                                            // neovim/neovim#2371
 EXTERN int bangredo INIT(= FALSE);          /* set to TRUE with ! command */
 EXTERN int searchcmdlen;                    /* length of previous search cmd */
 EXTERN int reg_do_extmatch INIT(= 0);       /* Used when compiling regexp:
diff --git a/src/nvim/os/input.c b/src/nvim/os/input.c
index a409a9ed13..8002d528ed 100644
--- a/src/nvim/os/input.c
+++ b/src/nvim/os/input.c
@@ -128,7 +128,9 @@ bool os_char_avail(void)
 // Check for CTRL-C typed by reading all available characters.
 void os_breakcheck(void)
 {
-  event_poll(0);
+  if (!disable_breakcheck && !got_int) {
+    event_poll(0);
+  }
 }
 
 /// Test whether a file descriptor refers to a terminal.
diff --git a/test/functional/job/job_spec.lua b/test/functional/job/job_spec.lua
index c517ae4c1b..6bcae62ebf 100644
--- a/test/functional/job/job_spec.lua
+++ b/test/functional/job/job_spec.lua
@@ -249,6 +249,52 @@ describe('jobs', function()
       eq({'notification', 'wait', {{-2}}}, next_msg())
     end)
 
+    it('can be called recursively', function()
+      source([[
+      let g:opts = {}
+      let g:counter = 0
+      function g:opts.on_stdout(id, msg)
+        if self.state == 0
+          if self.counter < 10
+            call Run()
+          endif
+          let self.state = 1
+          call jobsend(a:id, "line1\n")
+        elseif self.state == 1
+          let self.state = 2
+          call jobsend(a:id, "line2\n")
+        elseif self.state == 2
+          let self.state = 3
+          call jobsend(a:id, "line3\n")
+        else
+          call rpcnotify(g:channel, 'w', printf('job %d closed', self.counter))
+          call jobclose(a:id, 'stdin')
+        endif
+      endfunction
+      function g:opts.on_exit()
+        call rpcnotify(g:channel, 'w', printf('job %d exited', self.counter))
+      endfunction
+      function Run()
+        let g:counter += 1
+        let j = copy(g:opts)
+        let j.state = 0
+        let j.counter = g:counter
+        call jobwait([
+        \   jobstart([&sh, '-c', 'echo ready; cat -'], j),
+        \ ])
+      endfunction
+      ]])
+      execute('call Run()')
+      local r
+      for i = 10, 1, -1 do
+        r = next_msg()
+        eq('job '..i..' closed', r[3][1])
+        r = next_msg()
+        eq('job '..i..' exited', r[3][1])
+      end
+      eq(10, nvim('eval', 'g:counter'))
+    end)
+
     describe('with timeout argument', function()
       it('will return -1 if the wait timed out', function()
         source([[