diff options
author | erw7 <erw7.github@gmail.com> | 2022-07-03 01:14:08 +0900 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-07-02 09:14:08 -0700 |
commit | e11f3655fb3bbedc29e496db26a25e6c83d25baf (patch) | |
tree | 633e33e41e04d797de07fb20fb4d9145daf55bb4 | |
parent | c1b591dc8f99577c419192c7b34e3999758ed69f (diff) | |
download | rneovim-e11f3655fb3bbedc29e496db26a25e6c83d25baf.tar.gz rneovim-e11f3655fb3bbedc29e496db26a25e6c83d25baf.tar.bz2 rneovim-e11f3655fb3bbedc29e496db26a25e6c83d25baf.zip |
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.
-rw-r--r-- | src/nvim/event/loop.c | 2 | ||||
-rw-r--r-- | src/nvim/event/loop.h | 2 | ||||
-rw-r--r-- | src/nvim/event/process.c | 2 | ||||
-rw-r--r-- | src/nvim/globals.h | 2 | ||||
-rw-r--r-- | 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 index f40573a0bc..1b5cc23b09 100644 --- 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 index c0c5bc527f..65980c6c05 100644 --- 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 index 1ec11f1eb6..e029f778f6 100644 --- 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 index 5c1d6867ce..b539d50784 100644 --- 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 index 388fa2584c..de01443313 100644 --- 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); } |