Skip to content

Commit

Permalink
Document threadsafety of isready(::Channel) and remove data race
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
c42f committed Aug 9, 2021
1 parent 5118a1b commit 3e08033
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 2 deletions.
6 changes: 5 additions & 1 deletion base/channels.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
1 change: 0 additions & 1 deletion base/condition.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 3e08033

Please sign in to comment.