Skip to content

Commit

Permalink
channels: Don't rely on tight timing constraints (JuliaLang#49090)
Browse files Browse the repository at this point in the history
The test was relying on two particular non-guaranteed timing races:

1. That the 0.5s wait task was started within 0.5s of the timedwait
   starting.
2. That the timedwait finished within 0.5 seconds of its deadline.

Neither of these are guaranteed. The second of these was already
made non-fatal with a warning print. However, I don't like warning
prints in tests. They should either pass or fail. Nobody looks at
the output unless the tests fail, so change the tests to:

1. Add extra synchronization to solve the race condition (1) by
making sure the sleep starts before the timedwait.
2. Instead of doing a long wait, just have a synchronization channel
that will never be made active.

In particular, the thing that this test checks is that the timeout
in `timedwait` actually works, and doesn't return too early. This
arrangement checks both of those properties without unduly tight
timing constraints.
  • Loading branch information
Keno authored Mar 24, 2023
1 parent f78b3a6 commit 1b7f98a
Showing 1 changed file with 7 additions and 9 deletions.
16 changes: 7 additions & 9 deletions test/channels.jl
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ end

@testset "timedwait on multiple channels" begin
Experimental.@sync begin
sync = Channel(1)
rr1 = Channel(1)
rr2 = Channel(1)
rr3 = Channel(1)
Expand All @@ -320,20 +321,17 @@ end
@test !callback()
@test timedwait(callback, 0) === :timed_out

@async begin sleep(0.5); put!(rr1, :ok) end
@async begin put!(sync, :ready); sleep(0.5); put!(rr1, :ok) end
@async begin sleep(1.0); put!(rr2, :ok) end
@async begin sleep(2.0); put!(rr3, :ok) end
@async begin @test take!(rr3) == :done end

@test take!(sync) == :ready
et = @elapsed timedwait(callback, 1)

# assuming that 0.5 seconds is a good enough buffer on a typical modern CPU
try
@assert (et >= 1.0) && (et <= 1.5)
@assert !isready(rr3)
catch
@warn "`timedwait` tests delayed. et=$et, isready(rr3)=$(isready(rr3))"
end
@test et >= 1.0

@test isready(rr1)
put!(rr3, :done)
end
end

Expand Down

0 comments on commit 1b7f98a

Please sign in to comment.