Skip to content

Commit d0c4284

Browse files
authored
add missing wait during Timer and AsyncCondition close (#51847)
Can cause spurious warnings about not closing these properly and unexpected events to appear after `close` returns.
1 parent c54a3f2 commit d0c4284

File tree

2 files changed

+35
-10
lines changed

2 files changed

+35
-10
lines changed

base/asyncevent.jl

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -118,22 +118,26 @@ end
118118
unsafe_convert(::Type{Ptr{Cvoid}}, t::Timer) = t.handle
119119
unsafe_convert(::Type{Ptr{Cvoid}}, async::AsyncCondition) = async.handle
120120

121+
# if this returns true, the object has been signaled
122+
# if this returns false, the object is closed
121123
function _trywait(t::Union{Timer, AsyncCondition})
122124
set = t.set
123125
if set
124126
# full barrier now for AsyncCondition
125127
t isa Timer || Core.Intrinsics.atomic_fence(:acquire_release)
126128
else
127-
t.isopen || return false
128-
t.handle == C_NULL && return false
129+
if !isopen(t)
130+
close(t) # wait for the close to complete
131+
return false
132+
end
129133
iolock_begin()
130134
set = t.set
131135
if !set
132136
preserve_handle(t)
133137
lock(t.cond)
134138
try
135139
set = t.set
136-
if !set && t.isopen && t.handle != C_NULL
140+
if !set && t.handle != C_NULL # wait for set or handle, but not the isopen flag
137141
iolock_end()
138142
set = wait(t.cond)
139143
unlock(t.cond)
@@ -160,10 +164,28 @@ end
160164
isopen(t::Union{Timer, AsyncCondition}) = t.isopen && t.handle != C_NULL
161165

162166
function close(t::Union{Timer, AsyncCondition})
167+
t.handle == C_NULL && return # short-circuit path
163168
iolock_begin()
164-
if isopen(t)
165-
@atomic :monotonic t.isopen = false
166-
ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), t)
169+
if t.handle != C_NULL
170+
if t.isopen
171+
@atomic :monotonic t.isopen = false
172+
ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), t)
173+
end
174+
# implement _trywait here without the auto-reset function, just waiting for the final close signal
175+
preserve_handle(t)
176+
lock(t.cond)
177+
try
178+
while t.handle != C_NULL
179+
iolock_end()
180+
wait(t.cond)
181+
unlock(t.cond)
182+
iolock_begin()
183+
lock(t.cond)
184+
end
185+
finally
186+
unlock(t.cond)
187+
unpreserve_handle(t)
188+
end
167189
end
168190
iolock_end()
169191
nothing
@@ -220,7 +242,10 @@ function uv_timercb(handle::Ptr{Cvoid})
220242
@atomic :monotonic t.set = true
221243
if ccall(:uv_timer_get_repeat, UInt64, (Ptr{Cvoid},), t) == 0
222244
# timer is stopped now
223-
close(t)
245+
if t.isopen
246+
@atomic :monotonic t.isopen = false
247+
ccall(:jl_close_uv, Cvoid, (Ptr{Cvoid},), t)
248+
end
224249
end
225250
notify(t.cond, true)
226251
finally

test/channels.jl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -457,8 +457,8 @@ end
457457
Sys.iswindows() && Base.process_events() # schedule event (windows?)
458458
close(async) # and close
459459
@test !isopen(async)
460-
@test tc[] == 2
461-
@test tc[] == 2
460+
@test tc[] == 3
461+
@test tc[] == 3
462462
yield() # consume event & then close
463463
@test tc[] == 3
464464
sleep(0.1) # no further events
@@ -479,7 +479,7 @@ end
479479
close(async)
480480
@test !isopen(async)
481481
Base.process_events() # and close
482-
@test tc[] == 0
482+
@test tc[] == 1
483483
yield() # consume event & then close
484484
@test tc[] == 1
485485
sleep(0.1) # no further events

0 commit comments

Comments
 (0)