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:
Justin M. Keyes 2018-02-16 19:00:02 +01:00
parent de47515477
commit 8d90171f8b
3 changed files with 33 additions and 12 deletions

View File

@ -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;

View File

@ -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);
} }
} }
} }

View File

@ -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");