1 files changed, 80 insertions, 83 deletions

diff --git a/src/nvim/event/process.c b/src/nvim/event/process.c
index 4429a65f92..4eb2dd0baf 100644
--- a/src/nvim/event/process.c
+++ b/src/nvim/event/process.c
@@ -1,3 +1,6 @@
+// This is an open source non-commercial project. Dear PVS-Studio, please check
+// it. PVS-Studio Static Code Analyzer for C, C++ and C#: http://www.viva64.com
+
 #include <assert.h>
 #include <stdlib.h>
 
@@ -11,38 +14,39 @@
 #include "nvim/event/libuv_process.h"
 #include "nvim/os/pty_process.h"
 #include "nvim/globals.h"
+#include "nvim/macros.h"
 #include "nvim/log.h"
 
 #ifdef INCLUDE_GENERATED_DECLARATIONS
 # 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;
@@ -58,14 +62,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) {
@@ -78,27 +82,27 @@ int process_spawn(Process *proc) FUNC_ATTR_NONNULL_ALL
     return status;
   }
 
-  if (proc->in) {
-    stream_init(NULL, proc->in, -1, (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, (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, (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++;
   }
 
@@ -118,8 +122,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);
     }
   }
@@ -131,27 +133,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
@@ -159,16 +145,15 @@ 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;
   }
@@ -204,7 +189,9 @@ 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
-    status = interrupted ? -2 : proc->status;
+    if (interrupted) {
+      proc->status = -2;
+    }
     decref(proc);
     if (events) {
       // the decref call created an exit event, process it now
@@ -214,7 +201,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
@@ -230,7 +217,9 @@ 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);
+      ILOG("Sending SIGTERM to pid %d", proc->pid);
+      uv_kill(proc->pid, SIGTERM);
       break;
     case kProcessTypePty:
       // close all streams for pty processes to send SIGHUP to the process
@@ -244,9 +233,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);
   }
 }
 
@@ -262,15 +252,15 @@ 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;
+      ILOG("Sending %s to pid %d", sig == SIGTERM ? "SIGTERM" : "SIGKILL",
+           proc->pid);
+      uv_kill(proc->pid, sig);
     }
   }
 }
@@ -317,6 +307,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);
@@ -361,15 +358,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;
@@ -381,8 +378,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);