test,unix: fix race in test runner

The test runner inserted a 250 ms delay to give helper processes time to
settle. That's intrinsically race-y and caused tests to intermittently
fail on platforms like AIX.

Instead of a fixed delay, pass a file descriptor to the helper process
and wait until it closes the descriptor. That way we know for sure the
process has started.

Incidentally, this change reduces the running time of the test suite
from 112 to 26 seconds on my machine.

Fixes: libuv#2041
PR-URL: libuv#2056
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Santiago Gimeno <[email protected]>

test/echo-server.c

@@ -340,6 +340,7 @@ HELPER_IMPL(tcp4_echo_server) {
   if (tcp4_echo_start(TEST_PORT))
     return 1;
 
+  notify_parent_process();
   uv_run(loop, UV_RUN_DEFAULT);
   return 0;
 }
@@ -351,6 +352,7 @@ HELPER_IMPL(tcp6_echo_server) {
   if (tcp6_echo_start(TEST_PORT))
     return 1;
 
+  notify_parent_process();
   uv_run(loop, UV_RUN_DEFAULT);
   return 0;
 }
@@ -362,6 +364,7 @@ HELPER_IMPL(pipe_echo_server) {
   if (pipe_echo_start(TEST_PIPENAME))
     return 1;
 
+  notify_parent_process();
   uv_run(loop, UV_RUN_DEFAULT);
   return 0;
 }
@@ -373,6 +376,7 @@ HELPER_IMPL(udp4_echo_server) {
   if (udp4_echo_start(TEST_PORT))
     return 1;
 
+  notify_parent_process();
   uv_run(loop, UV_RUN_DEFAULT);
   return 0;
 }

test/run-tests.c

@@ -109,33 +109,39 @@ static int maybe_run_test(int argc, char **argv) {
   }
 
   if (strcmp(argv[1], "spawn_helper1") == 0) {
+    notify_parent_process();
     return 1;
   }
 
   if (strcmp(argv[1], "spawn_helper2") == 0) {
+    notify_parent_process();
     printf("hello world\n");
     return 1;
   }
 
   if (strcmp(argv[1], "spawn_tcp_server_helper") == 0) {
+    notify_parent_process();
     return spawn_tcp_server_helper();
   }
 
   if (strcmp(argv[1], "spawn_helper3") == 0) {
     char buffer[256];
+    notify_parent_process();
     ASSERT(buffer == fgets(buffer, sizeof(buffer) - 1, stdin));
     buffer[sizeof(buffer) - 1] = '\0';
     fputs(buffer, stdout);
     return 1;
   }
 
   if (strcmp(argv[1], "spawn_helper4") == 0) {
+    notify_parent_process();
     /* Never surrender, never return! */
     while (1) uv_sleep(10000);
   }
 
   if (strcmp(argv[1], "spawn_helper5") == 0) {
     const char out[] = "fourth stdio!\n";
+    notify_parent_process();
 #ifdef _WIN32
     DWORD bytes;
     WriteFile((HANDLE) _get_osfhandle(3), out, sizeof(out) - 1, &bytes, NULL);
@@ -156,6 +162,8 @@ static int maybe_run_test(int argc, char **argv) {
   if (strcmp(argv[1], "spawn_helper6") == 0) {
     int r;
 
+    notify_parent_process();
+
     r = fprintf(stdout, "hello world\n");
     ASSERT(r > 0);
 
@@ -168,6 +176,9 @@ static int maybe_run_test(int argc, char **argv) {
   if (strcmp(argv[1], "spawn_helper7") == 0) {
     int r;
     char *test;
+
+    notify_parent_process();
+
     /* Test if the test value from the parent is still set */
     test = getenv("ENV_TEST");
     ASSERT(test != NULL);
@@ -181,6 +192,8 @@ static int maybe_run_test(int argc, char **argv) {
 #ifndef _WIN32
   if (strcmp(argv[1], "spawn_helper8") == 0) {
     int fd;
+
+    notify_parent_process();
     ASSERT(sizeof(fd) == read(0, &fd, sizeof(fd)));
     ASSERT(fd > 2);
     ASSERT(-1 == write(fd, "x", 1));
@@ -190,6 +203,7 @@ static int maybe_run_test(int argc, char **argv) {
 #endif  /* !_WIN32 */
 
   if (strcmp(argv[1], "spawn_helper9") == 0) {
+    notify_parent_process();
     return spawn_stdin_stdout();
   }
 
@@ -200,6 +214,7 @@ static int maybe_run_test(int argc, char **argv) {
 
     ASSERT(uid == getuid());
     ASSERT(gid == getgid());
+    notify_parent_process();
 
     return 1;
   }

test/runner-unix.c

@@ -42,6 +42,30 @@
 
 extern char** environ;
 
+static void closefd(int fd) {
+  if (close(fd) == 0 || errno == EINTR || errno == EINPROGRESS)
+    return;
+
+  perror("close");
+  abort();
+}
+
+
+void notify_parent_process(void) {
+  char* arg;
+  int fd;
+
+  arg = getenv("UV_TEST_RUNNER_FD");
+  if (arg == NULL)
+    return;
+
+  fd = atoi(arg);
+  assert(fd > STDERR_FILENO);
+  unsetenv("UV_TEST_RUNNER_FD");
+  closefd(fd);
+}
+
+
 /* Do platform-specific initialization. */
 int platform_init(int argc, char **argv) {
   /* Disable stdio output buffering. */
@@ -65,6 +89,9 @@ int process_start(char* name, char* part, process_info_t* p, int is_helper) {
   int stdout_fd;
   const char* arg;
   char* args[16];
+  int pipefd[2];
+  char fdstr[8];
+  ssize_t rc;
   int n;
   pid_t pid;
 
@@ -94,6 +121,19 @@ int process_start(char* name, char* part, process_info_t* p, int is_helper) {
     return -1;
   }
 
+  if (is_helper) {
+    if (pipe(pipefd)) {
+      perror("pipe");
+      return -1;
+    }
+
+    snprintf(fdstr, sizeof(fdstr), "%d", pipefd[1]);
+    if (setenv("UV_TEST_RUNNER_FD", fdstr, /* overwrite */ 1)) {
+      perror("setenv");
+      return -1;
+    }
+  }
+
   p->terminated = 0;
   p->status = 0;
 
@@ -106,6 +146,8 @@ int process_start(char* name, char* part, process_info_t* p, int is_helper) {
 
   if (pid == 0) {
     /* child */
+    if (is_helper)
+      closefd(pipefd[0]);
     dup2(stdout_fd, STDOUT_FILENO);
     dup2(stdout_fd, STDERR_FILENO);
     execve(args[0], args, environ);
@@ -118,6 +160,28 @@ int process_start(char* name, char* part, process_info_t* p, int is_helper) {
   p->name = strdup(name);
   p->stdout_file = stdout_file;
 
+  if (!is_helper)
+    return 0;
+
+  closefd(pipefd[1]);
+  unsetenv("UV_TEST_RUNNER_FD");
+
+  do
+    rc = read(pipefd[0], &n, 1);
+  while (rc == -1 && errno == EINTR);
+
+  closefd(pipefd[0]);
+
+  if (rc == -1) {
+    perror("read");
+    return -1;
+  }
+
+  if (rc > 0) {
+    fprintf(stderr, "EOF expected but got data.\n");
+    return -1;
+  }
+
   return 0;
 }
 

test/runner-win.c

@@ -76,6 +76,11 @@ int process_start(char *name, char *part, process_info_t *p, int is_helper) {
   PROCESS_INFORMATION pi;
   DWORD result;
 
+  if (!is_helper) {
+    /* Give the helpers time to settle. Race-y, fix this. */
+    uv_sleep(250);
+  }
+
   if (GetTempPathW(sizeof(path) / sizeof(WCHAR), (WCHAR*)&path) == 0)
     goto error;
   if (GetTempFileNameW((WCHAR*)&path, L"uv", 0, (WCHAR*)&filename) == 0)

test/runner.c

@@ -215,9 +215,6 @@ int run_test(const char* test,
     process_count++;
   }
 
-  /* Give the helpers time to settle. Race-y, fix this. */
-  uv_sleep(250);
-
   /* Now start the test itself. */
   for (task = TASKS; task->main; task++) {
     if (strcmp(test, task->task_name) != 0) {

test/task.h

@@ -181,6 +181,12 @@ extern int snprintf(char*, size_t, const char*, ...);
 # define UNUSED
 #endif
 
+#if defined(_WIN32)
+#define notify_parent_process() ((void) 0)
+#else
+extern void notify_parent_process(void);
+#endif
+
 /* Fully close a loop */
 static void close_walk_cb(uv_handle_t* handle, void* arg) {
   if (!uv_is_closing(handle))

test/test-ipc-heavy-traffic-deadlock-bug.c

@@ -150,6 +150,7 @@ int ipc_helper_heavy_traffic_deadlock_bug(void) {
   r = uv_pipe_open(&pipe, 0);
   ASSERT(r == 0);
 
+  notify_parent_process();
   do_writes_and_reads((uv_stream_t*) &pipe);
   uv_sleep(100);
 

test/test-ipc-send-recv.c

@@ -397,6 +397,7 @@ int run_ipc_send_recv_helper(uv_loop_t* loop, int inprocess) {
     send_recv_start();
   }
 
+  notify_parent_process();
   r = uv_run(loop, UV_RUN_DEFAULT);
   ASSERT(r == 0);
 

test/test-ipc.c

@@ -724,6 +724,7 @@ int ipc_helper(int listen_after_write) {
     ASSERT(r == 0);
   }
 
+  notify_parent_process();
   r = uv_run(uv_default_loop(), UV_RUN_DEFAULT);
   ASSERT(r == 0);
 

test/test-stdio-over-pipes.c

@@ -232,6 +232,7 @@ int stdio_over_pipes_helper(void) {
     ASSERT(r == 0);
   }
 
+  notify_parent_process();
   uv_run(loop, UV_RUN_DEFAULT);
 
   ASSERT(after_write_called == 7);