Feature #21852: New improved allocator function interface

• Status: Open
• Assignee: -
• Target version: -

[byroot (Jean Boussier)]

When implementing native types with the TypedData API, You have to define an allocator function.
That function receive the class to allocate and is supposed to return a new instance.

/**
 * This is  the type of  functions that ruby calls  when trying to  allocate an
 * object.  It is  sometimes necessary to allocate extra memory  regions for an
 * object.  When you define a class that uses ::RTypedData, it is typically the
 * case.  On  such situations  define a function  of this type  and pass  it to
 * rb_define_alloc_func().
 *
 * @param[in]  klass  The class that this function is registered.
 * @return     A newly allocated instance of `klass`.
 */
typedef VALUE (*rb_alloc_func_t)(VALUE klass);

Current API shortcomings

There are a few limitations with the current API.

Hard to disallow .allocate without breaking #dup and #clone.

First, it is frequent for extensions to want to disable Class#allocate for their native types via rb_undef_alloc_func, as very often allowing uninitialized object would lead to bugs.

The problem with rb_undef_alloc_func is that the alloc func is also used internally by dup and clone, so most types that undefine the allocator also prevent object copy without necessarily realizing it.

If you want to both disable Class#allocate yet still allow copying, you need to entirely implement the #dup and #clone methods, which is non-trivial and very few types do. One notable exception is Binding, which has to implement these two methods: https://github.com/ruby/ruby/blob/bea48adbcacc29cce9536977e15ceba0d65c8a02/proc.c#L301-L326

This works for Ruby code, however it doesn't work with C-level rb_obj_dup(VALUE), as used by the Ractor logic to copy objects across ractors.
In the case of Binding we probably wouldn't allow it anyway, but for other types it may be a problem.

Can't support objects of variable width

When duping or cloning an object of variable width, you need access to the original object to be able to allocate the right slot size.

An example of that is Thread::Backtrace objects, as evidenced by [Bug #21818].

To support sending exception objects across ractors, we'd need to make rb_obj_dup() work for Thread::Backtrace, but to correctly duplicate a backtrace, the allocator needs to know the size.

Proposed new API

I'd like to propose a new API for defining allocators:

typedef VALUE (*rb_copy_alloc_func_t)(VALUE klass, VALUE other);

In addition to the class to allocate, the function also receives the instance to copy.
When called by Class#allocate, the other argument is set to Qundef. Example usage:

static VALUE
backtrace_alloc(VALUE klass, VALUE other)
{
    rb_backtrace_t *bt;
    if (UNDEF_P(other)) {
        // Regular alloc
        return TypedData_Make_Struct(klass, rb_backtrace_t, &backtrace_data_type, bt);
    }
    else {
        // Copy
        rb_backtrace_t *other_bt;
        TypedData_Get_Struct(other, rb_backtrace_t, &backtrace_data_type, other_bt);
        VALUE self = backtrace_alloc_capa(other_bt->backtrace_size, &bt);
        bt->backtrace_size = other_bt->backtrace_size;
        MEMCPY(bt->backtrace, other_bt->backtrace, rb_backtrace_location_t, other_bt->backtrace_size);
        return self;
    }
}

Backward compatibility

Older-style allocator can keep being supported as long as we wish.

The one backward potential compatibility concern is third party code that calls rb_alloc_func_t rb_get_alloc_func(VALUE klass);.
As its documentation suggest, there's not much valid use case for it, but regardless we can keep supporting it by returning
a "jump function". See copy_allocator_adapter: https://github.com/ruby/ruby/pull/15795/changes#diff-884a5a8a369ef1b4c7597e00aa65974cec8c5f54f25f03ad5d24848f64892869R1640-R1653

Opportunity for more changes?

I was discussing this new interface with @ko1 (Koichi Sasada) and it appears that the current allocator interface may also be a limitation for Ractors and Ractor local GC. i.e. it might be useful to let the allocator function know that we're copying from one Ractor to another.

But I know to little about Ractor local GC to make a proposition here, so I will let @ko1 (Koichi Sasada) make suggestions.

Implementation

I implemented this idea in https://github.com/ruby/ruby/pull/15795, to solve [Bug #21818].
It could remain a purely private API, but I think it would make sense to expose it.

[Eregon (Benoit Daloze)]

Are initialize_copy/initialize_dup/initialize_clone still called when using a copy_allocator?
In the backtrace_alloc example you seem to already do some copying in that function, which then makes it unclear where the copying should be done.

First, it is frequent for extensions to want to disable Class#allocate for their native types via rb_undef_alloc_func, as very often allowing uninitialized object would lead to bugs.

How about using rb_undef_method(klass, "allocate"); for such cases?
I think that's a simple solution and requires no changes.

Fundamentally there is a difference between an allocate method and an alloc function, new/dup/clone all require an alloc function, but they should not require (and they don't IIRC) an allocate method.

[byroot (Jean Boussier)]

Are initialize_copy/initialize_dup/initialize_clone still called when using a copy_allocator?

Yes, it is unchanged.

In the backtrace_alloc example you seem to already do some copying in that function, which then makes it unclear where the copying should be done.

Indeed. Technically it wouldn't be required, but I think it's more reliable to do it there than in initialize_copy as the later could e redefined and cause corruption.

How about using rb_undef_method(klass, "allocate"); for such cases?

It's a corner case, but that allows redefining it later on.

[Eregon (Benoit Daloze)]

byroot (Jean Boussier) wrote in #note-2:

Indeed. Technically it wouldn't be required, but I think it's more reliable to do it there than in initialize_copy as the later could be redefined and cause corruption.

That could cause leaks if copying state involves extra allocations though, as a previously-existing initialize_copy might allocate and just set the pointers, but not free that first copy done in the copy_allocator.
It makes the contract unclear about what is supposed to copy what.

I think we need to trust initialize_copy, or invent a new Ruby-level protocol for copying objects.

Inventing a new Ruby-level protocol for copying objects and for creation without uninitialized state would be great.
Some core classes already do this but then they typically don't support dup/clone.
It'd be great to have this in general, so one could write classes that never have to care about uninitialized state since there are no instances in that uninitialized state ever.
We'd have some method to do both allocation + initialization at once, and another method to create a copy + initialize it as one call.

How about using rb_undef_method(klass, "allocate"); for such cases?

It's a corner case, but that allows redefining it later on.

I don't think we need to worry about this corner case.
Such things are clearly violating internals of the class, and then they might as well rb_define_alloc_func() and break it too.
But I suppose for core classes there might be a point to try to not segfault in that case, mmh.

Maybe classes should have a flag for "allow/disallow Class#allocate" and then Class#allocate would check that?
There is already a check for singleton classes, so we could merge it with that check for free and just have singleton classes always set that flag to false.

[byroot (Jean Boussier)]

Inventing a new Ruby-level protocol for copying objects and for creation without uninitialized state would be great.

Yes, this is somewhat what this new allocator API does. It also solves the problem that the Ractor API must be able to clone objects but can hardly trust user defined initialize_copy methods.

[Eregon (Benoit Daloze)]

It only does it for classes defined in C, and if they do all state copying in copy_allocator.
I think this would be valuable to have for any class, i.e. also for classes defined in Ruby and not in C.

[Eregon (Benoit Daloze)]

Thinking more about this I think there should be a protocol or an easy way to avoid the allocated-but-uninitialized state completely, which is problematic for classes defined in C but also in Ruby (though Ruby-defined classes will typically raise instead of segfault in such cases, but still the error can be cryptic and it's bad to have to deal with potentially-uninitialized objects).

I think we would need only two things:

• A method to allocate+initialize at once. This must be defined as a method of Class.
  • There is already Class#new, so overriding that seems the simplest.
• A method to allocate+initialize_copy at once.
  • This could be either an instance method, or a class taking an existing object as argument.
  • Overriding Kernel#dup seems the easiest here, and maybe define Kernel#clone as dup + extra clone logic to simplify.

In code:

module Kernel
  def clone
    copy = dup
    # extra clone logic like freezing
  end
end

# Usage
class MyClassWithoutUninitializedState
  ALLOCATE = method(:allocate)
  private_const :ALLOCATE

  def self.new(*args)
    ALLOCATE.call.tap { it.send(:initialize) }
  end

  def dup
    ALLOCATE.call.tap { |copy| copy.send(:initialize_copy, self) }
  end

  # Could either:
  undef_method :allocate # but that still allow using e.g. Class.instance_method(:allocate).bind_call(MyClassWithoutUninitializedState)

  # Or
  remove_alloc_function # Same as rb_undef_alloc_func(), this would be a new method of Class
  # but that probably would break `ALLOCATE.call` above
end

And similar for classes defined in C.
In C it's easy to call the alloc function directly even when not registered as such on the Class.

The tricky part of classes defined in Ruby is how to call the alloc function in overridden new/dup and still not expose it to users.
So I think a better solution would be a new Class method that defines new and dup and clone while taking a snapshot of the current alloc_func and then removes the alloc_func, then that would work reliably.

Say Class#safe_initialization which does something equivalent to (but more performant):

class Class
  def safe_initialization
    alloc_function = rb_get_alloc_func(self)

    define_singleton_method(:new) do |...|
      alloc_function.call.tap { it.send(:initialize) }
    end

    define_method(:dup) do
      alloc_function.call.tap { |copy| copy.send(:initialize_dup, self) }
    end

    define_method(:clone) do
      alloc_function.call.tap { |copy| copy.send(:initialize_clone, self) }
      # extra clone logic
    end
    
    rb_undef_alloc_func(self)
    return self
  end
end

# Usage
class MyClassWithoutUninitializedState
  safe_initialization
end

# .new, .dup, .clone all work fine

MyClassWithoutUninitializedState.allocate # => exception
Class.instance_method(:allocate).bind_call(MyClassWithoutUninitializedState) # => exception

And that would work for both classes defined in Ruby and C.

The only methods which would see uninitialized instances would be initialize, initialize_copy, initialize_dup, initialize_clone, which seems reasonable and also unavoidable anyway
(I don't think anyone wants to e.g. write def self.new = ALLOCATE.call.tap { it.instance_variable_set(:@foo, 42); ... } instead of def initialize; @foo = 42; ...; end).

[Eregon (Benoit Daloze)]

Going further, Class#safe_initialization instead of redefining these 3 methods could just set a new internal_alloc_func field in RClass (only used by new/dup/clone and can never be read by anything else) + rb_undef_alloc_func().
And rb_define_alloc_func() (as long as the current alloc_func is not NULL, i.e. hasn't been removed) could set that field too so that new/dup/clone only need to read that field and not the "public alloc_func" field.

[Eregon (Benoit Daloze)]

I'm aware what I propose doesn't solve Can't support objects of variable width, i.e. the case for Thread::Backtrace.
But AFAIK variable width is only available for core classes (not as public API), and Thread::Backtrace is core too so could just define dup & clone to allocate the right size and use some internal functions to not have to reimplement the rest of dup/clone.
Even with #21853 the alloc size would be fixed (per class) so it wouldn't need rb_copy_alloc_func_t.

One clear downside of rb_copy_alloc_func_t is performance as it introduces an extra check for new/dup/clone to check whether to use a normal or copy alloc_func.

BTW Thread::Backtrace is AFAIK never exposed to users, so there might be more direct ways to deal with that.

[Eregon (Benoit Daloze)]

• Related to Bug #21818: Thread backtraces cannot be communicated over Ractor ports added