From 8fa49d392f42a332592c5d01c7b4de24821191e1 Mon Sep 17 00:00:00 2001
From: Tokuhiro Matsuno <tokuhirom@gmail.com>
Date: Thu, 10 Sep 2026 08:11:05 +0900
Subject: [PATCH] Fix race condition in http2-request-parser test harness

The per-request checks run in a `start` block, off the main thread, and
Test is not thread safe. With multiplexing tests (`$count == 2`) two of
those blocks emit `ok` concurrently, so the TAP output interleaved.

Worse, `$test-completed.keep if $current-counter + 1 == $count` only
waited for the *last* request's checks, so an earlier request's `ok`
could still be in flight and leak out past the `pass $desc` that should
have followed it -- shifting the test numbering and intermittently
failing the final `throws-like` subtest.

Record the check results in a Promise per request instead, and report
them with `ok` on the main thread, in request order, once every request
has been checked.

Measured over 26 parallel runs of the file: 8/26 runs produced identical
output before, 23/26 after (the remaining 3 hit the pre-existing 5s
deadline under CPU contention).

Signed-off-by: Tokuhiro Matsuno <tokuhirom@gmail.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 t/http2-request-parser.rakutest | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/t/http2-request-parser.rakutest b/t/http2-request-parser.rakutest
index 47b27a77..f783cfc6 100644
--- a/t/http2-request-parser.rakutest
+++ b/t/http2-request-parser.rakutest
@@ -21,14 +21,22 @@ sub test(@frames, $count, $desc, @checks, :$fail, :$test-supplies) {
     my $parser = Cro::HTTP2::RequestParser.new;
     my $fake-in = Supplier.new;
     my $counter = 0;
+    # The checks run off the main thread, and Test is not thread safe: emitting
+    # `ok` from there interleaves the TAP output of concurrent requests, and can
+    # even leak a test past the `pass`/`done-testing` that should follow it. So
+    # each request only records its check results here; they are reported below,
+    # on the main thread, in request order.
+    my @check-results = Promise.new xx $count;
+    my $all-checks-recorded = Promise.allof(|@check-results);
     $parser.transformer($fake-in.Supply, :$connection-state).tap:
             -> $request {
                 my $current-counter = $counter++;
                 start {
-                    for @checks[$current-counter].kv -> $i, $check {
-                        ok $check($request), "check {$i + 1}";
+                    my @results;
+                    for @checks[$current-counter].list -> $check {
+                        @results.push($check($request));
                     }
-                    $test-completed.keep if $current-counter + 1 == $count;
+                    @check-results[$current-counter].keep(@results);
                     CATCH {
                         default {
                             $test-completed.break($_);
@@ -45,9 +53,16 @@ sub test(@frames, $count, $desc, @checks, :$fail, :$test-supplies) {
         }
         $fake-in.done;
     }
-    await Promise.anyof($test-completed, Promise.in(5));
+    await Promise.anyof($test-completed, $all-checks-recorded, Promise.in(5));
     if $test-completed.status ~~ Kept {
         pass $desc;
+    } elsif $all-checks-recorded.status ~~ Kept {
+        for @check-results -> $recorded {
+            for $recorded.result.kv -> $i, $result {
+                ok $result, "check {$i + 1}";
+            }
+        }
+        pass $desc;
     } else {
         die X::Cro::HTTP2::Error.new(code => PROTOCOL_ERROR) if $fail;
         flunk $desc unless $test-supplies;