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.

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

fix dead-lock in WeakKeyDict #33756

Closed
wants to merge 2 commits into from
Closed

Conversation

swissr
Copy link
Contributor

@swissr swissr commented Nov 3, 2019

In Julia version v1.3.0-rc4 and v1.4.0-DEV.427 the following commands give a dead-lock:

x = WeakKeyDict(:a => 4)
push!(x, :b => 2)

It is caused by a lock in iterate(t::WeakKeyDict{K,V}) .. where the finalizer with unlock never gets called. The WeakKeyDict normally uses another task than the REPL; now when the terminal prints the wkd object, it uses the REPL task for the iterate! method and doesn't unlock it. Next time the WeakKeyDict does something, the object lock is 'locked_by' another task and the dict has to wait (forever...).

I'm not sure about my fix. I supposed that the iterate! methods should be protected but I don't understand the 'no keys will be deleted' remark. Please review. I would have a branch with nicely formatted log messages upon interest. Also I can add tests when it is clear how the fix will look like.

Hans-Peter Suter added 2 commits November 3, 2019 22:56
The command `x = WeakKeyDict(:a => 4)` followed by `push!(x, :b => 2)`
gives a deadlock. The reason is a `lock` command in the iterate! method
where the registered finalizer function with `unlock` never got called.
@swissr swissr changed the title Hps/wkdfix fix dead-lock in WeakKeyDict Nov 4, 2019
vtjnash added a commit that referenced this pull request Nov 12, 2019
This was a hold-over from the old iteration protocol, which needed to maintain state between `done` and `next`.
The `iteration` function of `Dict` has since been re-written to be safe for concurrent deletions.

Replaces: #33756
Co-Authored-By: Hans-Peter Suter <hps@treetron.ch>
@vtjnash
Copy link
Member

vtjnash commented Nov 12, 2019

Thanks! Sorry it's taken me so long to get back to you on this. I think this would have made sense, but that we can actually now just delete this gc-token. I've created a PR for that in #33825 and listed you as the co-author.

@swissr
Copy link
Contributor Author

swissr commented Nov 12, 2019

Thanks a lot for fixing! -- Next time will probably just submit an issue/bug report in cases like this (where I don't too well understand what is going on;)

@swissr swissr closed this Nov 12, 2019
@swissr swissr deleted the hps/wkdfix branch November 12, 2019 22:20
vtjnash added a commit that referenced this pull request Nov 13, 2019
This was a hold-over from the old iteration protocol, which needed to maintain state between `done` and `next`.
The `iteration` function of `Dict` has since been re-written to be safe for concurrent deletions.

Replaces: #33756
Co-Authored-By: Hans-Peter Suter <hps@treetron.ch>
JeffBezanson pushed a commit that referenced this pull request Nov 13, 2019
This was a hold-over from the old iteration protocol, which needed to maintain state between `done` and `next`.
The `iteration` function of `Dict` has since been re-written to be safe for concurrent deletions.

Replaces: #33756
Co-Authored-By: Hans-Peter Suter <hps@treetron.ch>
(cherry picked from commit 1731d0a)
KristofferC pushed a commit that referenced this pull request Dec 3, 2019
This was a hold-over from the old iteration protocol, which needed to maintain state between `done` and `next`.
The `iteration` function of `Dict` has since been re-written to be safe for concurrent deletions.

Replaces: #33756
Co-Authored-By: Hans-Peter Suter <hps@treetron.ch>
(cherry picked from commit 1731d0a)
KristofferC pushed a commit that referenced this pull request Feb 20, 2020
This was a hold-over from the old iteration protocol, which needed to maintain state between `done` and `next`.
The `iteration` function of `Dict` has since been re-written to be safe for concurrent deletions.

Replaces: #33756
Co-Authored-By: Hans-Peter Suter <hps@treetron.ch>
(cherry picked from commit 1731d0a)
KristofferC pushed a commit that referenced this pull request Feb 20, 2020
This was a hold-over from the old iteration protocol, which needed to maintain state between `done` and `next`.
The `iteration` function of `Dict` has since been re-written to be safe for concurrent deletions.

Replaces: #33756
Co-Authored-By: Hans-Peter Suter <hps@treetron.ch>
(cherry picked from commit 1731d0a)
KristofferC pushed a commit that referenced this pull request Feb 22, 2020
This was a hold-over from the old iteration protocol, which needed to maintain state between `done` and `next`.
The `iteration` function of `Dict` has since been re-written to be safe for concurrent deletions.

Replaces: #33756
Co-Authored-By: Hans-Peter Suter <hps@treetron.ch>
(cherry picked from commit 1731d0a)
KristofferC pushed a commit that referenced this pull request Apr 11, 2020
This was a hold-over from the old iteration protocol, which needed to maintain state between `done` and `next`.
The `iteration` function of `Dict` has since been re-written to be safe for concurrent deletions.

Replaces: #33756
Co-Authored-By: Hans-Peter Suter <hps@treetron.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants