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

finalizer is not being called for some REPL objects #10960

Closed
amitmurthy opened this issue Apr 23, 2015 · 15 comments
Closed

finalizer is not being called for some REPL objects #10960

amitmurthy opened this issue Apr 23, 2015 · 15 comments
Labels
bug Indicates an unexpected problem or unintended behavior regression Regression in behavior compared to a previous version

Comments

@amitmurthy
Copy link
Contributor

type Foo
    a
    function Foo(a)
        foo=new(a)
        finalizer(foo, x->@schedule print("FINALIZED!!!\n"))
        foo
    end
end

If executed in a single line, the finalizer is called.

julia> f=Foo(1);f=1;gc()
FINALIZED!!!

On separate lines, it is not.

julia> f=Foo(1)
Foo(1)

julia> f=1
1

julia> gc()

julia> gc()

julia> gc()

Came across this while debugging "leaks" in some parallel computation scenarios - turns out that the finalizer on RemoteRef objects is not being called in a consistent manner.

@amitmurthy
Copy link
Contributor Author

julia> versioninfo()
Julia Version 0.4.0-dev+4464
Commit 7e63e6a* (2015-04-23 00:49 UTC)
Platform Info:
  System: Linux (x86_64-linux-gnu)
  CPU: Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz
  WORD_SIZE: 64
  BLAS: libopenblas (USE64BITINT DYNAMIC_ARCH NO_AFFINITY Haswell)
  LAPACK: libopenblas
  LIBM: libopenlibm
  LLVM: libLLVM-3.3

@simonster
Copy link
Member

I can reproduce the issue above on 0.4, but the finalizer appears to be called correctly on 0.3:

julia> f=Foo(1)
Foo(1)

julia> f=1
1

julia> gc()
FINALIZED!!!

@simonster simonster added regression Regression in behavior compared to a previous version bug Indicates an unexpected problem or unintended behavior labels Apr 23, 2015
@JeffBezanson JeffBezanson added the GC Garbage collector label Apr 23, 2015
@carnaval
Copy link
Contributor

carnaval commented Jun 5, 2015

Not a GC problem. The object is staying in the task local "SHOWN_SET", whatever this is used for.

@carnaval carnaval removed the GC Garbage collector label Jun 5, 2015
@amitmurthy
Copy link
Contributor Author

Hmm, you are right. :SHOWNSET is used in show(::IO, ::Any) and show(::IO, ::Associative).

Defining a show(::IO, ::Foo) makes the problem go away.

I added

    ss = get(task_local_storage(), :SHOWNSET, [])
    if ss == nothing
        print("\n SHOWNSET not found")
    else
        print("\n num entries : $(length(ss))")
    end

to the end of

julia/base/show.jl

Lines 7 to 54 in e9fa25b

function show(io::IO, x::ANY)
t = typeof(x)::DataType
show(io, t)
print(io, '(')
nf = nfields(t)
if nf != 0 || t.size==0
recorded = false
shown_set = get(task_local_storage(), :SHOWNSET, nothing)
if shown_set == nothing
shown_set = ObjectIdDict()
task_local_storage(:SHOWNSET, shown_set)
end
try
if x in keys(shown_set)
print(io, "#= circular reference =#")
else
shown_set[x] = true
recorded = true
for i=1:nf
f = fieldname(t, i)
if !isdefined(x, f)
print(io, undef_ref_str)
else
show(io, x.(f))
end
if i < nf
print(io, ',')
end
end
end
catch e
rethrow(e)
finally
if recorded; delete!(shown_set, x); end
end
else
nb = t.size
print(io, "0x")
p = data_pointer_from_objref(x)
for i=nb-1:-1:0
print(io, hex(unsafe_load(convert(Ptr{UInt8}, p+i)), 2))
end
end
print(io,')')
end
and it shows 0 entries in the dict. Also :SHOWNSET is an ObjectIdDict .

How did you figure that the object is staying in the task local SHOWNSET?

@carnaval
Copy link
Contributor

carnaval commented Jun 5, 2015

ha this is a funny one. Turns out our eqtable implementation on which OIDict relies does not clear the pointer to the key when you delete an item. Looks like a one line fix, I'll push that.

To answer your question : unfortunately it's not very user friendly. I went into gdb and watched the type tag of the object when it is gced. At the point it is being marked the stack trace is essentially a "ground truth" explanation of why it is being kept alive.

We could have an actual tool for this but I won't have time to work on it any time soon.

@carnaval
Copy link
Contributor

carnaval commented Jun 5, 2015

Hum. It seems that nulling the key pointer leads to some pretty deep corruption. Someone somewhere must be relying on the bug.

@carnaval
Copy link
Contributor

carnaval commented Jun 5, 2015

Nevermind it was my bad. This should fix it.

@amitmurthy
Copy link
Contributor Author

Thanks.

@timholy
Copy link
Member

timholy commented Jun 6, 2015

Nice, @carnaval!

@ViralBShah
Copy link
Member

This is a nice fix!

@samuela
Copy link
Contributor

samuela commented Jun 7, 2015

Is there any way to test this? Can REPL sessions be tested somehow?

@carnaval
Copy link
Contributor

carnaval commented Jun 7, 2015

No need for repl session. Just put something with a finalizer as an ObjectIdDict key and delete! the entry.

@StefanKarpinski
Copy link
Member

Should this be backported to 0.3?

@tkelman
Copy link
Contributor

tkelman commented Jun 22, 2015

#10960 (comment) seems to indicate it's not a problem there?

@StefanKarpinski
Copy link
Member

Yeah, I'm trying to track down a memory leak in readall(echo hello) on release-0.3, and thought this might be the culprit, but cherry-picking this doesn't help so I suspect it doesn't need backporting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior regression Regression in behavior compared to a previous version
Projects
None yet
Development

No branches or pull requests

9 participants