GC.collect might not be thread-safe

[asterite]

Many CI failure sin the preview_mt part always fail in a spec that checks that finalize is called, and for that GC.collect is invoked.

I believe there is some missing thread-safety there. I know the usual GC collection should be thread safe by now: the one that is automatically triggered when there's no more heap memory and it needs to be expanded. GC.collect is different: it's an explicit request to run the GC.

I already reported this through other mediums, I'm just tracking it here so we don't forget it.

Proofs (all of these, and more, crash after a call to GC.collect):

• https://circleci.com/gh/crystal-lang/crystal/28929
• https://circleci.com/gh/crystal-lang/crystal/28894
• https://circleci.com/gh/crystal-lang/crystal/28886
• https://circleci.com/gh/crystal-lang/crystal/28872

[asterite]

In fact this is very easy to reproduce:

$ bin/crystal build spec/std_spec.cr -Dpreview_mt
$ for i in {1..100} ; do ./std_spec ; done

In my case the first iteration already fails every time GC.collect is called.

This is one of the traces:

Invalid memory access (signal 11) at address 0x3800922622e0
[0x100d7bc1b] *CallStack::print_backtrace:Int32 +107
[0x10081b015] __crystal_sigfault_handler +181
[0x102e5c3c3] sigfault_handler +35
[0x7fff72a16b5d] _sigtramp +29
[0x102e5370a] GC_mark_from +786
[0x102e53f9f] GC_do_local_mark +55
[0x102e54186] GC_mark_local +276
[0x102e53a6e] GC_do_parallel_mark +111
[0x102e531e0] GC_mark_some +254
[0x102e4bac3] GC_stopped_mark +177
[0x102e4b980] GC_try_to_collect_inner +310
[0x102e4c3e8] GC_try_to_collect_general +188
[0x102e4c45e] GC_gcollect +13
[0x100d84d39] *GC::collect:Nil +9
[0x100c5a03b] ~procProc(Nil)@spec/std/openssl/ssl/context_spec.cr:172 +123
[0x101b93382] *Spec::Example#run:(Array(Spec::Result) | Nil) +610
[0x101b94982] *Spec::NestedContext#run:Nil +306
[0x101b7c056] *Spec::RootContext#run:Nil +150
[0x10082f267] ~procProc(Int32, (Exception | Nil), Nil)@src/spec/dsl.cr:160 +247
[0x100ee41c1] *AtExitHandlers::run<Int32>:Int32 +193
[0x1017c3ade] *Crystal::main<Int32, Pointer(Pointer(UInt8))>:Int32 +126
[0x1007fa859] main +9

[refi64]

Looks like by default it indeed isn't: https://www.hboehm.info/gc/scale.html At minimum, Fedora does compile libgc with threads enabled, and I'd guess Ubuntu and other distros probably do as well. Then, it boils down to whatever the THREAD macros change at include time?

…

On Sat, Sep 14, 2019, 6:44 PM Ary Borenszweig ***@***.***> wrote: In fact this is very easy to reproduce: $ bin/crystal build spec/std_spec.cr -Dpreview_mt $ for i in {1..100} ; do ./std_spec ; done In my case the first iteration already fails every time GC.collect is called. This is one of the traces: Invalid memory access (signal 11) at address 0x3800922622e0 [0x100d7bc1b] *CallStack::print_backtrace:Int32 +107 [0x10081b015] __crystal_sigfault_handler +181 [0x102e5c3c3] sigfault_handler +35 [0x7fff72a16b5d] _sigtramp +29 [0x102e5370a] GC_mark_from +786 [0x102e53f9f] GC_do_local_mark +55 [0x102e54186] GC_mark_local +276 [0x102e53a6e] GC_do_parallel_mark +111 [0x102e531e0] GC_mark_some +254 [0x102e4bac3] GC_stopped_mark +177 [0x102e4b980] GC_try_to_collect_inner +310 [0x102e4c3e8] GC_try_to_collect_general +188 [0x102e4c45e] GC_gcollect +13 [0x100d84d39] *GC::collect:Nil +9 [0x100c5a03b] ***@***.***/std/openssl/ssl/context_spec.cr:172 +123 [0x101b93382] *Spec::Example#run:(Array(Spec::Result) | Nil) +610 [0x101b94982] *Spec::NestedContext#run:Nil +306 [0x101b7c056] *Spec::RootContext#run:Nil +150 [0x10082f267] ~procProc(Int32, (Exception | Nil), ***@***.***/spec/dsl.cr:160 +247 [0x100ee41c1] *AtExitHandlers::run<Int32>:Int32 +193 [0x1017c3ade] *Crystal::main<Int32, Pointer(Pointer(UInt8))>:Int32 +126 [0x1007fa859] main +9 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#8184?email_source=notifications&email_token=AAM4YSJ2PKQ3TWOI3W7YXJLQJVZPXA5CNFSM4IWYJ5JKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6XGEAY#issuecomment-531522051>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAM4YSMBYQQDLC4UOFLHTNDQJVZPXANCNFSM4IWYJ5JA> .

[waj]

None of that. At the end the issue is really simple but it was quite hard to find. Hopefully fixed with this PR: #8196