From 3e0803345e764b921805f3987556abf44267faae Mon Sep 17 00:00:00 2001 From: Chris Foster Date: Mon, 9 Aug 2021 12:46:49 +1000 Subject: [PATCH] Document threadsafety of isready(::Channel) and remove data race Make it clear that isready() is not threadsafe, as it has a data race. We could make it threadsafe with lock() but this would also make it blocking and wouldn't make uses of it non-racy without a lock around the surronding code. So for now just document the current state of affairs. Also remove an optimistic use of `isready()` in `wait()` as it's a data race. --- base/channels.jl | 6 +++++- base/condition.jl | 1 - 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/base/channels.jl b/base/channels.jl index 1557504bbe21e..5fb09f38f7a01 100644 --- a/base/channels.jl +++ b/base/channels.jl @@ -416,6 +416,11 @@ immediately, does not block. For unbuffered channels returns `true` if there are tasks waiting on a [`put!`](@ref). + +!!! warning + This function isn't threadsafe. For multithreaded code, use it inside a + block of code which is synchronized with `lock(c)` to avoid race + conditions. """ isready(c::Channel) = n_avail(c) > 0 n_avail(c::Channel) = isbuffered(c) ? length(c.data) : length(c.cond_put.waitq) @@ -427,7 +432,6 @@ unlock(c::Channel) = unlock(c.cond_take) trylock(c::Channel) = trylock(c.cond_take) function wait(c::Channel) - isready(c) && return lock(c) try while !isready(c) diff --git a/base/condition.jl b/base/condition.jl index be0f618865a48..ce725823dc3b5 100644 --- a/base/condition.jl +++ b/base/condition.jl @@ -79,7 +79,6 @@ lock(f, c::GenericCondition) = lock(f, c.lock) # have waiter wait for c function _wait2(c::GenericCondition, waiter::Task) - ct = current_task() assert_havelock(c) push!(c.waitq, waiter) return