Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix assert_havelock(::ReentrantLock) to assert that the _current-task_ has the lock. #33159

Merged
merged 2 commits into from
Sep 6, 2019
Merged

Fix assert_havelock(::ReentrantLock) to assert that the _current-task_ has the lock. #33159

merged 2 commits into from
Sep 6, 2019

Commits on Sep 4, 2019

  1. Fix assert_havelock(::ReentrantLock) to assert that the _current-ta… 

    …sk_ 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"#JuliaLang#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"#JuliaLang#21#22")() at ./threadingconstructs.jl:113
```

Also adds unit test that failed before this commit but now succeeds
    @NHDaly
    NHDaly committed Sep 4, 2019
    Configuration menu
    Copy the full SHA
    9745986 View commit details
    Browse the repository at this point in the history

  2. Remove default impl of assert_havelock; add ::SpinLock impl

    @NHDaly
    NHDaly committed Sep 4, 2019
    Configuration menu
    Copy the full SHA
    59864d5 View commit details
    Browse the repository at this point in the history