Skip to content

Commit

Permalink
add missing wait during Timer and AsyncCondition close
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 (as exemplified in the
tests changes).
  • Loading branch information
vtjnash committed Oct 24, 2023
1 parent df39cee commit 89d7aa7
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 89d7aa7

Please sign in to comment.