From 8cc00ffd1c7851003acec419e01a4896f9ed88ad 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). --- base/asyncevent.jl | 67 +++++++++++++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 25 deletions(-) diff --git a/base/asyncevent.jl b/base/asyncevent.jl index 0736bd463111f..1c52b7cf7ee48 100644 --- a/base/asyncevent.jl +++ b/base/asyncevent.jl @@ -45,13 +45,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 @@ -115,6 +124,7 @@ function _trywait(t::Union{Timer, AsyncCondition}) # 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 @@ -123,14 +133,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) @@ -266,19 +274,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