julia icon indicating copy to clipboard operation
julia copied to clipboard

gc: add missing root for binding->ty field

Open staticfloat opened this issue 3 years ago • 1 comments

Previously, we might observe this code segfault if the memory at binding->ty eventually was reused due to this missing GC root.

julia> x::Union{Int,Nothing} = 2
2

julia> GC.gc()

julia> Core.get_binding_type(Main, :x)
Union{Nothing, Int64}

NOTE: I opened this PR on Jameson's behalf; the branch name in https://github.com/JuliaLang/julia/pull/46804 broke cloning from windows, so I moved the commits to a new branch and deleted his branch.

staticfloat avatar Sep 16 '22 21:09 staticfloat

Could we add a test for this?

Edit: I'm not sure this works

julia> x::Union{Int,Nothing} = 2
2

julia> GC.gc()
GC error (probable corruption) :
Allocations: 322489 (Pool: 322233; Big: 256); GC: 0
0
0x123bf4000: Root object: 0x10a984010 :: 0x119cf4420 (bits: 1)
        of type Task
0x123bf4018: Root object: 0x10a9844c0 :: 0x119cf4420 (bits: 1)
        of type Task
0x123bf4030: Root object: 0x10aa74010 :: 0x119cf4420 (bits: 1)
        of type Task
0x123bf4048: Root object: 0x10aa7c010 :: 0x119cf4420 (bits: 1)
        of type Task
0x123bf4060: Root object: 0x10a9841a0 :: 0x119cf4420 (bits: 1)
        of type Task
0x123bf4078: Root object: 0x10aa78010 :: 0x119cf4420 (bits: 1)
        of type Task
0x123bf4090:  r-- Module (bindings) 0x119f7afe0 (bits 3) -- [0x14f9101d8, 0x14f910400)

[11046] signal (6): Abort trap: 6
in expression starting at REPL[2]:1
__pthread_kill at /usr/lib/system/libsystem_kernel.dylib (unknown line)
Allocations: 322489 (Pool: 322233; Big: 256); GC: 0
fish: Job 1, './julia' terminated by signal SIGABRT (Abort)

gbaraldi avatar Sep 16 '22 21:09 gbaraldi

This still fails for me.

gbaraldi avatar Sep 29 '22 20:09 gbaraldi

Would be good if someone addressed https://github.com/JuliaLang/julia/pull/46806#issuecomment-1262795256 to not leave it hanging.

KristofferC avatar Oct 06 '22 19:10 KristofferC

The test case didn't fail for me anymore. But maybe adding a test would be nice

gbaraldi avatar Oct 06 '22 19:10 gbaraldi