Skip to content

Commit bf44f8c

Browse files
authored
Make test synchronization deterministic (#1346)
* test(ci): remove wall-clock synchronization Replace sleeps, polling deadlines, and elapsed-time assertions with lifecycle signals, exact protocol reads, and expired deadlines. Add a policy test and contributor guidance to keep the suite deterministic across variable CI runner load. * test(ci): restore deterministic wait coverage Add lifecycle barriers and private clock and wait seams so deterministic tests still execute active wait, wake, handoff, and shutdown paths. Restore project coverage without reintroducing elapsed-time coordination.
1 parent 4b6c835 commit bf44f8c

33 files changed

Lines changed: 791 additions & 1489 deletions

src/http2_client.jl

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -310,16 +310,22 @@ end
310310
end
311311
end
312312

313-
function _wait_h2_send_window_locked!(conn::H2Connection, stream_id::UInt32, deadline_ns::Int64)::Nothing
313+
function _wait_h2_send_window_locked!(
314+
conn::H2Connection,
315+
stream_id::UInt32,
316+
deadline_ns::Int64;
317+
clock_ns::Function=time_ns,
318+
wait_for::Function=IOPoll.timedwait,
319+
)::Nothing
314320
if deadline_ns == 0
315321
wait(conn.window_condition)
316322
return nothing
317323
end
318-
remaining_ns = deadline_ns - Int64(time_ns())
324+
remaining_ns = deadline_ns - Int64(clock_ns())
319325
remaining_ns <= 0 && throw(IOPoll.DeadlineExceededError())
320326
unlock(conn.state_lock)
321327
try
322-
status = IOPoll.timedwait(() -> begin
328+
status = wait_for(() -> begin
323329
lock(conn.state_lock)
324330
try
325331
return _h2_send_window_ready_locked(conn, stream_id)

src/http2_server.jl

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -310,16 +310,21 @@ function _apply_h2_window_update!(send_state::_H2SendWindowState, frame::WindowU
310310
return nothing
311311
end
312312

313-
function _wait_h2_send_window_locked!(send_state::_H2SendWindowState, deadline_ns::Int64)::Nothing
313+
function _wait_h2_send_window_locked!(
314+
send_state::_H2SendWindowState,
315+
deadline_ns::Int64;
316+
clock_ns::Function=time_ns,
317+
wait_ns::Function=IOPoll.sleep_ns,
318+
)::Nothing
314319
if deadline_ns == 0
315320
wait(send_state.window_condition)
316321
return nothing
317322
end
318-
remaining_ns = deadline_ns - Int64(time_ns())
323+
remaining_ns = deadline_ns - Int64(clock_ns())
319324
remaining_ns <= 0 && throw(IOPoll.DeadlineExceededError())
320325
unlock(send_state.state_lock)
321326
try
322-
IOPoll.sleep_ns(min(remaining_ns, Int64(1_000_000)))
327+
wait_ns(min(remaining_ns, Int64(1_000_000)))
323328
finally
324329
lock(send_state.state_lock)
325330
end

src/http_client_timeouts.jl

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,11 @@ function _apply_request_timeout_settings!(
7777
ctx::RequestContext,
7878
request_timeout_ns::Int64,
7979
config::Union{Nothing,_RequestTimeoutConfig},
80+
;
81+
now_ns::Int64=Int64(time_ns()),
8082
)::RequestContext
8183
request_timeout_ns < 0 && throw(ArgumentError("request_timeout_ns must be >= 0"))
8284
if request_timeout_ns > 0
83-
now_ns = Int64(time_ns())
8485
deadline_ns = now_ns > typemax(Int64) - request_timeout_ns ? typemax(Int64) : now_ns + request_timeout_ns
8586
set_deadline!(ctx, deadline_ns)
8687
end

src/http_retry.jl

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,10 @@ function _retry_after_delay_ns(headers::Headers)::Union{Nothing,Int64}
174174
return _parse_retry_after_delay_ns(value::String)
175175
end
176176

177-
function _parse_retry_after_delay_ns(value::AbstractString)::Union{Nothing,Int64}
177+
function _parse_retry_after_delay_ns(
178+
value::AbstractString;
179+
now::Dates.DateTime=Dates.now(Dates.UTC),
180+
)::Union{Nothing,Int64}
178181
stripped = strip(String(value))
179182
isempty(stripped) && return nothing
180183
parsed_secs = try
@@ -190,7 +193,7 @@ function _parse_retry_after_delay_ns(value::AbstractString)::Union{Nothing,Int64
190193
end
191194
parsed_dt = Cookies._parse_http_gmt_datetime(stripped)
192195
parsed_dt === nothing && return nothing
193-
delta = parsed_dt::Dates.DateTime - Dates.now(Dates.UTC)
196+
delta = parsed_dt::Dates.DateTime - now
194197
millis = Dates.value(delta)
195198
millis <= 0 && return Int64(0)
196199
millis > typemax(Int64) ÷ 1_000_000 && return typemax(Int64)

src/http_transport.jl

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ mutable struct Transport
217217
max_conns_per_host::Int
218218
idle_timeout_ns::Int64
219219
lock::ReentrantLock
220+
waiter_condition::Threads.Condition
220221
idle::Dict{String,Vector{Conn}}
221222
waiters::Dict{String,Vector{_ConnWaiter}}
222223
conns_per_host::Dict{String,Int}
@@ -280,6 +281,7 @@ function Transport(;
280281
max_conns_per_host >= 0 || throw(ArgumentError("max_conns_per_host must be >= 0"))
281282
idle_timeout_ns >= 0 || throw(ArgumentError("idle_timeout_ns must be >= 0"))
282283
host_resolver = HostResolvers.HostResolver(local_addr=_normalize_local_addr(local_addr))
284+
lock = ReentrantLock()
283285
return Transport(
284286
host_resolver,
285287
tls_config,
@@ -289,7 +291,8 @@ function Transport(;
289291
Int(max_idle_total),
290292
Int(max_conns_per_host),
291293
Int64(idle_timeout_ns),
292-
ReentrantLock(),
294+
lock,
295+
Threads.Condition(lock),
293296
Dict{String,Vector{Conn}}(),
294297
Dict{String,Vector{_ConnWaiter}}(),
295298
Dict{String,Int}(),
@@ -541,6 +544,7 @@ function _enqueue_waiter_locked!(transport::Transport, waiter::_ConnWaiter)
541544
queue = get(() -> _ConnWaiter[], transport.waiters, waiter.key)
542545
push!(queue, waiter)
543546
transport.waiters[waiter.key] = queue
547+
notify(transport.waiter_condition; all=true)
544548
return waiter
545549
end
546550

@@ -643,7 +647,13 @@ function _deliver_waiter_error_locked!(waiter::_ConnWaiter, err::Exception)::Boo
643647
return true
644648
end
645649

646-
function _wait_for_conn!(transport::Transport, waiter::_ConnWaiter, deadline_ns::Int64)
650+
function _wait_for_conn!(
651+
transport::Transport,
652+
waiter::_ConnWaiter,
653+
deadline_ns::Int64;
654+
clock_ns::Function=time_ns,
655+
wait_for::Function=IOPoll.timedwait,
656+
)
647657
while true
648658
state = @atomic :acquire waiter.state
649659
if state == _CONN_WAITER_CONN
@@ -659,7 +669,7 @@ function _wait_for_conn!(transport::Transport, waiter::_ConnWaiter, deadline_ns:
659669
wait(waiter.signal)
660670
continue
661671
end
662-
now_ns = Int64(time_ns())
672+
now_ns = Int64(clock_ns())
663673
if now_ns >= deadline_ns
664674
lock(transport.lock)
665675
try
@@ -674,7 +684,7 @@ function _wait_for_conn!(transport::Transport, waiter::_ConnWaiter, deadline_ns:
674684
continue
675685
end
676686
timeout_s = min((deadline_ns - now_ns) / 1.0e9, 0.05)
677-
IOPoll.timedwait(() -> (@atomic :acquire waiter.state) != _CONN_WAITER_WAITING, timeout_s; pollint=0.001)
687+
wait_for(() -> (@atomic :acquire waiter.state) != _CONN_WAITER_WAITING, timeout_s; pollint=0.001)
678688
end
679689
end
680690

test/README.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
# Deterministic test synchronization
2+
3+
HTTP.jl tests must not depend on scheduler speed or elapsed wall-clock time.
4+
GitHub Actions runners can pause a task for an unknown period. A delay that is
5+
safe on one runner can fail on another runner without a product defect.
6+
7+
Use observable state transitions instead:
8+
9+
- Use a `Channel`, `Base.Event`, or `Threads.Condition` for task handshakes.
10+
- Read exact byte counts, complete protocol frames, markers, or EOF.
11+
- Use `fetch(task)` or `wait(task)` for task completion. Wrap unexpected
12+
`Threads.@spawn` failures with `errormonitor`.
13+
- Inject a fixed clock value into pure deadline calculations.
14+
- Use an already-expired absolute deadline when a test must enter a product
15+
timeout branch. Do not wait for a future deadline to expire.
16+
- Mutate private lifecycle state only when the test directly covers that state,
17+
such as an idle-pool eviction test.
18+
19+
Do not use `sleep`, `timedwait`, `time`, `time_ns`, `Timer`, elapsed-time
20+
assertions, polling intervals, or helper-level timeout arguments in test code.
21+
Do not use a short delay to prove that an event has not occurred. Build a
22+
barrier that makes the event impossible until the test releases it.
23+
24+
Product timeout configuration remains valid test input. It tests parsing,
25+
propagation, and expired-deadline behavior. It must not act as the test harness.
26+
The GitHub Actions job timeout remains the final guard for a true deadlock.

test/http1_wire_tests.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ end
267267
# _ConnReader pulls it into one buffer fill -> later lines are served
268268
# from the buffered fast path.
269269
write(client, bytes)
270-
Reseau.IOPoll.timedwait(() -> server_conn[] !== nothing, 5.0; pollint = 0.001)
270+
fetch(t)
271271
return HT._ConnReader(server_conn[]::Reseau.TCP.Conn), client, listener
272272
catch
273273
HT.@try_ignore close(listener)
@@ -326,7 +326,7 @@ end
326326
t = Task(() -> (server_conn[] = Reseau.TCP.accept(listener)))
327327
schedule(t)
328328
client = Reseau.TCP.connect(Reseau.TCP.loopback_addr(Int(addr.port)))
329-
Reseau.IOPoll.timedwait(() -> server_conn[] !== nothing, 5.0; pollint = 0.001)
329+
fetch(t)
330330
for c in chunks
331331
write(client, c)
332332
end

0 commit comments

Comments
 (0)