From f5f8c1408438eabf3fc0b5829c14ca1084ce4458 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Tue, 12 Apr 2022 15:56:43 -0400 Subject: [PATCH] asyncevents: fix missing GC root and race 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