From 8d13430448cd94d73af5a53de058485c48f9c9f3 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Wed, 13 Apr 2022 16:26:36 -0400 Subject: [PATCH] asyncevents: fix missing GC root and race (#44956) The event might have triggered on another thread before we observed it here, or it might have gotten finalized before it got triggered. Either outcome can result in a lost event. (I observed the later situation occurring locally during the Dates test once). (cherry picked from commit 8cc00ffd1c7851003acec419e01a4896f9ed88ad) --- base/asyncevent.jl | 72 +++++++++++++++++++++++++++++----------------- 1 file changed, 46 insertions(+), 26 deletions(-) diff --git a/base/asyncevent.jl b/base/asyncevent.jl index ca0e5da512147..08d32e0eac250 100644 --- a/base/asyncevent.jl +++ b/base/asyncevent.jl @@ -43,13 +43,22 @@ the async condition object itself. """ function AsyncCondition(cb::Function) async = AsyncCondition() - t = @task while _trywait(async) - cb(async) - isopen(async) || return + t = @task begin + unpreserve_handle(async) + while _trywait(async) + cb(async) + isopen(async) || return + end + end + # here we are mimicking parts of _trywait, in coordination with task `t` + preserve_handle(async) + @lock async.cond begin + if async.set + schedule(t) + else + _wait2(async.cond, t) + end end - lock(async.cond) - _wait2(async.cond, t) - unlock(async.cond) return async end @@ -103,7 +112,11 @@ unsafe_convert(::Type{Ptr{Cvoid}}, async::AsyncCondition) = async.handle function _trywait(t::Union{Timer, AsyncCondition}) set = t.set - if !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 iolock_begin() set = t.set @@ -112,14 +125,12 @@ function _trywait(t::Union{Timer, AsyncCondition}) lock(t.cond) try set = t.set - if !set - if t.handle != C_NULL - iolock_end() - set = wait(t.cond) - unlock(t.cond) - iolock_begin() - lock(t.cond) - end + if !set && t.isopen && t.handle != C_NULL + iolock_end() + set = wait(t.cond) + unlock(t.cond) + iolock_begin() + lock(t.cond) end finally unlock(t.cond) @@ -255,19 +266,28 @@ julia> begin """ function Timer(cb::Function, timeout::Real; interval::Real=0.0) timer = Timer(timeout, interval=interval) - t = @task while _trywait(timer) - try - cb(timer) - catch err - write(stderr, "Error in Timer:\n") - showerror(stderr, err, catch_backtrace()) - return + t = @task begin + unpreserve_handle(timer) + while _trywait(timer) + try + cb(timer) + catch err + write(stderr, "Error in Timer:\n") + showerror(stderr, err, catch_backtrace()) + return + end + isopen(timer) || return + end + end + # here we are mimicking parts of _trywait, in coordination with task `t` + preserve_handle(timer) + @lock timer.cond begin + if timer.set + schedule(t) + else + _wait2(timer.cond, t) end - isopen(timer) || return end - lock(timer.cond) - _wait2(timer.cond, t) - unlock(timer.cond) return timer end