neovim

commit e11f3655fb3bbedc29e496db26a25e6c83d25baf
parent c1b591dc8f99577c419192c7b34e3999758ed69f
Author: erw7 <erw7.github@gmail.com>
Date:   Sun,  3 Jul 2022 01:14:08 +0900

fix(jobs): deadlock in channel.c:exit_event #19082

In the rare case that exit_event is called from process_close_handles,
it stalls waiting for the process to exit (the routine is currently
underway to do just that). This causes `job_spec.lua` to sometimes
stall.

REJECTED IDEAS:
==============================================================
1. Currently `exit_event` is placed on `main_loop.fast_events`. Would the problem
   be solved by using `main_loop.events` instead?
    - A: Maybe, but it will cause other problems, such as queuing exit_event()
      during "Press Enter..." prompt which may result in the event not being
      processed, leading to another stall.
2. Can we avoid the timer?
    - A: Using a timer is just the easiest way to queue a delayed event without
      causing an infinite loop in the queue currently being processed.
3. Can we avoid the new `exit_need_delay` global...
    1. by using `process_is_tearing_down` instead?
        - A: Can't use `process_is_tearing_down` because its semantics are different.
    2. by checking a similar condition as `process_teardown`? https://github.com/neovim/neovim/blob/f50135a32e11c535e1dc3a8e9460c5b4e640ee86/src/nvim/event/process.c#L141-L142
       ```
       if (!process_is_tearing_down || (kl_empty(main_loop.children) && multiqueue_empty(main_loop.events))) {
         uv_timer_start(&main_loop.exit_delay_timer, exit_delay_cb, 0, 0);
         return;
       }
       ```
        - A: Tried but it did not work (other stalls occurred). Maybe
          exit_event() is called from a source other than
          process_close_handles() and is delayed, the delayed exit_event() will
          be executed before main_loop.events is processed, resulting in an
          infinite loop.

Diffstat:
M
src/nvim/event/loop.c
 | 
2
++
M
src/nvim/event/loop.h
 | 
2
++
M
src/nvim/event/process.c
 | 
2
++
M
src/nvim/globals.h
 | 
2
++
M
src/nvim/msgpack_rpc/channel.c
 | 
11
+++++++++++

5 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/src/nvim/event/loop.c b/src/nvim/event/loop.c
@@ -27,6 +27,7 @@ void loop_init(Loop *loop, void *data)
   uv_signal_init(&loop->uv, &loop->children_watcher);
   uv_timer_init(&loop->uv, &loop->children_kill_timer);
   uv_timer_init(&loop->uv, &loop->poll_timer);
+  uv_timer_init(&loop->uv, &loop->exit_delay_timer);
   loop->poll_timer.data = xmalloc(sizeof(bool));  // "timeout expired" flag
 }
 
@@ -136,6 +137,7 @@ bool loop_close(Loop *loop, bool wait)
   uv_close((uv_handle_t *)&loop->children_watcher, NULL);
   uv_close((uv_handle_t *)&loop->children_kill_timer, NULL);
   uv_close((uv_handle_t *)&loop->poll_timer, timer_close_cb);
+  uv_close((uv_handle_t *)&loop->exit_delay_timer, NULL);
   uv_close((uv_handle_t *)&loop->async, NULL);
   uint64_t start = wait ? os_hrtime() : 0;
   bool didstop = false;
diff --git a/src/nvim/event/loop.h b/src/nvim/event/loop.h
@@ -36,6 +36,8 @@ typedef struct loop {
   // generic timer, used by loop_poll_events()
   uv_timer_t poll_timer;
 
+  uv_timer_t exit_delay_timer;
+
   uv_async_t async;
   uv_mutex_t mutex;
   int recursive;
diff --git a/src/nvim/event/process.c b/src/nvim/event/process.c
@@ -386,11 +386,13 @@ static void process_close_handles(void **argv)
 {
   Process *proc = argv[0];
 
+  exit_need_delay++;
   flush_stream(proc, &proc->out);
   flush_stream(proc, &proc->err);
 
   process_close_streams(proc);
   process_close(proc);
+  exit_need_delay--;
 }
 
 static void on_process_exit(Process *proc)
diff --git a/src/nvim/globals.h b/src/nvim/globals.h
@@ -1075,4 +1075,6 @@ typedef enum {
 // Only filled for Win32.
 EXTERN char windowsVersion[20] INIT(= { 0 });
 
+EXTERN int exit_need_delay INIT(= 0);
+
 #endif  // NVIM_GLOBALS_H
diff --git a/src/nvim/msgpack_rpc/channel.c b/src/nvim/msgpack_rpc/channel.c
@@ -532,8 +532,19 @@ void rpc_close(Channel *channel)
   }
 }
 
+static void exit_delay_cb(uv_timer_t *handle)
+{
+  uv_timer_stop(&main_loop.exit_delay_timer);
+  multiqueue_put(main_loop.fast_events, exit_event, 0);
+}
+
 static void exit_event(void **argv)
 {
+  if (exit_need_delay) {
+    uv_timer_start(&main_loop.exit_delay_timer, exit_delay_cb, 0, 0);
+    return;
+  }
+
   if (!exiting) {
     os_exit(0);
   }