crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.21k stars 1.61k forks source link

`ReferenceStorage(T)` is always atomic even when `T` isn't #14692

Closed HertzDevil closed 1 week ago

HertzDevil commented 3 weeks ago

Nothing is printed after running the snippet below, because Bar contains an inner pointer, and the compiler will automatically use GC.malloc:

class Foo
  @x = 123

  def finalize
    puts "finalize"
  end
end

class Bar
  @foo = Foo.new
end

x = Bar.new
GC.collect

However, finalize is printed if we replace Bar.new with .unsafe_construct (i.e. .pre_initialize + #initialize):

x = Bar.unsafe_construct(Pointer(ReferenceStorage(Bar)).malloc(1))
GC.collect # prints "finalize"

This is only possible if Pointer(ReferenceStorage(Bar)) uses GC.malloc_atomic behind the scenes; indeed, allocating the memory with GC.malloc_atomic(instance_sizeof(Bar)) would give the same result. I don't think this is ever intended, since this blocks some use cases of ReferenceStorage where multiple objects are allocated at once (more on this later).

straight-shoota commented 3 weeks ago

Yeah I had to wrap my head a couple of times to understand what this is about.

Until I realized that x is still alive on the stack, so the Bar instance doesn't get finalized. When allocating through ReferenceStorage via malloc_atomic, the garbage collector doesn't know that there is still a reference to the Foo instance and thus collects the memory (which calls the finalizer).

So the issue is that Pointer(ReferenceStorage(Bar)).malloc(1) resolves to GC.malloc_atomic despite Bar having inner pointers.