Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix
assert_havelock(::ReentrantLock)
to assert that the _current-ta…
…sk_ has the lock. (#33159) * 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 ``` Also adds unit test that failed before this commit but now succeeds * Remove default impl of `assert_havelock`; add `::SpinLock` impl (cherry picked from commit 784eb57)
- Loading branch information