Skip to content

Commit

Permalink
add missing wait during Timer and AsyncCondition close (#51847)
Browse files Browse the repository at this point in the history
Can cause spurious warnings about not closing these properly and
unexpected events to appear after `close` returns.
  • Loading branch information
vtjnash authored Oct 25, 2023
1 parent c54a3f2 commit d0c4284
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 10 deletions.
39 changes: 32 additions & 7 deletions base/asyncevent.jl
Original file line number Diff line number Diff line change
Expand Up @@ -118,22 +118,26 @@ end
unsafe_convert(::Type{Ptr{Cvoid}}, t::Timer) = t.handle
unsafe_convert(::Type{Ptr{Cvoid}}, async::AsyncCondition) = async.handle

# if this returns true, the object has been signaled
# if this returns false, the object is closed
function _trywait(t::Union{Timer, AsyncCondition})
set = t.set
if set
# full barrier now for AsyncCondition
t isa Timer || Core.Intrinsics.atomic_fence(:acquire_release)
else
t.isopen || return false
t.handle == C_NULL && return false
if !isopen(t)
close(t) # wait for the close to complete
return false
end
iolock_begin()
set = t.set
if !set
preserve_handle(t)
lock(t.cond)
try
set = t.set
if !set && t.isopen && t.handle != C_NULL
if !set && t.handle != C_NULL # wait for set or handle, but not the isopen flag
iolock_end()
set = wait(t.cond)
unlock(t.cond)
Expand All @@ -160,10 +164,28 @@ end
isopen(t::Union{Timer, AsyncCondition}) = t.isopen && t.handle != C_NULL

function close(t::Union{Timer, AsyncCondition})
t.handle == C_NULL && return # short-circuit path
iolock_begin()
if isopen(t)
@atomic :monotonic t.isopen = false
ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), t)
if t.handle != C_NULL
if t.isopen
@atomic :monotonic t.isopen = false
ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), t)
end
# implement _trywait here without the auto-reset function, just waiting for the final close signal
preserve_handle(t)
lock(t.cond)
try
while t.handle != C_NULL
iolock_end()
wait(t.cond)
unlock(t.cond)
iolock_begin()
lock(t.cond)
end
finally
unlock(t.cond)
unpreserve_handle(t)
end
end
iolock_end()
nothing
Expand Down Expand Up @@ -220,7 +242,10 @@ function uv_timercb(handle::Ptr{Cvoid})
@atomic :monotonic t.set = true
if ccall(:uv_timer_get_repeat, UInt64, (Ptr{Cvoid},), t) == 0
# timer is stopped now
close(t)
if t.isopen
@atomic :monotonic t.isopen = false
ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), t)
end
end
notify(t.cond, true)
finally
Expand Down
6 changes: 3 additions & 3 deletions test/channels.jl
Original file line number Diff line number Diff line change
Expand Up @@ -457,8 +457,8 @@ end
Sys.iswindows() && Base.process_events() # schedule event (windows?)
close(async) # and close
@test !isopen(async)
@test tc[] == 2
@test tc[] == 2
@test tc[] == 3
@test tc[] == 3
yield() # consume event & then close
@test tc[] == 3
sleep(0.1) # no further events
Expand All @@ -479,7 +479,7 @@ end
close(async)
@test !isopen(async)
Base.process_events() # and close
@test tc[] == 0
@test tc[] == 1
yield() # consume event & then close
@test tc[] == 1
sleep(0.1) # no further events
Expand Down

0 comments on commit d0c4284

Please sign in to comment.