mirrors/neovim
1
0
Fork 0
mirror of https://github.com/neovim/neovim.git synced 2025-09-27 21:48:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

job-control: mitigate process-kill race

Browse Source 
children_kill_cb() is racey. One obvious problem is that
process_close_handles() is *queued* by on_process_exit(), so when
children_kill_cb() is invoked, the dead process might still be in the
`loop->children` list.  If the OS already reclaimed the dead PID, Nvim
may try to SIGKILL it.

Avoid that by checking `proc->status`.

Vim doesn't have this problem because it doesn't attempt to kill
processes that ignored SIGTERM after a timeout.

closes #8269
This commit is contained in:
Justin M. Keyes
2018-04-14 12:03:34 +02:00
parent c8f409c2f2
commit 8fa0b8051d
3 changed files with 17 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/nvim/channel.c
View File

@@ -658,9 +658,9 @@ static void channel_process_exit_cb(Process *proc, int status, void *data)
terminal_close(chan->term, msg);
}
// if status is -1 the process did not really exit,
// we just closed the handle onto a detached process
if (status >= 0) {
// If process did not exit, we only closed the handle of a detached process.
bool exited = (status >= 0);
if (exited) {
process_channel_event(chan, &chan->on_exit, "exit", NULL, 0, status);
}

20
src/nvim/event/process.c
View File

@@ -188,8 +188,7 @@ int process_wait(Process *proc, int ms, MultiQueue *events)
}
if (proc->refcount == 1) {
// Job exited, collect status and manually invoke close_cb to free the job
// resources
// Job exited, free its resources.
decref(proc);
if (events) {
// the decref call created an exit event, process it now
@@ -205,7 +204,8 @@ int process_wait(Process *proc, int ms, MultiQueue *events)
/// Ask a process to terminate and eventually kill if it doesn't respond
void process_stop(Process *proc) FUNC_ATTR_NONNULL_ALL
{
if (proc->stopped_time) {
bool exited = (proc->status >= 0);
if (exited || proc->stopped_time) {
return;
}
@@ -228,14 +228,14 @@ void process_stop(Process *proc) FUNC_ATTR_NONNULL_ALL
}
Loop *loop = proc->loop;
// Start a timer to periodically check if a signal should be send to the job.
// Start a timer to periodically check if a signal should be sent to the job.
ILOG("starting job kill timer");
uv_timer_start(&loop->children_kill_timer, children_kill_cb,
KILL_TIMEOUT_MS, KILL_TIMEOUT_MS);
}
/// Iterates the process list sending SIGTERM to stopped processes and SIGKILL
/// to those that didn't die from SIGTERM after a while(exit_timeout is 0).
/// Sends SIGKILL (or SIGTERM for PTY jobs) to processes that didn't terminate
/// after process_stop() requested them.
static void children_kill_cb(uv_timer_t *handle)
{
Loop *loop = handle->loop->data;
@@ -243,11 +243,11 @@ static void children_kill_cb(uv_timer_t *handle)
kl_iter(WatcherPtr, loop->children, current) {
Process *proc = (*current)->data;
if (!proc->stopped_time) {
bool exited = (proc->status >= 0);
if (exited || !proc->stopped_time) {
continue;
}
uint64_t elapsed = (now - proc->stopped_time) / 1000000 + 1;
if (elapsed >= KILL_TIMEOUT_MS) {
int sig = proc->type == kProcessTypePty && elapsed < KILL_TIMEOUT_MS * 2
? SIGTERM
@@ -380,8 +380,10 @@ static void process_close_handles(void **argv)
static void on_process_exit(Process *proc)
{
Loop *loop = proc->loop;
ILOG("exited: pid=%d status=%d stoptime=%" PRId64, proc->pid,
proc->status, proc->stopped_time);
if (proc->stopped_time) {
DLOG("stopping process kill timer");
ILOG("stopping process kill timer");
uv_timer_stop(&loop->children_kill_timer);
}