Bug #21208: `Ractor#send(move: true)` allow moving objects that are on the stack, and used by C code.

• Status: Open
• Assignee: ractor
• Target version: -
• Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN

[byroot (Jean Boussier)]

The following script causes a crash:

rac = Ractor.new do
  Ractor.receive
end

hash = Hash[*50.times]

hash.merge!(12 => 0, 14 => 0) do |key, old_val, new_val|
  if key == 12
    rac.send(hash, move: true)
  end
  new_val
end

p rac.take

Contrary to previous crashes related to send(move: true), I'm afraid this one is with the design of the feature itself, not just the implementation.

If we allow objects to be moved, any C code that calls rb_yield() may cause memory corruption if one of the objects it uses is moved.

I personally can't see any realistic solution to this.

[byroot (Jean Boussier)]

The following script causes a crash

Actually, that was only because I was on an older branch, it no longer crash on master, and even behave somewhat correctly by sheer luck.

The general issue remain, send(obj, move: true) essentially zero-out the object slot, and none of the C methods that call rb_yield expect that, and I don't think we can realistically handle that everywhere in Ruby itself, and certainly not in C extensions.

[luke-gru (Luke Gruber)]

Hmm... this is tricky, good find! This probably won't occur often, I'm having a hard time coming up with a situation where it would happen with some benign looking code.

It's fun thinking about solutions to these kinds of things, so as thought experiment let's say we don't free the src object or zero out its slot and let the GC do it later. So for example an array would be moved by copying its storage into the new moved object and then keeping the original array around but niling out the contents or clearing it, keeping it a T_ARRAY so things like RARRAY_LEN still work on it (so Array#each still works if we're already in a call to it) but it appears as if it's a MovedObject to users that try to call methods on it. Then during GC, the GC could notice it's still a T_ARRAY and take care of the heap storage. Maybe we wouldn't even have to change the class to MovedObject, because that could complicate things if some code assumes that T_ARRAYs always have a class that is a descendant of Array for example. It could be a flag on the object that gets checked during every method call, and raises an error there instead of being its own real class.