From b1bfeee37bf28a69b04bc92e46a9cd9069bba63a Mon Sep 17 00:00:00 2001
From: Shizuo Fujita <fujita@clear-code.com>
Date: Fri, 31 Jul 2026 17:40:25 +0900
Subject: [PATCH] Fix flaky tcp_server specs under valgrind

test_run and test_run_timeout send data, sleep TIMEOUT (10ms), then tear the
server down and assert on what arrived. That assumes the reactor accepts the
connection and dispatches on_read within those 10ms. Under valgrind it often
does not, and the example fails with `expected: "hello", got: nil`.

Seen on the Memcheck workflow, job "4.0 on ubuntu":
https://github.com/socketry/cool.io/actions/runs/30615723672/job/91108332098
The other four matrix jobs passed on the same commit.

Wait for the data instead of for the clock, under Timeout.timeout so the wait
can never become an indefinite sleep. Five seconds is generous enough that a
slow machine still passes, and a genuine hang now raises Timeout::Error at the
wait itself rather than surfacing later as a confusing nil comparison. Both
helpers also reset @data first, so the wait condition cannot be satisfied by a
value left over from an earlier example.

Only these two helpers are affected. test_run_once already blocks on
thread.join, and the two run_once(timeout) examples do not assert on @data.

Verified by shrinking TIMEOUT to 0.0001 to stand in for valgrind's slowdown,
which reproduces the CI failure locally:

  before: 6 runs -> 4 failed (./spec/tcp_server_spec.rb:127)
  after: 10 runs -> 0 failed

The timeout path was checked separately with a condition that never becomes
true, which raises Timeout::Error at the deadline instead of hanging.

rspec: 58 examples, 1 failure (detach_race_condition_spec.rb:35), a pre-existing
Windows failure unrelated to this change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 spec/tcp_server_spec.rb | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/spec/tcp_server_spec.rb b/spec/tcp_server_spec.rb
index b78b874..6790980 100644
--- a/spec/tcp_server_spec.rb
+++ b/spec/tcp_server_spec.rb
@@ -1,4 +1,5 @@
 require File.expand_path('../spec_helper', __FILE__)
+require 'timeout'
 
 TIMEOUT = 0.010
 HOST = '127.0.0.1'
@@ -39,13 +40,31 @@ def on_message(data)
   @data = data
 end
 
+# The reactor can be an order of magnitude slower under valgrind, so sleeping a
+# fixed interval and assuming the event was processed by then makes these specs
+# fail intermittently. Wait for the result instead. The timeout is generous
+# enough that a slow machine still passes, and a genuine hang raises
+# Timeout::Error here instead of blocking the suite forever.
+WAIT_TIMEOUT = 5.0
+
+def wait_until(timeout = WAIT_TIMEOUT)
+  Timeout.timeout(timeout) do
+    sleep 0.001 until yield
+  end
+end
+
 def test_run(data = nil)
+  @data = ""
   reactor = Coolio::Loop.new
   server = Cool.io::TCPServer.new(HOST, PORT, MyConnection, method(:on_message))
   reactor.attach(server)
   thread = Thread.new { reactor.run }
-  send_data(data) if data
-  sleep TIMEOUT
+  if data
+    send_data(data)
+    wait_until { @data == data }
+  else
+    sleep TIMEOUT
+  end
   reactor.stop
   server.detach
   send_data('') # to leave from blocking loop
@@ -86,6 +105,7 @@ def test_run_once_timeout(timeout = TIMEOUT)
 end
 
 def test_run_timeout(data = nil, timeout = TIMEOUT)
+  @data = ""
   reactor = Coolio::Loop.new
   server = Cool.io::TCPServer.new(HOST, PORT, MyConnection, method(:on_message))
   reactor.attach(server)
@@ -95,8 +115,12 @@ def test_run_timeout(data = nil, timeout = TIMEOUT)
       reactor.run_once(timeout)
     end
   end
-  send_data(data) if data
-  sleep timeout
+  if data
+    send_data(data)
+    wait_until { @data == data }
+  else
+    sleep timeout
+  end
   server.detach
   running = false # another send is not required
   thread.join