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

get! for WeakKeyDict does not add elements to finalizer, so they are not removed on they keys being garbage collected #24721

Closed
oxinabox opened this issue Nov 23, 2017 · 1 comment

Comments

@oxinabox
Copy link
Contributor

I think (and someone should check me if I am wrong)
that get! is borked for WeakKeyDict.

A key part of the functioning of WeakKeyDict, is that when an index is set,
the finalizers are hooked up, such that when that key is garbage collected,
its entry is deleted from the dictionary.

The following fails in 0.6

using Base.Test
    wkd = WeakKeyDict{Vector{Int}, Vector{Int}}()
    x = [1]
    wkd[x] = [-1]

    @test length(wkd)==1 # passes
    x = [2]
    gc()
    @test length(wkd)==0 # passes

    get!(wkd, WeakRef(x), [-2])
    @test length(wkd)==1 # passes

    x = [3]
    gc()
    @test length(wkd)==0 # fails, length(wkd)==1

    @show wkd.ht # wkd.ht = Dict(WeakRef(nothing)=>[-2]) 

Looking at the code: (https://github.com/JuliaLang/yyjulia/blob/0d7248e2ff65bd6886ba3f003bf5aeab929edab5/base/weakkeydict.jl#L109-L110)
get! on a WeakKeyDict uses to get! on a Dict, which in turn uses setindex!(::Dict...) (well _setindex2 or something),
rather than setindex!(::WeakKeyDict,...).

And thus the finalisers are never hooked in.
So the garbage collector does not remove the element from the dict.
The key does get change to WeakRef(nothing) but the value stays in the dict.
This is probably not intended

This is also way get! (unlike setindex) requires you to use WeakRef(k) for the key argument.
(similar for almost every other method. I thing all should take the specified keytype (rather than a WeakRef to it. Just like setindex! )

@oxinabox oxinabox changed the title get! for WeakKeyDict does not add elements to finalizer, so they are not removed on a GC() get! for WeakKeyDict does not add elements to finalizer, so they are not removed on they keys being garbage collected Nov 23, 2017
@laborg
Copy link
Contributor

laborg commented Feb 20, 2022

This got fixed in #38180

@laborg laborg closed this as completed Feb 20, 2022
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

No branches or pull requests

2 participants