Skip to content

Commit 6e055f5

Browse files
authored
Fix: Avoid hangs in fast_fallback tests under scheduling jitter (ruby#17889)
Some fast_fallback tests inject an artificial delay to test timing-sensitive behavior. Under CI's parallel test execution, threadscheduling jitter can make the delay longer than expected, and in that case these tests may fail to connect to the server process they are actually supposed to connect to. For example: 1. The IPv6 name resolution is artificially delayed by 25ms, so the IPv4 name resolution succeeds first. 2. If the IPv6 name resolution succeeds within 25ms, the client starts connecting to the `::1` server process and the test passes. However, if the IPv6 name resolution is delayed by more than 50ms for some reason, fast_fallback starts connecting to `127.0.0.1` instead, by design. 3. If another test happens to have a server listening on `127.0.0.1` with the same port number, the connection accidentally succeeds. 4. The `::1` server process is left waiting, and the thread cleanup hangs. To address this, the listening server process now receives the result of `accept` through a local variable instead of `Thread#value`. On success, it joins the thread with a short timeout; otherwise, it terminates the thread with `Thread#kill.join`. This change is expected to prevent hangs even when such a wrong connection occurs.
1 parent c7c0483 commit 6e055f5

1 file changed

Lines changed: 36 additions & 9 deletions

File tree

test/socket/test_tcp.rb

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -230,16 +230,22 @@ def test_initialize_v6_hostname_resolved_in_resolution_delay
230230
port = server.addr[1]
231231
delay_time = 25 # Socket::RESOLUTION_DELAY (private) is 50ms
232232

233-
server_thread = Thread.new { server.accept }
233+
accepted = nil
234+
server_thread = Thread.new { accepted = server.accept }
234235
socket = TCPSocket.new(
235236
"localhost",
236237
port,
237238
fast_fallback: true,
238239
test_mode_settings: { delay: { ipv6: delay_time } }
239240
)
240-
assert_true(socket.remote_address.ipv6?)
241+
# Another process may be listening on the same port on 127.0.0.1.
242+
omit "connected to an unrelated process on the same port" unless socket.remote_address.ipv6?
243+
server_thread.join(3)
244+
assert_not_nil(accepted, "server did not accept the connection")
245+
assert_equal(socket.local_address.to_sockaddr, accepted.remote_address.to_sockaddr)
241246
ensure
242-
server_thread&.value&.close
247+
stop_accept_thread(server_thread, socket)
248+
accepted&.close
243249
server&.close
244250
socket&.close
245251
end
@@ -252,16 +258,21 @@ def test_initialize_v6_hostname_resolved_earlier_and_v6_server_is_not_listening
252258
server.bind(Socket.pack_sockaddr_in(0, ipv4_address))
253259
port = server.connect_address.ip_port
254260

255-
server_thread = Thread.new { server.listen(1); server.accept }
261+
accepted = nil
262+
server_thread = Thread.new { server.listen(1); accepted, _ = server.accept }
256263
socket = TCPSocket.new(
257264
"localhost",
258265
port,
259266
fast_fallback: true,
260267
test_mode_settings: { delay: { ipv4: 10 } }
261268
)
262-
assert_equal(ipv4_address, socket.remote_address.ip_address)
269+
# Another process may be listening on the same port on ::1.
270+
omit "connected to an unrelated process on the same port" if socket.remote_address.ipv6?
271+
server_thread.join(3)
272+
assert_not_nil(accepted, "server did not accept the connection")
273+
assert_equal(socket.local_address.to_sockaddr, accepted.remote_address.to_sockaddr)
263274
ensure
264-
accepted, _ = server_thread&.value
275+
stop_accept_thread(server_thread, socket)
265276
accepted&.close
266277
server&.close
267278
socket&.close
@@ -274,16 +285,21 @@ def test_initialize_v6_hostname_resolved_later_and_v6_server_is_not_listening
274285
server.bind(Socket.pack_sockaddr_in(0, "127.0.0.1"))
275286
port = server.connect_address.ip_port
276287

277-
server_thread = Thread.new { server.listen(1); server.accept }
288+
accepted = nil
289+
server_thread = Thread.new { server.listen(1); accepted, _ = server.accept }
278290
socket = TCPSocket.new(
279291
"localhost",
280292
port,
281293
fast_fallback: true,
282294
test_mode_settings: { delay: { ipv6: 25 } }
283295
)
284-
assert_true(socket.remote_address.ipv4?)
296+
# Another process may be listening on the same port on ::1.
297+
omit "connected to an unrelated process on the same port" if socket.remote_address.ipv6?
298+
server_thread.join(3)
299+
assert_not_nil(accepted, "server did not accept the connection")
300+
assert_equal(socket.local_address.to_sockaddr, accepted.remote_address.to_sockaddr)
285301
ensure
286-
accepted, _ = server_thread&.value
302+
stop_accept_thread(server_thread, socket)
287303
accepted&.close
288304
server&.close
289305
socket&.close
@@ -420,4 +436,15 @@ def test_initialize_fast_fallback_is_false
420436
server&.close
421437
socket&.close
422438
end
439+
440+
private
441+
442+
# On success, wait for the accept thread to finish. If the test failed
443+
# before the client connected, `server.accept` never returns, so kill
444+
# the thread instead of waiting for it.
445+
def stop_accept_thread(server_thread, socket)
446+
return unless server_thread
447+
server_thread.join(1) if socket
448+
server_thread.kill.join
449+
end
423450
end if defined?(TCPSocket)

0 commit comments

Comments
 (0)