Merge PR #2237 'Job fixes to core and tests'

src/nvim/os/job.c

@@ -1,6 +1,8 @@
 #include <stdint.h>
 #include <stdbool.h>
 
+#include <sys/wait.h>
+
 #include <uv.h>
 
 #include "nvim/os/uv_helpers.h"
@@ -43,6 +45,7 @@
 Job *table[MAX_RUNNING_JOBS] = {NULL};
 size_t stop_requests = 0;
 uv_timer_t job_stop_timer;
+uv_signal_t schld;
 
 // Some helpers shared in this module
 
@@ -56,6 +59,8 @@ void job_init(void)
 {
   uv_disable_stdio_inheritance();
   uv_timer_init(uv_default_loop(), &job_stop_timer);
+  uv_signal_init(uv_default_loop(), &schld);
+  uv_signal_start(&schld, chld_handler, SIGCHLD);
 }
 
 /// Releases job control resources and terminates running jobs
@@ -73,6 +78,8 @@ void job_teardown(void)
 
   // Wait until all jobs are closed
   event_poll_until(-1, !stop_requests);
+  uv_signal_stop(&schld);
+  uv_close((uv_handle_t *)&schld, NULL);
   // Close the timer
   uv_close((uv_handle_t *)&job_stop_timer, NULL);
 }
@@ -135,13 +142,13 @@ Job *job_start(JobOptions opts, int *status)
   // Spawn the job
   if (!process_spawn(job)) {
     if (opts.writable) {
-      uv_close((uv_handle_t *)&job->proc_stdin, close_cb);
+      uv_close((uv_handle_t *)job->proc_stdin, close_cb);
     }
     if (opts.stdout_cb) {
-      uv_close((uv_handle_t *)&job->proc_stdout, close_cb);
+      uv_close((uv_handle_t *)job->proc_stdout, close_cb);
     }
     if (opts.stderr_cb) {
-      uv_close((uv_handle_t *)&job->proc_stderr, close_cb);
+      uv_close((uv_handle_t *)job->proc_stderr, close_cb);
     }
     process_close(job);
     event_poll(0);
@@ -200,10 +207,16 @@ void job_stop(Job *job)
   }
 
   job->stopped_time = os_hrtime();
-  // Close the job's stdin. If the job doesn't close it's own stdout/stderr,
+  if (job->opts.pty) {
+    // close all streams for pty jobs to send SIGHUP to the process
+    job_close_streams(job);
+    pty_process_close_master(job);
+  } else {
+    // Close the job's stdin. If the job doesn't close its own stdout/stderr,
     // they will be closed when the job exits(possibly due to being terminated
     // after a timeout)
     close_job_in(job);
+  }
 
   if (!stop_requests++) {
     // When there's at least one stop request pending, start a timer that
@@ -312,6 +325,12 @@ int job_id(Job *job)
   return job->id;
 }
 
+// Get the job pid
+int job_pid(Job *job)
+{
+  return job->pid;
+}
+
 /// Get data associated with a job
 ///
 /// @param job A pointer to the job
@@ -331,6 +350,13 @@ bool job_resize(Job *job, uint16_t width, uint16_t height)
   return true;
 }
 
+void job_close_streams(Job *job)
+{
+  close_job_in(job);
+  close_job_out(job);
+  close_job_err(job);
+}
+
 /// Iterates the table, sending SIGTERM to stopped jobs and SIGKILL to those
 /// that didn't die from SIGTERM after a while(exit_timeout is 0).
 static void job_stop_timer_cb(uv_timer_t *handle)
@@ -375,14 +401,41 @@ static void read_cb(RStream *rstream, void *data, bool eof)
   }
 }
 
-void job_close_streams(Job *job)
-{
-  close_job_in(job);
-  close_job_out(job);
-  close_job_err(job);
-}
-
 static void close_cb(uv_handle_t *handle)
 {
   job_decref(handle_get_job(handle));
 }
+
+static void chld_handler(uv_signal_t *handle, int signum)
+{
+  int stat = 0;
+  int pid;
+
+  do {
+    pid = waitpid(-1, &stat, WNOHANG);
+  } while (pid < 0 && errno == EINTR);
+
+  if (pid <= 0) {
+    return;
+  }
+
+  if (WIFSTOPPED(stat) || WIFCONTINUED(stat)) {
+    // Only care for processes that exited
+    return;
+  }
+
+  Job *job = NULL;
+  // find the job corresponding to the exited pid
+  for (int i = 0; i < MAX_RUNNING_JOBS; i++) {
+    if ((job = table[i]) != NULL && job->pid == pid) {
+      if (WIFEXITED(stat)) {
+        job->status = WEXITSTATUS(stat);
+      } else if (WIFSIGNALED(stat)) {
+        job->status = WTERMSIG(stat);
+      }
+      process_close(job);
+      break;
+    }
+  }
+}
+

src/nvim/os/pty_process.c

@@ -34,7 +34,6 @@
 typedef struct {
   struct winsize winsize;
   uv_pipe_t proc_stdin, proc_stdout, proc_stderr;
-  uv_signal_t schld;
   int tty_fd;
 } PtyProcess;
 
@@ -136,9 +135,6 @@ bool pty_process_spawn(Job *job) FUNC_ATTR_NONNULL_ALL
     goto error;
   }
 
-  uv_signal_init(uv_default_loop(), &ptyproc->schld);
-  uv_signal_start(&ptyproc->schld, chld_handler, SIGCHLD);
-  ptyproc->schld.data = job;
   ptyproc->tty_fd = master;
   job->pid = pid;
   return true;
@@ -162,9 +158,6 @@ error:
 
 void pty_process_close(Job *job) FUNC_ATTR_NONNULL_ALL
 {
-  PtyProcess *ptyproc = job->process;
-  uv_signal_stop(&ptyproc->schld);
-  uv_close((uv_handle_t *)&ptyproc->schld, NULL);
   pty_process_close_master(job);
   job_close_streams(job);
   job_decref(job);
@@ -207,31 +200,6 @@ static void init_child(Job *job) FUNC_ATTR_NONNULL_ALL
   fprintf(stderr, "execvp failed: %s\n", strerror(errno));
 }
 
-static void chld_handler(uv_signal_t *handle, int signum) FUNC_ATTR_NONNULL_ALL
-{
-  Job *job = handle->data;
-  int stat = 0;
-
-  if (waitpid(job->pid, &stat, 0) < 0) {
-    fprintf(stderr, "Waiting for pid %d failed: %s\n", job->pid,
-        strerror(errno));
-    return;
-  }
-
-  if (WIFSTOPPED(stat) || WIFCONTINUED(stat)) {
-    // Did not exit
-    return;
-  }
-
-  if (WIFEXITED(stat)) {
-    job->status = WEXITSTATUS(stat);
-  } else if (WIFSIGNALED(stat)) {
-    job->status = WTERMSIG(stat);
-  }
-
-  pty_process_close(job);
-}
-
 static void init_termios(struct termios *termios) FUNC_ATTR_NONNULL_ALL
 {
   memset(termios, 0, sizeof(struct termios));

test/functional/job/tty-test.c

@@ -80,6 +80,12 @@ static void read_cb(uv_stream_t *stream, ssize_t cnt, const uv_buf_t *buf)
   free(buf->base);
 }
 
+static void prepare_cb(uv_prepare_t *handle)
+{
+  fprintf(stderr, "tty ready\n");
+  uv_prepare_stop(handle);
+}
+
 int main(int argc, char **argv)
 {
   if (!is_terminal(stdin)) {
@@ -98,7 +104,9 @@ int main(int argc, char **argv)
   }
 
   bool interrupted = false;
-  fprintf(stderr, "tty ready\n");
+  uv_prepare_t prepare;
+  uv_prepare_init(uv_default_loop(), &prepare);
+  uv_prepare_start(&prepare, prepare_cb);
   uv_tty_t tty;
   uv_tty_init(uv_default_loop(), &tty, fileno(stderr), 1);
   uv_read_start((uv_stream_t *)&tty, alloc_cb, read_cb);