From 98639f97ee0fd3e0f12be6c94ba094e522ca9967 Mon Sep 17 00:00:00 2001 From: Nathan Daly Date: Wed, 4 Sep 2019 13:24:57 -0400 Subject: [PATCH] Fix `assert_havelock(::ReentrantLock)` to assert that the _current-task_ has the lock. Before this commit, new threads would incorrectly believe that they held a lock on a Condition when they actually didn't, and would allow illegal operations, e.g. notify: ```julia julia> c = Threads.Condition() Base.GenericCondition{ReentrantLock}(Base.InvasiveLinkedList{Task}(nothing, nothing), ReentrantLock(nothing, Base.GenericCondition{Base.Threads.SpinLock}(Base.InvasiveLinkedList{Task}(nothing, nothing), Base.Threads.SpinLock(Base.Threads.Atomic{Int64}(0))), 0)) julia> lock(c) julia> fetch(Threads.@spawn Base.assert_havelock(c)) # This should be an ERROR (the new thread doesn't have the lock) julia> fetch(Threads.@spawn notify(c)) # This should be an ERROR (the new thread doesn't have the lock) 0 julia> fetch(Threads.@spawn wait(c)) # This error should be caught earlier (in assert_havelock). ERROR: TaskFailedException: unlock from wrong thread Stacktrace: [1] error(::String) at ./error.jl:33 [2] unlockall(::ReentrantLock) at ./lock.jl:121 [3] wait(::Base.GenericCondition{ReentrantLock}) at ./condition.jl:105 [4] (::var"##19#20")() at ./threadingconstructs.jl:113 ``` (The same holds for `@async` as `@spawn`.) After this change, the assertion works correctly: ``` julia> c = Threads.Condition(); julia> lock(c) julia> fetch(Threads.@spawn Base.assert_havelock(c)) # This correctly ERRORs ERROR: TaskFailedException: concurrency violation detected Stacktrace: [1] error(::String) at ./error.jl:33 [2] concurrency_violation() at ./condition.jl:8 [3] assert_havelock at ./condition.jl:28 [inlined] [4] assert_havelock at ./REPL[22]:1 [inlined] [5] assert_havelock(::Base.GenericCondition{ReentrantLock}) at ./condition.jl:73 [6] (::var"##21#22")() at ./threadingconstructs.jl:113 ``` --- base/lock.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/base/lock.jl b/base/lock.jl index 60066220b744e6..ad728280c43e9d 100644 --- a/base/lock.jl +++ b/base/lock.jl @@ -16,6 +16,7 @@ mutable struct ReentrantLock <: AbstractLock ReentrantLock() = new(nothing, GenericCondition{Threads.SpinLock}(), 0) end +assert_havelock(l::ReentrantLock) = assert_havelock(l, l.locked_by) """ islocked(lock) -> Status (Boolean)