mirror of
https://github.com/neovim/neovim.git
synced 2025-02-25 18:55:25 -06:00
jobs: child proc must have a separate process-group
UV_PROCESS_DETACHED compels libuv:uv__process_child_init() to call setsid() in the child just after fork(). That ensures the process and its descendants are grouped in a separate session (and process group). The following jobstart() call correctly groups `sh` and `sleep` in a new session (and process-group), where `sh` is the "session leader" (and process-group leader): :call jobstart(['sh','-c','sleep 60']) SESN PGRP PID PPID Command 30383 30383 30383 3620 │ ├─ -bash 30383 31432 31432 30383 │ │ └─ nvim -u NORC 30383 31432 31433 30383 │ │ ├─ nvim -u NORC 8105 8105 8105 31432 │ │ └─ sh -c sleep 60 8105 8105 8106 8105 │ │ └─ sleep 60 closes #6530 ref: https://stackoverflow.com/q/1046933 ref: https://unix.stackexchange.com/a/404065 Helped-by: Marco Hinz <mh.codebro+github@gmail.com> Discussion ------------------------------------------------------------------------ On my linux box before this patch, the termclose_spec.lua:'kills job trapping SIGTERM' test indirectly causes cmake/busted to wait for 60s. That's because the test spawns a `sleep 60` descendant process which hangs around even after nvim exits: nvim killed the parent PID, but not PGID (process-group), so the grandchild "reparented" to init (PID 1). Session contains processes (and process-groups) which are logically part of the same "login session". Process-group is a set of logically/informally-related processes within a session; for example, shells assign a process group to each "job". Session IDs and PGIDs both have type pid_t (like PIDs). These OS-level mechanisms are, as usual, legacy accidents whose purpose is upheld by convention and folklore. We can use session-level grouping (setsid), or we could use process-group-level grouping (setpgid). Vim uses setsid() if available, otherwise setpgid(0,0). Windows ------------------------------------------------------------------------ UV_PROCESS_DETACHED on win32 sets CREATE_NEW_PROCESS_GROUP flag. But uv_kill() does not kill the process-group: https://github.com/nodejs/node/issues/3617 Ideas: - Set UV_PROCESS_DETACHED (CREATE_NEW_PROCESS_GROUP), then call GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, pid) - Maybe won't work because MSDN says "Only processes that share the same console as the calling process receive the signal." https://docs.microsoft.com/en-us/windows/console/generateconsolectrlevent But CREATE_NEW_PROCESS_GROUP creates a new console ... ref https://stackoverflow.com/q/1453520 - Group processes within a "job". libuv does that *globally* for non-detached processes: uv__init_global_job_handle. - Iterate through CreateToolhelp32Snapshot(). - https://stackoverflow.com/q/1173342 - Vim does this, see terminate_all()
This commit is contained in:
parent
de47515477
commit
8d90171f8b
@ -26,15 +26,18 @@ int libuv_process_spawn(LibuvProcess *uvproc)
|
|||||||
uvproc->uvopts.file = proc->argv[0];
|
uvproc->uvopts.file = proc->argv[0];
|
||||||
uvproc->uvopts.args = proc->argv;
|
uvproc->uvopts.args = proc->argv;
|
||||||
uvproc->uvopts.flags = UV_PROCESS_WINDOWS_HIDE;
|
uvproc->uvopts.flags = UV_PROCESS_WINDOWS_HIDE;
|
||||||
if (proc->detach) {
|
|
||||||
uvproc->uvopts.flags |= UV_PROCESS_DETACHED;
|
|
||||||
}
|
|
||||||
#ifdef WIN32
|
#ifdef WIN32
|
||||||
// libuv collapses the argv to a CommandLineToArgvW()-style string. cmd.exe
|
// libuv collapses the argv to a CommandLineToArgvW()-style string. cmd.exe
|
||||||
// expects a different syntax (must be prepared by the caller before now).
|
// expects a different syntax (must be prepared by the caller before now).
|
||||||
if (os_shell_is_cmdexe(proc->argv[0])) {
|
if (os_shell_is_cmdexe(proc->argv[0])) {
|
||||||
uvproc->uvopts.flags |= UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS;
|
uvproc->uvopts.flags |= UV_PROCESS_WINDOWS_VERBATIM_ARGUMENTS;
|
||||||
}
|
}
|
||||||
|
if (proc->detach) {
|
||||||
|
uvproc->uvopts.flags |= UV_PROCESS_DETACHED;
|
||||||
|
}
|
||||||
|
#else
|
||||||
|
// Always setsid() on unix-likes. #8107
|
||||||
|
uvproc->uvopts.flags |= UV_PROCESS_DETACHED;
|
||||||
#endif
|
#endif
|
||||||
uvproc->uvopts.exit_cb = exit_cb;
|
uvproc->uvopts.exit_cb = exit_cb;
|
||||||
uvproc->uvopts.cwd = proc->cwd;
|
uvproc->uvopts.cwd = proc->cwd;
|
||||||
|
@ -201,6 +201,23 @@ int process_wait(Process *proc, int ms, MultiQueue *events)
|
|||||||
return proc->status;
|
return proc->status;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Kills a process and its descendants.
|
||||||
|
static void os_proc_tree_kill(int pid, int sig) {
|
||||||
|
assert(sig == SIGTERM || sig == SIGKILL);
|
||||||
|
int pgid = getpgid(pid);
|
||||||
|
if (pgid > 0) { // Ignore error. Never kill self (pid=0).
|
||||||
|
if (pgid == pid) {
|
||||||
|
ILOG("sending %s to process group: -%d",
|
||||||
|
sig == SIGTERM ? "SIGTERM" : "SIGKILL",
|
||||||
|
pgid);
|
||||||
|
uv_kill(-pgid, sig);
|
||||||
|
} else {
|
||||||
|
// Should never happen, because process_spawn() did setsid() in the child.
|
||||||
|
ELOG("pgid %d != pid %d", pgid, pid);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Ask a process to terminate and eventually kill if it doesn't respond
|
/// Ask a process to terminate and eventually kill if it doesn't respond
|
||||||
void process_stop(Process *proc) FUNC_ATTR_NONNULL_ALL
|
void process_stop(Process *proc) FUNC_ATTR_NONNULL_ALL
|
||||||
{
|
{
|
||||||
@ -215,8 +232,7 @@ void process_stop(Process *proc) FUNC_ATTR_NONNULL_ALL
|
|||||||
// stdout/stderr, they will be closed when it exits(possibly due to being
|
// stdout/stderr, they will be closed when it exits(possibly due to being
|
||||||
// terminated after a timeout)
|
// terminated after a timeout)
|
||||||
stream_may_close(&proc->in);
|
stream_may_close(&proc->in);
|
||||||
ILOG("Sending SIGTERM to pid %d", proc->pid);
|
os_proc_tree_kill(proc->pid, SIGTERM);
|
||||||
uv_kill(proc->pid, SIGTERM);
|
|
||||||
break;
|
break;
|
||||||
case kProcessTypePty:
|
case kProcessTypePty:
|
||||||
// close all streams for pty processes to send SIGHUP to the process
|
// close all streams for pty processes to send SIGHUP to the process
|
||||||
@ -231,7 +247,7 @@ void process_stop(Process *proc) FUNC_ATTR_NONNULL_ALL
|
|||||||
if (!loop->children_stop_requests++) {
|
if (!loop->children_stop_requests++) {
|
||||||
// When there's at least one stop request pending, start a timer that
|
// When there's at least one stop request pending, start a timer that
|
||||||
// will periodically check if a signal should be send to the job.
|
// will periodically check if a signal should be send to the job.
|
||||||
ILOG("Starting job kill timer");
|
ILOG("starting job kill timer");
|
||||||
uv_timer_start(&loop->children_kill_timer, children_kill_cb,
|
uv_timer_start(&loop->children_kill_timer, children_kill_cb,
|
||||||
KILL_TIMEOUT_MS, KILL_TIMEOUT_MS);
|
KILL_TIMEOUT_MS, KILL_TIMEOUT_MS);
|
||||||
}
|
}
|
||||||
@ -253,11 +269,9 @@ static void children_kill_cb(uv_timer_t *handle)
|
|||||||
|
|
||||||
if (elapsed >= KILL_TIMEOUT_MS) {
|
if (elapsed >= KILL_TIMEOUT_MS) {
|
||||||
int sig = proc->type == kProcessTypePty && elapsed < KILL_TIMEOUT_MS * 2
|
int sig = proc->type == kProcessTypePty && elapsed < KILL_TIMEOUT_MS * 2
|
||||||
? SIGTERM
|
? SIGTERM
|
||||||
: SIGKILL;
|
: SIGKILL;
|
||||||
ILOG("Sending %s to pid %d", sig == SIGTERM ? "SIGTERM" : "SIGKILL",
|
os_proc_tree_kill(proc->pid, sig);
|
||||||
proc->pid);
|
|
||||||
uv_kill(proc->pid, sig);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -145,8 +145,12 @@ void pty_process_teardown(Loop *loop)
|
|||||||
uv_signal_stop(&loop->children_watcher);
|
uv_signal_stop(&loop->children_watcher);
|
||||||
}
|
}
|
||||||
|
|
||||||
static void init_child(PtyProcess *ptyproc) FUNC_ATTR_NONNULL_ALL
|
static void init_child(PtyProcess *ptyproc)
|
||||||
|
FUNC_ATTR_NONNULL_ALL
|
||||||
{
|
{
|
||||||
|
// New session/process-group. #6530
|
||||||
|
setsid();
|
||||||
|
|
||||||
unsetenv("COLUMNS");
|
unsetenv("COLUMNS");
|
||||||
unsetenv("LINES");
|
unsetenv("LINES");
|
||||||
unsetenv("TERMCAP");
|
unsetenv("TERMCAP");
|
||||||
|
Loading…
Reference in New Issue
Block a user