1 files changed, 73 insertions, 88 deletions

diff --git a/src/nvim/event/process.c b/src/nvim/event/process.c
index cad49e2007..60650344ce 100644
--- a/src/nvim/event/process.c
+++ b/src/nvim/event/process.c
@@ -12,6 +12,7 @@
 #include "nvim/event/wstream.h"
 #include "nvim/event/process.h"
 #include "nvim/event/libuv_process.h"
+#include "nvim/os/process.h"
 #include "nvim/os/pty_process.h"
 #include "nvim/globals.h"
 #include "nvim/macros.h"
@@ -21,32 +22,32 @@
 # include "event/process.c.generated.h"
 #endif
 
-// Time (ns) for a process to exit cleanly before we send TERM/KILL.
-#define TERM_TIMEOUT 1000000000
-#define KILL_TIMEOUT (TERM_TIMEOUT * 2)
-
-#define CLOSE_PROC_STREAM(proc, stream) \
-  do { \
-    if (proc->stream && !proc->stream->closed) { \
-      stream_close(proc->stream, NULL, NULL); \
-    } \
-  } while (0)
+// Time for a process to exit cleanly before we send KILL.
+// For pty processes SIGTERM is sent first (in case SIGHUP was not enough).
+#define KILL_TIMEOUT_MS 2000
 
 static bool process_is_tearing_down = false;
 
 /// @returns zero on success, or negative error code
-int process_spawn(Process *proc) FUNC_ATTR_NONNULL_ALL
+int process_spawn(Process *proc, bool in, bool out, bool err)
+  FUNC_ATTR_NONNULL_ALL
 {
-  if (proc->in) {
-    uv_pipe_init(&proc->loop->uv, &proc->in->uv.pipe, 0);
+  if (in) {
+    uv_pipe_init(&proc->loop->uv, &proc->in.uv.pipe, 0);
+  } else {
+    proc->in.closed = true;
   }
 
-  if (proc->out) {
-    uv_pipe_init(&proc->loop->uv, &proc->out->uv.pipe, 0);
+  if (out) {
+    uv_pipe_init(&proc->loop->uv, &proc->out.uv.pipe, 0);
+  } else {
+    proc->out.closed = true;
   }
 
-  if (proc->err) {
-    uv_pipe_init(&proc->loop->uv, &proc->err->uv.pipe, 0);
+  if (err) {
+    uv_pipe_init(&proc->loop->uv, &proc->err.uv.pipe, 0);
+  } else {
+    proc->err.closed = true;
   }
 
   int status;
@@ -62,14 +63,14 @@ int process_spawn(Process *proc) FUNC_ATTR_NONNULL_ALL
   }
 
   if (status) {
-    if (proc->in) {
-      uv_close((uv_handle_t *)&proc->in->uv.pipe, NULL);
+    if (in) {
+      uv_close((uv_handle_t *)&proc->in.uv.pipe, NULL);
     }
-    if (proc->out) {
-      uv_close((uv_handle_t *)&proc->out->uv.pipe, NULL);
+    if (out) {
+      uv_close((uv_handle_t *)&proc->out.uv.pipe, NULL);
     }
-    if (proc->err) {
-      uv_close((uv_handle_t *)&proc->err->uv.pipe, NULL);
+    if (err) {
+      uv_close((uv_handle_t *)&proc->err.uv.pipe, NULL);
     }
 
     if (proc->type == kProcessTypeUv) {
@@ -82,30 +83,27 @@ int process_spawn(Process *proc) FUNC_ATTR_NONNULL_ALL
     return status;
   }
 
-  if (proc->in) {
-    stream_init(NULL, proc->in, -1,
-                STRUCT_CAST(uv_stream_t, &proc->in->uv.pipe));
-    proc->in->events = proc->events;
-    proc->in->internal_data = proc;
-    proc->in->internal_close_cb = on_process_stream_close;
+  if (in) {
+    stream_init(NULL, &proc->in, -1,
+                STRUCT_CAST(uv_stream_t, &proc->in.uv.pipe));
+    proc->in.internal_data = proc;
+    proc->in.internal_close_cb = on_process_stream_close;
     proc->refcount++;
   }
 
-  if (proc->out) {
-    stream_init(NULL, proc->out, -1,
-                STRUCT_CAST(uv_stream_t, &proc->out->uv.pipe));
-    proc->out->events = proc->events;
-    proc->out->internal_data = proc;
-    proc->out->internal_close_cb = on_process_stream_close;
+  if (out) {
+    stream_init(NULL, &proc->out, -1,
+                STRUCT_CAST(uv_stream_t, &proc->out.uv.pipe));
+    proc->out.internal_data = proc;
+    proc->out.internal_close_cb = on_process_stream_close;
     proc->refcount++;
   }
 
-  if (proc->err) {
-    stream_init(NULL, proc->err, -1,
-                STRUCT_CAST(uv_stream_t, &proc->err->uv.pipe));
-    proc->err->events = proc->events;
-    proc->err->internal_data = proc;
-    proc->err->internal_close_cb = on_process_stream_close;
+  if (err) {
+    stream_init(NULL, &proc->err, -1,
+                STRUCT_CAST(uv_stream_t, &proc->err.uv.pipe));
+    proc->err.internal_data = proc;
+    proc->err.internal_close_cb = on_process_stream_close;
     proc->refcount++;
   }
 
@@ -125,8 +123,6 @@ void process_teardown(Loop *loop) FUNC_ATTR_NONNULL_ALL
       // Close handles to process without killing it.
       CREATE_EVENT(loop->events, process_close_handles, 1, proc);
     } else {
-      uv_kill(proc->pid, SIGTERM);
-      proc->term_sent = true;
       process_stop(proc);
     }
   }
@@ -138,27 +134,11 @@ void process_teardown(Loop *loop) FUNC_ATTR_NONNULL_ALL
   pty_process_teardown(loop);
 }
 
-// Wrappers around `stream_close` that protect against double-closing.
 void process_close_streams(Process *proc) FUNC_ATTR_NONNULL_ALL
 {
-  process_close_in(proc);
-  process_close_out(proc);
-  process_close_err(proc);
-}
-
-void process_close_in(Process *proc) FUNC_ATTR_NONNULL_ALL
-{
-  CLOSE_PROC_STREAM(proc, in);
-}
-
-void process_close_out(Process *proc) FUNC_ATTR_NONNULL_ALL
-{
-  CLOSE_PROC_STREAM(proc, out);
-}
-
-void process_close_err(Process *proc) FUNC_ATTR_NONNULL_ALL
-{
-  CLOSE_PROC_STREAM(proc, err);
+  stream_may_close(&proc->in);
+  stream_may_close(&proc->out);
+  stream_may_close(&proc->err);
 }
 
 /// Synchronously wait for a process to finish
@@ -166,16 +146,14 @@ void process_close_err(Process *proc) FUNC_ATTR_NONNULL_ALL
 /// @param process  Process instance
 /// @param ms       Time in milliseconds to wait for the process.
 ///                 0 for no wait. -1 to wait until the process quits.
-/// @return Exit code of the process.
+/// @return Exit code of the process. proc->status will have the same value.
 ///         -1 if the timeout expired while the process is still running.
 ///         -2 if the user interruped the wait.
 int process_wait(Process *proc, int ms, MultiQueue *events)
   FUNC_ATTR_NONNULL_ARG(1)
 {
-  int status = -1;  // default
-  bool interrupted = false;
   if (!proc->refcount) {
-    status = proc->status;
+    int status = proc->status;
     LOOP_PROCESS_EVENTS(proc->loop, proc->events, 0);
     return status;
   }
@@ -195,7 +173,6 @@ int process_wait(Process *proc, int ms, MultiQueue *events)
   // we'll assume that a user frantically hitting interrupt doesn't like
   // the current job. Signal that it has to be killed.
   if (got_int) {
-    interrupted = true;
     got_int = false;
     process_stop(proc);
     if (ms == -1) {
@@ -206,12 +183,13 @@ int process_wait(Process *proc, int ms, MultiQueue *events)
     } else {
       LOOP_PROCESS_EVENTS(proc->loop, events, 0);
     }
+
+    proc->status = -2;
   }
 
   if (proc->refcount == 1) {
     // Job exited, collect status and manually invoke close_cb to free the job
     // resources
-    status = interrupted ? -2 : proc->status;
     decref(proc);
     if (events) {
       // the decref call created an exit event, process it now
@@ -221,7 +199,7 @@ int process_wait(Process *proc, int ms, MultiQueue *events)
     proc->refcount--;
   }
 
-  return status;
+  return proc->status;
 }
 
 /// Ask a process to terminate and eventually kill if it doesn't respond
@@ -237,7 +215,8 @@ void process_stop(Process *proc) FUNC_ATTR_NONNULL_ALL
       // Close the process's stdin. If the process doesn't close its own
       // stdout/stderr, they will be closed when it exits(possibly due to being
       // terminated after a timeout)
-      process_close_in(proc);
+      stream_may_close(&proc->in);
+      os_proc_tree_kill(proc->pid, SIGTERM);
       break;
     case kProcessTypePty:
       // close all streams for pty processes to send SIGHUP to the process
@@ -251,9 +230,10 @@ void process_stop(Process *proc) FUNC_ATTR_NONNULL_ALL
   Loop *loop = proc->loop;
   if (!loop->children_stop_requests++) {
     // When there's at least one stop request pending, start a timer that
-    // will periodically check if a signal should be send to a to the job
-    DLOG("Starting job kill timer");
-    uv_timer_start(&loop->children_kill_timer, children_kill_cb, 100, 100);
+    // will periodically check if a signal should be send to the job.
+    ILOG("starting job kill timer");
+    uv_timer_start(&loop->children_kill_timer, children_kill_cb,
+                   KILL_TIMEOUT_MS, KILL_TIMEOUT_MS);
   }
 }
 
@@ -269,15 +249,13 @@ static void children_kill_cb(uv_timer_t *handle)
     if (!proc->stopped_time) {
       continue;
     }
-    uint64_t elapsed = now - proc->stopped_time;
-
-    if (!proc->term_sent && elapsed >= TERM_TIMEOUT) {
-      ILOG("Sending SIGTERM to pid %d", proc->pid);
-      uv_kill(proc->pid, SIGTERM);
-      proc->term_sent = true;
-    } else if (elapsed >= KILL_TIMEOUT) {
-      ILOG("Sending SIGKILL to pid %d", proc->pid);
-      uv_kill(proc->pid, SIGKILL);
+    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
+                : SIGKILL;
+      os_proc_tree_kill(proc->pid, sig);
     }
   }
 }
@@ -324,6 +302,13 @@ static void process_close(Process *proc)
   }
   assert(!proc->closed);
   proc->closed = true;
+
+  if (proc->detach) {
+    if (proc->type == kProcessTypeUv) {
+      uv_unref((uv_handle_t *)&(((LibuvProcess *)proc)->uv));
+    }
+  }
+
   switch (proc->type) {
     case kProcessTypeUv:
       libuv_process_close((LibuvProcess *)proc);
@@ -368,15 +353,15 @@ static void flush_stream(Process *proc, Stream *stream)
 
     // Poll for data and process the generated events.
     loop_poll_events(proc->loop, 0);
-    if (proc->events) {
-        multiqueue_process_events(proc->events);
+    if (stream->events) {
+        multiqueue_process_events(stream->events);
     }
 
     // Stream can be closed if it is empty.
     if (num_bytes == stream->num_bytes) {
-      if (stream->read_cb) {
+      if (stream->read_cb && !stream->did_eof) {
         // Stream callback could miss EOF handling if a child keeps the stream
-        // open.
+        // open. But only send EOF if we haven't already.
         stream->read_cb(stream, stream->buffer, 0, stream->cb_data, true);
       }
       break;
@@ -388,8 +373,8 @@ static void process_close_handles(void **argv)
 {
   Process *proc = argv[0];
 
-  flush_stream(proc, proc->out);
-  flush_stream(proc, proc->err);
+  flush_stream(proc, &proc->out);
+  flush_stream(proc, &proc->err);
 
   process_close_streams(proc);
   process_close(proc);