test(self-talk): Replace superstitious and timing-racy sleep() with proper primitives

tests/unit/s2n_mem_allocator_test.c

@@ -82,8 +82,8 @@ void mock_client(struct s2n_test_io_pair *io_pair)
     struct s2n_config *config = NULL;
     s2n_blocked_status blocked;
 
-    /* Give the server a chance to listen */
-    sleep(1);
+    /* The socketpair is valid from fork; the kernel buffers writes until the
+     * parent reads, so no pre-negotiate sleep is needed. */
 
     conn = s2n_connection_new(S2N_CLIENT);
     config = s2n_config_new();
@@ -138,8 +138,8 @@ void mock_client(struct s2n_test_io_pair *io_pair)
     s2n_connection_free(conn);
     s2n_config_free(config);
 
-    /* Give the server a chance to a void a sigpipe */
-    sleep(1);
+    /* SIGPIPE is inherited as SIG_IGN across fork via s2n_io_pair_init, so
+     * no pre-cleanup sleep is needed. */
 
     s2n_io_pair_close_one_end(io_pair, S2N_CLIENT);
 

tests/unit/s2n_self_talk_alerts_test.c

@@ -50,8 +50,8 @@ int mock_client(struct s2n_test_io_pair *io_pair, s2n_alert_behavior alert_behav
     int result = 0;
     int rc = 0;
 
-    /* Give the server a chance to listen */
-    sleep(1);
+    /* The socketpair is valid from fork; the kernel buffers writes until the
+     * parent reads, so no pre-negotiate sleep is needed. */
 
     conn = s2n_connection_new(S2N_CLIENT);
     config = s2n_config_new();

tests/unit/s2n_self_talk_broken_pipe_test.c

@@ -30,14 +30,15 @@
 static const char *certificate_paths[SUPPORTED_CERTIFICATE_FORMATS] = { S2N_RSA_2048_PKCS1_CERT_CHAIN, S2N_RSA_2048_PKCS8_CERT_CHAIN };
 static const char *private_key_paths[SUPPORTED_CERTIFICATE_FORMATS] = { S2N_RSA_2048_PKCS1_KEY, S2N_RSA_2048_PKCS8_KEY };
 
-void mock_client(struct s2n_test_io_pair *io_pair)
+void mock_client(struct s2n_test_io_pair *io_pair, int ready_pipe_w, int done_pipe_r)
 {
     struct s2n_connection *conn = NULL;
     struct s2n_config *config = NULL;
     s2n_blocked_status blocked;
 
-    /* Give the server a chance to listen */
-    sleep(1);
+    /* The socketpair underlying io_pair is valid from the moment of fork,
+     * so the child can start the handshake immediately. The kernel buffers
+     * writes until the parent is ready to read. */
 
     conn = s2n_connection_new(S2N_CLIENT);
     config = s2n_config_new();
@@ -64,8 +65,23 @@ void mock_client(struct s2n_test_io_pair *io_pair)
     /* Close client read fd to mock half closed pipe at server side */
     s2n_io_pair_shutdown_one_end(io_pair, S2N_CLIENT, SHUT_RD);
 #endif
-    /* Give server a chance to send data on broken pipe */
-    sleep(2);
+
+    /* Signal the parent: the read end of the socketpair is now shut down,
+     * so the parent's next s2n_send is guaranteed to observe EPIPE. */
+    uint8_t ready = 1;
+    if (write(ready_pipe_w, &ready, 1) != 1) {
+        exit(1);
+    }
+    close(ready_pipe_w);
+
+    /* Wait for the parent to finish its broken-pipe assertions before
+     * continuing teardown. Without this, the child could race ahead and
+     * close the socket entirely while the parent is mid-assertion. */
+    uint8_t done = 0;
+    if (read(done_pipe_r, &done, 1) != 1) {
+        exit(1);
+    }
+    close(done_pipe_r);
 
     s2n_shutdown(conn, &blocked);
 
@@ -79,8 +95,9 @@ void mock_client(struct s2n_test_io_pair *io_pair)
         exit(1);
     }
 
-    /* Give the server a chance to avoid a sigpipe */
-    sleep(1);
+    /* The parent has SIGPIPE ignored (via s2n_io_pair_init), and the child
+     * inherits that signal disposition through fork(), so there's no need to
+     * sleep before tearing down the other end of the socketpair. */
 
     s2n_io_pair_shutdown_one_end(io_pair, S2N_CLIENT, SHUT_WR);
     s2n_io_pair_close_one_end(io_pair, S2N_CLIENT);
@@ -107,16 +124,35 @@ int main(int argc, char **argv)
         DEFER_CLEANUP(struct s2n_test_io_pair io_pair = { 0 }, s2n_io_pair_close);
         EXPECT_SUCCESS(s2n_io_pair_init(&io_pair));
 
+        /* Create control pipes for child<->parent synchronization. The child
+         * writes to ready_pipe after shutting down its read side, so the
+         * parent can deterministically observe EPIPE on its next s2n_send.
+         * The parent writes to done_pipe after completing its assertions, so
+         * the child knows when it's safe to tear down the socket. This
+         * replaces two timing-dependent sleep() calls. */
+        int ready_pipe[2] = { -1, -1 };
+        int done_pipe[2] = { -1, -1 };
+        EXPECT_EQUAL(pipe(ready_pipe), 0);
+        EXPECT_EQUAL(pipe(done_pipe), 0);
+
         /* Create a child process */
         pid_t pid = fork();
         if (pid == 0) {
             /* This is the client process, close the server end of the pipe */
             EXPECT_SUCCESS(s2n_io_pair_close_one_end(&io_pair, S2N_SERVER));
 
+            /* Close the pipe ends the child doesn't use */
+            close(ready_pipe[0]);
+            close(done_pipe[1]);
+
             /* Write the fragmented hello message */
-            mock_client(&io_pair);
+            mock_client(&io_pair, ready_pipe[1], done_pipe[0]);
         }
 
+        /* Close the pipe ends the parent doesn't use */
+        close(ready_pipe[1]);
+        close(done_pipe[0]);
+
         /* This is the server process, close the client end of the pipe */
         EXPECT_SUCCESS(s2n_io_pair_close_one_end(&io_pair, S2N_CLIENT));
 
@@ -146,8 +182,12 @@ int main(int argc, char **argv)
         EXPECT_SUCCESS(s2n_negotiate(conn, &blocked));
         EXPECT_EQUAL(conn->actual_protocol_version, S2N_TLS12);
 
-        /* Give client a chance to close pipe at the receiving end */
-        sleep(1);
+        /* Wait for the child to shut down its read side of the socketpair.
+         * Once this read returns, the next s2n_send on the server conn is
+         * guaranteed to observe EPIPE. */
+        uint8_t ready = 0;
+        EXPECT_EQUAL(read(ready_pipe[0], &ready, 1), 1);
+        EXPECT_SUCCESS(close(ready_pipe[0]));
         char buffer[1];
         /* Fist flush on half closed pipe should get EPIPE */
         ssize_t w = s2n_send(conn, buffer, 1, &blocked);
@@ -161,6 +201,12 @@ int main(int argc, char **argv)
         EXPECT_EQUAL(s2n_errno, S2N_ERR_IO);
         EXPECT_EQUAL(errno, 0);
 
+        /* Signal the child that the assertions are complete so it can tear
+         * down its socket and exit. */
+        uint8_t done = 1;
+        EXPECT_EQUAL(write(done_pipe[1], &done, 1), 1);
+        EXPECT_SUCCESS(close(done_pipe[1]));
+
         EXPECT_SUCCESS(s2n_connection_free(conn));
         for (int cert = 0; cert < SUPPORTED_CERTIFICATE_FORMATS; cert++) {
             EXPECT_SUCCESS(s2n_cert_chain_and_key_free(chain_and_keys[cert]));

tests/unit/s2n_self_talk_client_hello_cb_test.c

@@ -48,8 +48,9 @@ int mock_client(struct s2n_test_io_pair *io_pair, int expect_failure, int expect
     int rc = 0;
     const char *protocols[] = { "h2", "http/1.1" };
 
-    /* Give the server a chance to listen */
-    sleep(1);
+    /* The socketpair underlying io_pair is valid from the moment of fork, so
+     * the child can start the handshake immediately. The kernel will buffer
+     * writes until the parent starts reading. No pre-negotiate sleep needed. */
 
     conn = s2n_connection_new(S2N_CLIENT);
     config = s2n_config_new();
@@ -96,8 +97,9 @@ int mock_client(struct s2n_test_io_pair *io_pair, int expect_failure, int expect
     s2n_connection_free(conn);
     s2n_config_free(config);
 
-    /* Give the server a chance to a void a sigpipe */
-    sleep(1);
+    /* SIGPIPE is handled at the test's signal level (or ignored via
+     * SIGPIPE policy), so no post-shutdown sleep is needed to avoid a race
+     * with the server trying to send while we tear down. */
 
     s2n_cleanup();
     s2n_io_pair_close_one_end(io_pair, S2N_CLIENT);

tests/unit/s2n_self_talk_nonblocking_test.c

@@ -44,8 +44,9 @@ int mock_client(struct s2n_test_io_pair *io_pair, uint8_t *expected_data, uint32
      * CI/CD pipelines might never complete */
     int should_block = 1;
 
-    /* Give the server a chance to listen */
-    sleep(1);
+    /* The socketpair underlying io_pair is valid from the moment of fork,
+     * so the child can start the handshake immediately. The kernel buffers
+     * writes until the parent is ready to read. */
 
     client_conn = s2n_connection_new(S2N_CLIENT);
     client_config = s2n_config_new();
@@ -110,8 +111,9 @@ int mock_client_iov(struct s2n_test_io_pair *io_pair, struct iovec *iov, uint32_
     uint8_t *buffer = malloc(total_size + iov[0].iov_len);
     int buffer_offs = 0;
 
-    /* Give the server a chance to listen */
-    sleep(1);
+    /* The socketpair underlying io_pair is valid from the moment of fork,
+     * so the child can start the handshake immediately. The kernel buffers
+     * writes until the parent is ready to read. */
 
     client_conn = s2n_connection_new(S2N_CLIENT);
     client_config = s2n_config_new();
@@ -336,8 +338,14 @@ int test_send(int use_tls13, int use_iov, int prefer_throughput)
     EXPECT_TRUE(remaining < data_size);
     EXPECT_TRUE(remaining > 0);
 
-    /* Wait for the child process to read some bytes and block itself*/
-    sleep(1);
+    /* Wait for the child to actually be stopped before sending SIGCONT.
+     * WUNTRACED causes waitpid to return when the child changes state to
+     * stopped, not just when it exits. This removes the timing race that
+     * would otherwise require a sleep() here. */
+    int stop_status = 0;
+    pid_t wait_rc = waitpid(pid, &stop_status, WUNTRACED);
+    EXPECT_EQUAL(wait_rc, pid);
+    EXPECT_TRUE(WIFSTOPPED(stop_status));
     /* Wake the child process by sending it SIGCONT */
     EXPECT_SUCCESS(kill(pid, SIGCONT));
 

tests/unit/s2n_self_talk_session_id_test.c

@@ -147,8 +147,8 @@ void mock_client(struct s2n_test_io_pair *io_pair)
     s2n_blocked_status blocked;
     int result = 0;
 
-    /* Give the server a chance to listen */
-    sleep(1);
+    /* The socketpair is valid from fork; the kernel buffers writes until the
+     * parent reads, so no pre-negotiate sleep is needed. */
 
     /* Initial handshake */
     conn = s2n_connection_new(S2N_CLIENT);
@@ -208,8 +208,8 @@ void mock_client(struct s2n_test_io_pair *io_pair)
 
     s2n_connection_free(conn);
 
-    /* Give the server a chance to avoid sigpipe */
-    sleep(1);
+    /* SIGPIPE is inherited as SIG_IGN across fork via s2n_io_pair_init, so
+     * no pre-cleanup sleep is needed. */
 
     /* Session resumption */
     conn = s2n_connection_new(S2N_CLIENT);
@@ -241,8 +241,8 @@ void mock_client(struct s2n_test_io_pair *io_pair)
 
     s2n_connection_free(conn);
 
-    /* Give the server a chance to avoid sigpipe */
-    sleep(1);
+    /* SIGPIPE is inherited as SIG_IGN across fork via s2n_io_pair_init, so
+     * no pre-cleanup sleep is needed. */
 
     /* Session resumption with bad session state */
     conn = s2n_connection_new(S2N_CLIENT);
@@ -282,8 +282,8 @@ void mock_client(struct s2n_test_io_pair *io_pair)
     s2n_connection_free(conn);
     s2n_config_free(config);
 
-    /* Give the server a chance to avoid sigpipe */
-    sleep(1);
+    /* SIGPIPE is inherited as SIG_IGN across fork via s2n_io_pair_init, so
+     * no pre-cleanup sleep is needed. */
 
     s2n_io_pair_close_one_end(io_pair, S2N_CLIENT);
     exit(result);

tests/unit/s2n_self_talk_tls12_test.c

@@ -36,8 +36,8 @@ void mock_client(struct s2n_test_io_pair *io_pair)
     struct s2n_config *config = NULL;
     s2n_blocked_status blocked;
 
-    /* Give the server a chance to listen */
-    sleep(1);
+    /* The socketpair is valid from fork; the kernel buffers writes until the
+     * parent reads, so no pre-negotiate sleep is needed. */
 
     conn = s2n_connection_new(S2N_CLIENT);
     config = s2n_config_new();
@@ -89,8 +89,8 @@ void mock_client(struct s2n_test_io_pair *io_pair)
     s2n_connection_free(conn);
     s2n_config_free(config);
 
-    /* Give the server a chance to a void a sigpipe */
-    sleep(1);
+    /* SIGPIPE is inherited as SIG_IGN across fork via s2n_io_pair_init, so
+     * no pre-cleanup sleep is needed. */
 
     s2n_io_pair_close_one_end(io_pair, S2N_CLIENT);
 

tests/unit/s2n_self_talk_tls13_test.c

@@ -32,8 +32,8 @@ void mock_client(struct s2n_test_io_pair *io_pair)
     struct s2n_config *config = NULL;
     s2n_blocked_status blocked;
 
-    /* Give the server a chance to listen */
-    sleep(1);
+    /* The socketpair is valid from fork; the kernel buffers writes until the
+     * parent reads, so no pre-negotiate sleep is needed. */
 
     conn = s2n_connection_new(S2N_CLIENT);
     config = s2n_config_new();
@@ -86,8 +86,8 @@ void mock_client(struct s2n_test_io_pair *io_pair)
     s2n_connection_free(conn);
     s2n_config_free(config);
 
-    /* Give the server a chance to avoid a sigpipe */
-    sleep(1);
+    /* SIGPIPE is inherited as SIG_IGN across fork via s2n_io_pair_init, so
+     * no pre-cleanup sleep is needed. */
 
     s2n_io_pair_close_one_end(io_pair, S2N_CLIENT);