Bug #21451: Ractor.make_shareable(->{}, copy: true) raises unhelpful error

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

[tenderlovemaking (Aaron Patterson)]

> ruby -e'Ractor.make_shareable(->{}, copy:true)'
<internal:ractor>:828:in 'Ractor.make_shareable': allocator undefined for Proc (TypeError)
	from -e:1:in '<main>'

This error isn't very helpful and I think we can improve it. The exception happens when we call rb_obj_clone on the lambda.

I've made a patch to improve the error message so it's like this:

> ./miniruby -e'Ractor.make_shareable(->{}, copy:true)'
-e:1:in 'Ractor.make_shareable': cannot copy #<Proc:0x000000011f311a80 -e:1 (lambda)> (Ractor::IsolationError)
	from -e:1:in '<main>'
-e:1:in 'Ractor.make_shareable': allocator undefined for Proc (TypeError)
	from -e:1:in '<main>'

The patch is here: https://github.com/ruby/ruby/pull/13703

[Eregon (Benoit Daloze)]

This doesn't really explain from a user POV why it can't copy the Proc though, after all ->{}.dup works fine.
"No allocator" is an internal thing Ractor.make_shareable could work around, is there a more fundamental reason why it shouldn't work?

Maybe the correct fix here is to actually support Ractor.make_shareable(->{}, copy: true), as mentioned in #21039?

[tenderlovemaking (Aaron Patterson)]

Eregon (Benoit Daloze) wrote in #note-1:

This doesn't really explain from a user POV why it can't copy the Proc though, after all ->{}.dup works fine.
"No allocator" is an internal thing Ractor.make_shareable could work around, is there a more fundamental reason why it shouldn't work?

Maybe the correct fix here is to actually support Ractor.make_shareable(->{}, copy: true), as mentioned in #21039?

AFAIK, procs fundamentally can't be shared because their environment is mutable. Even if we copy the proc, its environment is still mutable so I'm not sure if make_shareable should ever work on them. OTOH, if you send the proc to a Ractor I could see it getting copied at that boundary as only the receiving Ractor gets the proc.

That said, I think I should have been more clear in this ticket description. IMO the problem isn't with Procs in particular, it's any object that can't be copied via make_shareable. The error message is not helpful.

Here is an example that doesn't use a Proc:

obj = "".freeze
begin
  obj.bar
rescue => err
end

p err
Ractor.make_shareable err, copy: true

The exception is a TypeError rather than something more helpful:

> ruby test.rb
#<NoMethodError: undefined method 'bar' for an instance of String>
<internal:ractor>:828:in 'Ractor.make_shareable': allocator undefined for RubyVM::InstructionSequence (TypeError)
	from test.rb:8:in '<main>'

I think we should just accept that rb_obj_clone can possibly raise an exception, and we should re-raise with a more helpful exception. The current behavior is even more unhelpful when you consider that the non-copiable object may be part of a larger object graph.

Consider this code:

def make_lambda
  lambda { }
end

def make_lambda2
  lambda { }
end

hash = { key: make_lambda }

Ractor.make_shareable hash, copy: true

With the current version of Ruby, the exception is like this:

> ruby test.rb
<internal:ractor>:828:in 'Ractor.make_shareable': allocator undefined for Proc (TypeError)
	from test.rb:11:in '<main>'

Fortunately the test program is short, but if this hash came from a distant place in the application, how would we know which Proc is the problem? Currently we get this type of error when trying to make a Rails application shareable. But the only way I can find the specific lambdas causing problems is by hacking Ruby. At least with the patch I've provided, I can see from the error message more info about the troublesome object.

[Eregon (Benoit Daloze)]

tenderlovemaking (Aaron Patterson) wrote in #note-2:

AFAIK, procs fundamentally can't be shared because their environment is mutable.

Currently Ractor.make_shareable(proc) makes a shallow copy of the environment, inplace. I believe that's wrong for Ruby semantics because it "breaks" an existing Proc, that's the subject of #21039.
But with copy: true it would at least not affect existing references to the Proc, which seems significantly better, and also it makes it clearer it takes a shallow copy of the Proc environment.

tenderlovemaking (Aaron Patterson) wrote in #note-2:

I think we should just accept that rb_obj_clone can possibly raise an exception, and we should re-raise with a more helpful exception.

I see, makes sense, agreed it's a good way.
I would suggest a different message and error class to improve clarity though:

-e:1:in 'Ractor.make_shareable': cannot copy #<Proc:0x000000011f311a80 -e:1 (lambda)> (Ractor::IsolationError)

=>

-e:1:in 'Ractor.make_shareable': cannot make #<Proc:0x000000011f311a80 -e:1 (lambda)> shareable with copy (ArgumentError)
OR
-e:1:in 'Ractor.make_shareable': cannot make shareable with copy: #<Proc:0x000000011f311a80 -e:1 (lambda)> (ArgumentError)

Ractor::IsolationError seems a bit strange, so I think simply ArgumentError, or maybe something like Ractor::CopyError would be better.