Project

General

Profile

Actions

Bug #18243

open

Ractor.make_shareable does not freeze the receiver of a Proc but allows accessing ivars of it

Added by Eregon (Benoit Daloze) 2 months ago. Updated about 1 month ago.

Status:
Open
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:105573]

Description

class C
  attr_accessor :foo
  def setter_proc
    Ractor.make_shareable(-> v { @foo = v })
  end
end

c = C.new
c.foo = 1
p c
proc = c.setter_proc
p c.frozen?
Ractor.new(proc) { |s| s.call(42) }.take
p c

gives

#<C:0x00007f2a3aae7a98 @foo=1>
false
#<C:0x00007f2a3aae7a98 @foo=42> # BUG

But that must be a bug, it means the non-main Ractor can directly mutate an object from the main Ractor.

I found this while thinking about https://github.com/ruby/ostruct/pull/29/files and
whether Ractor.make_shareable would freeze @table and the OpenStruct instance (I think it needs to).

Repro code for ostruct and changing ostruct.rb to $setter = ::Ractor.make_shareable(setter_proc):

require 'ostruct'

os = OpenStruct.new
os.foo = 1

$setter.call(2)
p os

Ractor.new($setter) { |s| s.call(42) }.take
p os

gives

#<OpenStruct foo=2>
<internal:ractor>:267: warning: Ractor is experimental, and the behavior may change in future versions of Ruby! Also there are many implementation issues.
#<OpenStruct foo=42> # BUG

Related issues

Related to Ruby master - Bug #18232: Ractor.make_shareable is broken in code loaded with RubyVM::InstructionSequence.load_from_binaryClosedActions
Related to Ruby master - Feature #18276: `Proc#bind_call(obj)` same as `obj.instance_exec(..., &proc_obj)`RejectedActions
Actions #1

Updated by Eregon (Benoit Daloze) 2 months ago

  • Related to Bug #18232: Ractor.make_shareable is broken in code loaded with RubyVM::InstructionSequence.load_from_binary added

Updated by Eregon (Benoit Daloze) 2 months ago

Smaller repro:

$ ruby -e 'Object.new.instance_eval { p object_id; proc = Ractor.make_shareable -> { self }; Ractor.new(proc) { |c| p c.call.object_id }.take }'
60
<internal:ractor>:267: warning: Ractor is experimental, and the behavior may change in future versions of Ruby! Also there are many implementation issues.
60

In contrast to:

$ ruby -e 'a=Object.new; Ractor.make_shareable -> { a }'
<internal:ractor>:816:in `make_shareable': can not make shareable Proc because it can refer unshareable object #<Object:0x00007fd8bdd8f688> from variable `a' (Ractor::IsolationError)
    from -e:1:in `<main>'

It seems there is no check for self but there should be, it's like a capture local variable.

Either it raises like for local vars or it Ractor.make_shareable the self.

If it raises ostruct.rb will break but that's fixable by Ractor.make_shareable self first which is probably best for clarity anyway, cc marcandre (Marc-Andre Lafortune).

Updated by Eregon (Benoit Daloze) about 1 month ago

  • Description updated (diff)

Sorry, I missed to paste some part of the snippet, fixed now.

Updated by Eregon (Benoit Daloze) about 1 month ago

After some discussion with ko1 (Koichi Sasada), it seems there are different cases.
Sharing self is the only way to preserve the Proc's context correctly, i.e., method calls, @ivar, etc still work in the Proc

I think raising if self is not shareable might be a good way to let the user choose appropriately.
It's not clear if the user wants to make the Proc's self shareable, or if they want to change the Proc's self to a shareable object.
Then user can choose:

  • someProc = -> { ... }; Ractor.make_shareable(self); Ractor.make_shareable(someProc)
  • someProc = -> { ... }; Ractor.make_shareable(someProc.with_self(nil)).

define_method(:name) do; ...; end is special case because the receiver will be changed when called. And that's expected for users too. So maybe some special integration between define_method and Ractor makes sense (to skip the self check since not needed in that case).

Current semantics of passing the Proc across Ractors while it refers to unshareable objects is violation of semantics, i.e., I don't think raising on Proc#call is good idea.

------------------------------------------------------------------------
  Ractor.make_shareable(obj, copy: false) -> shareable_obj
------------------------------------------------------------------------
Make obj shareable between ractors.

obj and all the objects it refers to will be frozen, unless they are
already shareable.

I believe we need to preserve this simple definition to avoid explosive amounts of complexity and edge cases.

Actions #6

Updated by Eregon (Benoit Daloze) about 1 month ago

  • Related to Feature #18276: `Proc#bind_call(obj)` same as `obj.instance_exec(..., &proc_obj)` added

Updated by Dan0042 (Daniel DeLorme) about 1 month ago

It's not uncommon for a proc to have behavior that is independent of self. Let's say:

 Ractor.make_shareable(-> x { x + 1 })

In this case it's irrelevant if self is shareable or not, because it's never used in the proc. There's no method call or ivar access for self. It would be inconvenient if that kind of proc required self to be shareable. From the posts by Eregon (Benoit Daloze) it was unclear to me if "self must be shareable" was intended only for an explicit reference to self.

Updated by Eregon (Benoit Daloze) about 1 month ago

Dan0042 (Daniel DeLorme) wrote in #note-7:

It's not uncommon for a proc to have behavior that is independent of self.

Yes, but it's not so trivial to assert it is the case for any non-trivial block/Proc.
Changing the self is breaking the context in which it was written, and I think that should be avoided in most cases.
define_method(name) { ... } is clear that the self is not the surrounding self, but that's about the only core method with those semantics and it needs to be like that of course.

From the posts by Eregon (Benoit Daloze) it was unclear to me if "self must be shareable" was intended only for an explicit reference to self.

All Proc capture self, there is no other way currently.

I think we need define_method(name, make_shareable: true) { ... } (name up for discussion) and there we know it does not matter what the surrounding self is.

Ractor.make_shareable(some_proc) must make some_proc and all objects reachable from it shareable, hence the self captured by the Proc must be shareable, or it breaks the entire concept of Ractor isolation and shareable.
So either it does Ractor.make_shareable(proc_self) or it raises an exception.

Changing the proc's self on make_shareable seems way too confusing, I don't even consider it a reasonable possibility.

Updated by Eregon (Benoit Daloze) about 1 month ago

From #18276,
I think the best solution by far is Ractor.make_shareable(proc.bind(nil)) (concise, clear, clean as it does not pass unshared object to other Ractor), and raise on Ractor.make_shareable(proc) if the Proc's self is not shareable.
And of course Ractor.make_shareable(self); Ractor.make_shareable(proc) is also possible if desired, and explicit so safe and expected/intuitive.

Actions

Also available in: Atom PDF