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

BigFloat stored in WeakKeyDicts cause segfaults when garbage collected #26939

Closed
oxinabox opened this issue May 1, 2018 · 4 comments · Fixed by #38180
Closed

BigFloat stored in WeakKeyDicts cause segfaults when garbage collected #26939

oxinabox opened this issue May 1, 2018 · 4 comments · Fixed by #38180
Labels
bug Indicates an unexpected problem or unintended behavior GC Garbage collector regression Regression in behavior compared to a previous version

Comments

@oxinabox
Copy link
Contributor

oxinabox commented May 1, 2018

In v0.6.2

This error is similar to #18204 (comment)

MWE:

x = WeakKeyDict()
x[big"1.0"] = 1
gc()

REPL:

               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: https://docs.julialang.org
   _ _   _| |_  __ _   |  Type "?help" for help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.6.2 (2017-12-13 18:08 UTC)
 _/ |\__'_|_|_|\__'_|  |  Official http://julialang.org/ release
|__/                   |  x86_64-pc-linux-gnu

julia> x = WeakKeyDict()
WeakKeyDict{Any,Any} with 0 entries

julia> x[big"1.0"] = 1
1

julia> gc()

signal (11): Segmentation fault
while loading no file, in expression starting on line 0
unknown function (ip: 0x7f33961c1a4f)
decompose at ./hashing2.jl:140
hash at ./hashing2.jl:32
hashindex at ./dict.jl:210 [inlined]
ht_keyindex at ./dict.jl:322
delete! at ./dict.jl:558 [inlined]
#333 at ./weakkeydict.jl:113
lock at ./lock.jl:101
#311 at ./weakkeydict.jl:25
jl_call_fptr_internal at /buildworker/worker/package_linux64/build/src/julia_internal.h:339 [inlined]
jl_call_method_internal at /buildworker/worker/package_linux64/build/src/julia_internal.h:358 [inlined]
jl_apply_generic at /buildworker/worker/package_linux64/build/src/gf.c:1926
jl_apply at /buildworker/worker/package_linux64/build/src/julia.h:1424 [inlined]
run_finalizer at /buildworker/worker/package_linux64/build/src/gc.c:111
jl_gc_run_finalizers_in_list at /buildworker/worker/package_linux64/build/src/gc.c:200
run_finalizers at /buildworker/worker/package_linux64/build/src/gc.c:234 [inlined]
jl_gc_collect at /buildworker/worker/package_linux64/build/src/gc.c:2112
gc at ./base.jl:144
unknown function (ip: 0x7f336bfa35af)
jl_call_fptr_internal at /buildworker/worker/package_linux64/build/src/julia_internal.h:339 [inlined]
jl_call_method_internal at /buildworker/worker/package_linux64/build/src/julia_internal.h:358 [inlined]
jl_apply_generic at /buildworker/worker/package_linux64/build/src/gf.c:1926
do_call at /buildworker/worker/package_linux64/build/src/interpreter.c:75
eval at /buildworker/worker/package_linux64/build/src/interpreter.c:242
jl_interpret_toplevel_expr at /buildworker/worker/package_linux64/build/src/interpreter.c:34
jl_toplevel_eval_flex at /buildworker/worker/package_linux64/build/src/toplevel.c:577
jl_toplevel_eval_in at /buildworker/worker/package_linux64/build/src/builtins.c:496
eval at ./boot.jl:235
unknown function (ip: 0x7f33907cdd2f)
jl_call_fptr_internal at /buildworker/worker/package_linux64/build/src/julia_internal.h:339 [inlined]
jl_call_method_internal at /buildworker/worker/package_linux64/build/src/julia_internal.h:358 [inlined]
jl_apply_generic at /buildworker/worker/package_linux64/build/src/gf.c:1926
eval_user_input at ./REPL.jl:66
unknown function (ip: 0x7f339084f18f)
jl_call_fptr_internal at /buildworker/worker/package_linux64/build/src/julia_internal.h:339 [inlined]
jl_call_method_internal at /buildworker/worker/package_linux64/build/src/julia_internal.h:358 [inlined]
jl_apply_generic at /buildworker/worker/package_linux64/build/src/gf.c:1926
macro expansion at /home/uniwa/students2/students/20361362/linux/.julia/v0.6/Revise/src/Revise.jl:775 [inlined]
#17 at ./event.jl:73
unknown function (ip: 0x7f336bf9409f)
jl_call_fptr_internal at /buildworker/worker/package_linux64/build/src/julia_internal.h:339 [inlined]
jl_call_method_internal at /buildworker/worker/package_linux64/build/src/julia_internal.h:358 [inlined]
jl_apply_generic at /buildworker/worker/package_linux64/build/src/gf.c:1926
jl_apply at /buildworker/worker/package_linux64/build/src/julia.h:1424 [inlined]
start_task at /buildworker/worker/package_linux64/build/src/task.c:267
unknown function (ip: 0xffffffffffffffff)
Allocations: 2381902 (Pool: 2380620; Big: 1282); GC: 3
Segmentation fault (core dumped)
(venv_p3) 20361362@ECM-DTC-678L:~$ 
@JeffBezanson JeffBezanson added bug Indicates an unexpected problem or unintended behavior GC Garbage collector labels May 1, 2018
@JeffBezanson
Copy link
Member

Finalizers are currently being run in the order they were added, but should instead be run in reverse order to help guard against this. In versions <=0.3 we did that by keeping a linked list for each object with finalizers, which was slower but avoided this issue. For now reversing the for loop should be sufficient.

@JeffBezanson
Copy link
Member

Why the reopen?

@JeffBezanson
Copy link
Member

Answer: if the WeakKeyDict is locked, the finalizer re-schedules itself to try again later, but that will be too late since the other finalizers for the key will run during the current round of finalization. This is at least much less likely than the example in this issue though.

Also, the problem should no longer exist for BigFloats since they no longer use finalizers.

@vtjnash
Copy link
Member

vtjnash commented Jun 13, 2018

This is at least much less likely than the example in this issue though.

Note that in this example, its likely guaranteed not to occur. Since we only add precisely one value to the dictionary and then drop everything, for the test we will never be found holding the lock while there are potentially values in the dictionary.

vtjnash added a commit that referenced this issue Oct 23, 2020
Delay cleanup of WeakKeyDict items until the next insertion. And fix
`get!`, since previously usage of it would have added keys without
finalizers to the dict.

Fixes #26939
vtjnash added a commit that referenced this issue Dec 6, 2020
Delay cleanup of WeakKeyDict items until the next insertion. And fix
`get!`, since previously usage of it would have added keys without
finalizers to the dict.

Fixes #26939
JeffBezanson pushed a commit that referenced this issue Dec 8, 2020
Delay cleanup of WeakKeyDict items until the next insertion. And fix
`get!`, since previously usage of it would have added keys without
finalizers to the dict.

Fixes #26939
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this issue May 4, 2021
Delay cleanup of WeakKeyDict items until the next insertion. And fix
`get!`, since previously usage of it would have added keys without
finalizers to the dict.

Fixes JuliaLang#26939
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 GC Garbage collector regression Regression in behavior compared to a previous version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants