Rewrite, update, and refactor sockets-related tests (WebAssembly#768)

In WebAssembly#762 I'm seeing an error related to `EADDRINUSE` which I can't
reproduce locally. Tests currently all use the same port which means
that they are required to be run serially and additionally may run into
this risk if the port happens to already be taken. I don't really know
what's going on in CI, but I wanted to nevertheless refactor these
tests.

This commit rearchitects, rewrites, and fixes sockets-related tests.
Changes here are:

* Servers now print the port they're listening to. Clients read this
port and then connect to that port. This orchestration happens through
some specially crafted bash and CMake, similar to before. All tests now
bind to port 0 to get an OS-assigned port instead of picking a port.

* All tests are now ungated, notably enabling some nonblocking I/O tests
that were previously gated.

* A number of tests are refactored or updated to be less flaky. Many
tests were failing nondeterministically locally due to how the test was
constructed and assuming certain things were ready when they may not be.
Some tests were entirely rewritten, such as
`sockets-nonblocking-multiple.c`, while others were only lightly
refactored to `poll` in a few more places.

* All tests are updated to pass in less than 100ms. Some tests before
hung for awhile as timeouts were reached for example, but this wasn't a
necessary part of the test. Other tests for nonblocking I/O had many
calls to `poll` with a number of them reaching the timeout. These are
rewritten to use a single call to `poll` with no timeout which enables
the test to progress much faster as timeouts are never reached.

Overall the intent is that the sockets-related tests are more
comprehensive now, more robust and/or less flaky, and work in more
environments.

Author: alexcrichton

test/CMakeLists.txt

@@ -123,7 +123,7 @@ function(register_test test_name executable_name)
     list(APPEND wasmtime_args --dir ${fsdir}::/)
   endif()
   if (arg_NETWORK)
-    list(APPEND wasmtime_args -Sinherit-network)
+    list(APPEND wasmtime_args -Sinherit-network,allow-ip-name-lookup)
   endif()
   foreach(env IN LISTS arg_ENV)
     list(APPEND wasmtime_args --env ${env})
@@ -135,7 +135,6 @@ function(register_test test_name executable_name)
     list(APPEND wasmtime_args --wasm component-model-async)
     list(APPEND wasmtime_args --wasi p3)
   endif()
-  list(APPEND wasmtime_args --wasi allow-ip-name-lookup)
 
   add_test(
     NAME "${test_name}"
@@ -155,12 +154,6 @@ function(register_test test_name executable_name)
     set_tests_properties("${test_name}" PROPERTIES FIXTURES_REQUIRED "fs_${test_name}")
   endif()
 
-  # All sockets tests use the same port right now, so only one can run at a
-  # time.
-  if (arg_NETWORK)
-    set_tests_properties(${test_name} PROPERTIES RESOURCE_LOCK socket-test)
-  endif()
-
   if (arg_PASS_REGULAR_EXPRESSION)
     set_tests_properties(${test_name} PROPERTIES PASS_REGULAR_EXPRESSION "${arg_PASS_REGULAR_EXPRESSION}")
   endif()
@@ -341,10 +334,8 @@ if (NOT (WASI STREQUAL "p1"))
   add_wasilibc_test(sockets-fcntl.c NETWORK)
   add_wasilibc_test(sockets-udp-connected.c NETWORK)
   add_wasilibc_test(getaddrinfo.c NETWORK)
-
-  # TODO: flaky tests
-  # add_wasilibc_test(sockets-nonblocking.c NETWORK)
-  # add_wasilibc_test(sockets-nonblocking-udp-no-connection.c NETWORK)
+  add_wasilibc_test(sockets-nonblocking.c NETWORK FAILP3)
+  add_wasilibc_test(sockets-nonblocking-udp-no-connection.c NETWORK FAILP3)
 
   # Define executables for server/client tests, and they're paired together in
   # various combinations below for various tests.
@@ -387,7 +378,6 @@ if (NOT (WASI STREQUAL "p1"))
           -DNCLIENTS=${arg_NCLIENTS}
           -P ${CMAKE_CURRENT_SOURCE_DIR}/socket-test.cmake
     )
-    set_tests_properties(${test_name} PROPERTIES RESOURCE_LOCK socket-test)
 
     if (arg_FAILP3 AND (WASI STREQUAL "p3"))
       set_tests_properties(${test_name} PROPERTIES WILL_FAIL TRUE)

test/socket-test.cmake

@@ -10,11 +10,39 @@ if (NOT NCLIENTS)
 endif()
 
 foreach(i RANGE 1 ${NCLIENTS})
-  list(APPEND CLIENTS COMMAND bash -c "exec ${ENGINE} -Wcomponent-model-async -Sp3,inherit-network ${CLIENT} 1>&2")
+  # This is a bit tricky to setup, but the flow is:
+  #
+  # * The first line of output from the server is the port number that it's
+  #   listening on.
+  #
+  # * This is read by the script below, and then printed again. In case it wasn't
+  #   actually a port number, or if there's more clients, this means that the port
+  #   will show up to the next client or print at the end if it's an error.
+  #
+  # * Next the shell's stdout is closed by putting stderr into the stdout slot.
+  #   This closes stdout for the next client in the pipeline or the final cmake
+  #   reader itself. Importantly this means that any prints of the client will
+  #   show up on stderr which will make its way out of cmake.
+  #
+  # * Finally the client is run with the port as an argument.
+  #
+  # Overall this should handle cross-process synchronization to let the server bind
+  # any arbitrary port, print out the port, and then thread that port to any number
+  # of clients. All the while all other output from the tests is still surfaced in
+  # CMake to debug and such. A bit baroque but we are scripting in CMake after all
+  # so there's only so much that can be done.
+  set(client_script "
+read port  # read the first line of stdin from the previous process
+echo $port # forward this line to the next process, or out to cmake itself
+exec 2>&1  # close our stdout and replace it with stderr
+cat <&0 &  # forward the rest of stdin to stderr so it shows up in cmake
+exec ${ENGINE} -Wcomponent-model-async -Sp3,inherit-network ${CLIENT} \"$port\"
+  ")
+  list(APPEND CLIENTS COMMAND bash -c ${client_script})
 endforeach()
 
 execute_process(
-  COMMAND bash -c "exec ${ENGINE} -Wcomponent-model-async -Sp3,inherit-network ${SERVER} 1>&2"
+  COMMAND ${ENGINE} -Wcomponent-model-async -Sp3,inherit-network ${SERVER}
   ${CLIENTS}
   TIMEOUT 5
   COMMAND_ERROR_IS_FATAL ANY

test/src/poll-connect.c

@@ -115,9 +115,20 @@ int main() {
 
   // Test that we can `recv` what we `send`
   {
-    // Send some data from the server to the client.
     uint8_t data[5] = {1, 2, 3, 4, 5};
-    TEST(send(server_client, data, sizeof(data), 0) == sizeof(data));
+    while (!t_status) {
+      // Send some data from the server to the client.
+      ssize_t sent = send(server_client, data, sizeof(data), 0);
+      if (sent != -1) {
+        TEST2(sent == sizeof(data));
+        break;
+      }
+      if (errno != EWOULDBLOCK)
+        t_error("send failed (errno = %d)\n", errno);
+      struct pollfd fd = {
+          .fd = server_client, .events = POLLWRNORM, .revents = 0};
+      TEST(poll(&fd, 1, -1) != -1);
+    }
 
     fds[0].events = POLLRDNORM;
     errno = 0;

test/src/poll-nonblocking-socket.c

@@ -26,11 +26,21 @@ void test_tcp_client() {
   int socket_fd = socket(AF_INET, SOCK_STREAM, 0);
   TEST(socket_fd != -1);
 
-  int server_port = 4001;
+  // Bind a socket to get a free port, and leave it open. Note that `listen`
+  // is specifically not used to refuse future `connect`s.
+  int tmp_fd = socket(AF_INET, SOCK_STREAM, 0);
+  struct sockaddr_in tmp_addr;
+  socklen_t tmp_addr_len = sizeof(tmp_addr);
+  tmp_addr.sin_family = AF_INET;
+  tmp_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+  tmp_addr.sin_port = 0;
+  TEST(bind(tmp_fd, (struct sockaddr *)&tmp_addr, sizeof(tmp_addr)) != -1);
+  TEST(getsockname(tmp_fd, (struct sockaddr *)&tmp_addr, &tmp_addr_len) != -1);
+  int server_port = ntohs(tmp_addr.sin_port);
 
   // Check that if a non-blocking socket is in progress, poll() indicates the
   // right result
-  socket_fd = socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0);
+  TEST((socket_fd = socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0)) != -1);
   struct sockaddr_in sockaddr_in;
   // Doesn't matter what address we connect to, since we only attempt to connect
   // once
@@ -44,14 +54,17 @@ void test_tcp_client() {
 
   // Poll the socket for writability
   struct pollfd poll_fd = {.fd = socket_fd, .events = POLLWRNORM, .revents = 0};
-  poll(&poll_fd, 1, 10);
+  TEST(poll(&poll_fd, 1, 10) != -1);
 
   // Socket should not be writable
   int32_t error = -1;
   socklen_t len = -1;
   TEST(getsockopt(socket_fd, SOL_SOCKET, SO_ERROR, &error, &len) == 0);
   TEST(error == ECONNREFUSED);
-  close(socket_fd);
+  TEST(close(socket_fd) == 0);
+
+  // Release our port which is now no longer needed.
+  TEST(close(tmp_fd) == 0);
 }
 
 int main(void) {

test/src/sockets-client-handle-hangups.c

@@ -19,23 +19,9 @@
 
 int BUFSIZE = 256;
 
-static int wait_for_server(struct sockaddr_in *addr) {
-  for (int attempt = 0; attempt < 200; attempt++) {
-    int fd = socket(AF_INET, SOCK_STREAM, 0);
-    TEST(fd != -1);
-    if (connect(fd, (struct sockaddr *)addr, sizeof(*addr)) != -1)
-      return fd;
-    close(fd);
-    usleep(5000); // sleep for 5ms
-  }
-  return -1;
-}
-
 // Server must be running already as a separate executable
 // (except when testing the case where connect() fails)
-void test_tcp_client() {
-  // Prepare server socket
-  int server_port = 4001;
+void test_tcp_client(int server_port) {
 
   // Prepare sockaddr_in for client
   struct sockaddr_in sockaddr_in;
@@ -48,14 +34,19 @@ void test_tcp_client() {
   int len = strlen(message);
   char client_buffer[BUFSIZE];
 
-  int socket_fd = wait_for_server(&sockaddr_in);
-  if (socket_fd == -1)
+  int socket_fd = socket(AF_INET, SOCK_STREAM, 0);
+  TEST(socket_fd != -1);
+  if (connect(socket_fd, (struct sockaddr *)&sockaddr_in,
+              sizeof(sockaddr_in)) == -1) {
+    close(socket_fd);
     return;
+  }
 
   // Client writes a message to server
   // This should succeed even in the presence of hangups,
   // since send() only fails if there's a local error
   TEST(send(socket_fd, message, len, 0) == len);
+  TEST(shutdown(socket_fd, SHUT_WR) != -1);
 
   // Set timeout for recv
   struct timeval tv;
@@ -74,8 +65,18 @@ void test_tcp_client() {
   close(socket_fd);
 }
 
-int main(void) {
-  test_tcp_client();
+int main(int argc, char *argv[]) {
+  if (argc != 2) {
+    fprintf(stderr, "Usage: %s <server_port>\n", argv[0]);
+    return 1;
+  }
+  int port;
+  if (sscanf(argv[1], "%d", &port) != 1) {
+    // test a port without anything listening on it
+    test_tcp_client(1);
+  } else {
+    test_tcp_client(port);
+  }
 
   return t_status;
 }

test/src/sockets-client-hangup-after-connect.c

@@ -19,40 +19,36 @@
 
 int BUFSIZE = 256;
 
-static int wait_for_server(struct sockaddr_in *addr) {
-  for (int attempt = 0; attempt < 200; attempt++) {
-    int fd = socket(AF_INET, SOCK_STREAM, 0);
-    TEST(fd != -1);
-    if (connect(fd, (struct sockaddr *)addr, sizeof(*addr)) != -1)
-      return fd;
-    close(fd);
-    usleep(5000); // sleep for 5ms
-  }
-  t_error("server didn't come online within 1 second (errno = %d)\n", errno);
-  return -1;
-}
-
 // Server must be running already as a separate executable
 // (except when testing the case where connect() fails)
-void test_tcp_client() {
-  // Prepare server socket
-  int server_port = 4001;
+void test_tcp_client(int server_port) {
 
   // Prepare sockaddr_in for client
   struct sockaddr_in sockaddr_in;
   sockaddr_in.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
   sockaddr_in.sin_family = AF_INET;
   sockaddr_in.sin_port = htons(server_port);
 
-  // Connect from client
-  int socket_fd = wait_for_server(&sockaddr_in);
+  int socket_fd = socket(AF_INET, SOCK_STREAM, 0);
+  TEST(socket_fd != -1);
+  TEST(connect(socket_fd, (struct sockaddr *)&sockaddr_in,
+               sizeof(sockaddr_in)) == 0);
 
   // Hang up immediately
   close(socket_fd);
 }
 
-int main(void) {
-  test_tcp_client();
+int main(int argc, char *argv[]) {
+  if (argc != 2) {
+    fprintf(stderr, "Usage: %s <server_port>\n", argv[0]);
+    return 1;
+  }
+  int port;
+  if (sscanf(argv[1], "%d", &port) != 1) {
+    fprintf(stderr, "Invalid port number: %s\n", argv[1]);
+    return 1;
+  }
+  test_tcp_client(port);
 
   return t_status;
 }

test/src/sockets-client-hangup-after-sending.c

@@ -19,23 +19,8 @@
 
 int BUFSIZE = 256;
 
-static int wait_for_server(struct sockaddr_in *addr) {
-  for (int attempt = 0; attempt < 200; attempt++) {
-    int fd = socket(AF_INET, SOCK_STREAM, 0);
-    TEST(fd != -1);
-    if (connect(fd, (struct sockaddr *)addr, sizeof(*addr)) != -1)
-      return fd;
-    close(fd);
-    usleep(5000); // sleep for 5ms
-  }
-  t_error("server didn't come online within 1 second (errno = %d)\n", errno);
-  return -1;
-}
-
 // See sockets-server.c -- must be running already as a separate executable
-void test_tcp_client() {
-  // Prepare server socket
-  int server_port = 4001;
+void test_tcp_client(int server_port) {
 
   // Prepare sockaddr_in for client
   struct sockaddr_in sockaddr_in;
@@ -47,7 +32,10 @@ void test_tcp_client() {
   char message[] = "There's gonna be a party when the wolf comes home";
   int len = strlen(message);
 
-  int socket_fd = wait_for_server(&sockaddr_in);
+  int socket_fd = socket(AF_INET, SOCK_STREAM, 0);
+  TEST(socket_fd != -1);
+  TEST(connect(socket_fd, (struct sockaddr *)&sockaddr_in,
+               sizeof(sockaddr_in)) == 0);
 
   // Client writes a message to server
   TEST(send(socket_fd, message, len, 0) == len);
@@ -56,8 +44,17 @@ void test_tcp_client() {
   close(socket_fd);
 }
 
-int main(void) {
-  test_tcp_client();
+int main(int argc, char *argv[]) {
+  if (argc != 2) {
+    fprintf(stderr, "Usage: %s <server_port>\n", argv[0]);
+    return 1;
+  }
+  int port;
+  if (sscanf(argv[1], "%d", &port) != 1) {
+    fprintf(stderr, "Invalid port number: %s\n", argv[1]);
+    return 1;
+  }
+  test_tcp_client(port);
 
   return t_status;
 }

test/src/sockets-client-hangup-while-receiving.c

@@ -19,23 +19,8 @@
 
 int BUFSIZE = 256;
 
-static int wait_for_server(struct sockaddr_in *addr) {
-  for (int attempt = 0; attempt < 200; attempt++) {
-    int fd = socket(AF_INET, SOCK_STREAM, 0);
-    TEST(fd != -1);
-    if (connect(fd, (struct sockaddr *)addr, sizeof(*addr)) != -1)
-      return fd;
-    close(fd);
-    usleep(5000); // sleep for 5ms
-  }
-  t_error("server didn't come online within 1 second (errno = %d)\n", errno);
-  return -1;
-}
-
 // See sockets-server.c -- must be running already as a separate executable
-void test_tcp_client() {
-  // Prepare server socket
-  int server_port = 4001;
+void test_tcp_client(int server_port) {
 
   // Prepare sockaddr_in for client
   struct sockaddr_in sockaddr_in;
@@ -48,7 +33,10 @@ void test_tcp_client() {
   int len = strlen(message);
   char client_buffer[BUFSIZE];
 
-  int socket_fd = wait_for_server(&sockaddr_in);
+  int socket_fd = socket(AF_INET, SOCK_STREAM, 0);
+  TEST(socket_fd != -1);
+  TEST(connect(socket_fd, (struct sockaddr *)&sockaddr_in,
+               sizeof(sockaddr_in)) == 0);
 
   // Client writes a message to server
   TEST(send(socket_fd, message, len, 0) == len);
@@ -65,8 +53,17 @@ void test_tcp_client() {
   close(socket_fd);
 }
 
-int main(void) {
-  test_tcp_client();
+int main(int argc, char *argv[]) {
+  if (argc != 2) {
+    fprintf(stderr, "Usage: %s <server_port>\n", argv[0]);
+    return 1;
+  }
+  int port;
+  if (sscanf(argv[1], "%d", &port) != 1) {
+    fprintf(stderr, "Invalid port number: %s\n", argv[1]);
+    return 1;
+  }
+  test_tcp_client(port);
 
   return t_status;
 }