From fd9d35959e0ea50166b5dff3243001139950cd49 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 13 Jan 2025 14:52:49 -0800 Subject: [PATCH] Allow lambdas that don't access self to be made shareable This commit allows lambdas that don't access self to be "made shareable" regardless of the shareability of self. For example, the following code used to produce a lambda that was not shareable: ```ruby class Foo def make_block; lambda { 1234 }; end end Ractor.make_shareable(Foo.new.make_block) # exception ``` This lambda was not allowed to be shareable because it could possibly access `self` which is an unfrozen instance of `Foo`. However, we know by looking at the code that it doesn't access the instance of `Foo`, so I think we should lift this restriction. Upon calling `make_shareable`, this change scans the instructions of the block looking for particular instructions that access `self`. If it sees any of those instructions, then we use the default behavior (checking sharability of `self`). If we don't see those instructions, then we'll allow the lambda to be shareable. For example, this is shareable: ```ruby def make_block foo = 123 lambda { foo } end ``` But these are not shareable: ```ruby def make_block lambda { @foo } end ``` ```ruby def make_block lambda { @foo = 1 } end ``` ```ruby def make_block lambda { eval("123") } end ``` [Feature #21033] --- bootstraptest/test_ractor.rb | 138 +++++++++++++++++++++++++++++++++++ iseq.c | 19 +++++ iseq.h | 1 + vm.c | 2 +- 4 files changed, 159 insertions(+), 1 deletion(-) diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index 5eb297b57a..bd547aa3e2 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -1361,6 +1361,144 @@ def /(other) Ractor.shareable?(pr) } +# Ractor.make_shareable(a_proc) that doesn't access self +assert_equal 'true', %q{ + class Foo + def make_block; lambda { 123 }; end + end + + pr = Foo.new.make_block + Ractor.make_shareable(pr) + Ractor.shareable?(pr) +} + +# Ractor.make_shareable(a_proc) should fail when accessing self +assert_equal 'true', %q{ + class Foo + def make_block; lambda { self }; end + end + + pr = Foo.new.make_block + begin + Ractor.make_shareable(pr) + rescue Ractor::IsolationError + true + end +} + +# Ractor.make_shareable(a_proc) should fail when accessing self via method +assert_equal 'true', %q{ + class Foo + def hi; end + def make_block; lambda { hi }; end + end + + pr = Foo.new.make_block + begin + Ractor.make_shareable(pr) + rescue Ractor::IsolationError + true + end +} + +# Ractor.make_shareable(a_proc) should fail when accessing self via eval (also a method call) +assert_equal 'true', %q{ + class Foo + def make_block; lambda { eval("123") }; end + end + + pr = Foo.new.make_block + begin + Ractor.make_shareable(pr) + rescue Ractor::IsolationError + true + end +} + +# Ractor.make_shareable(a_proc) should fail when accessing self via local +assert_equal 'true', %q{ + class Foo + def make_block + x = self + lambda { x } + end + end + + pr = Foo.new.make_block + begin + Ractor.make_shareable(pr) + rescue Ractor::IsolationError + true + end +} + +# Ractor.make_shareable(a_proc) should fail when accessing self via ivar get +assert_equal 'true', %q{ + class Foo + def initialize; @x = 1; end + def make_block + lambda { @x } + end + end + + pr = Foo.new.make_block + begin + Ractor.make_shareable(pr) + rescue Ractor::IsolationError + true + end +} + +# Ractor.make_shareable(a_proc) should fail when accessing self via ivar set +assert_equal 'true', %q{ + class Foo + def initialize; @x = 1; end + def make_block + lambda { @x = 456 } + end + end + + pr = Foo.new.make_block + begin + Ractor.make_shareable(pr) + rescue Ractor::IsolationError + true + end +} + +# Ractor.make_shareable(a_proc) should fail with a circular lambda +assert_equal 'true', %q{ + class Foo + def make_block + x = lambda { x } + x + end + end + + pr = Foo.new.make_block + begin + Ractor.make_shareable(pr) + rescue Ractor::IsolationError + true + end +} + +# Ractor.make_shareable(a_proc) should fail with `defined?(@iv)` +assert_equal 'true', %q{ + class Foo + def make_block + lambda { defined?(@foo) } + end + end + + pr = Foo.new.make_block + begin + Ractor.make_shareable(pr) + rescue Ractor::IsolationError + true + end +} + # Ractor.shareable?(recursive_objects) assert_equal '[false, false]', %q{ y = [] diff --git a/iseq.c b/iseq.c index 614d4a0f87..ff8be7a984 100644 --- a/iseq.c +++ b/iseq.c @@ -3814,6 +3814,25 @@ rb_vm_insn_decode(const VALUE encoded) return insn; } +// Returns a boolean indicating whether or not the iseq accesses self. +bool +rb_vm_iseq_reads_self(const rb_iseq_t *iseq) +{ + unsigned int pos = 0; + while (pos < ISEQ_BODY(iseq)->iseq_size) { + int opcode = rb_vm_insn_decode(ISEQ_BODY(iseq)->iseq_encoded[pos]); + unsigned int next_pos = pos + insn_len(opcode); + if (opcode == BIN(putself) || + opcode == BIN(getinstancevariable) || + opcode == BIN(setinstancevariable) || + opcode == BIN(definedivar)) { + return true; + } + pos = next_pos; + } + return false; +} + static inline int encoded_iseq_trace_instrument(VALUE *iseq_encoded_insn, rb_event_flag_t turnon, bool remain_current_trace) { diff --git a/iseq.h b/iseq.h index a9af64572e..4a45026d79 100644 --- a/iseq.h +++ b/iseq.h @@ -331,6 +331,7 @@ VALUE rb_iseq_defined_string(enum defined_type type); /* vm.c */ VALUE rb_iseq_local_variables(const rb_iseq_t *iseq); +bool rb_vm_iseq_reads_self(const rb_iseq_t *iseq); attr_index_t rb_estimate_iv_count(VALUE klass, const rb_iseq_t * initialize_iseq); diff --git a/vm.c b/vm.c index db168e62c0..8eda685804 100644 --- a/vm.c +++ b/vm.c @@ -1397,7 +1397,7 @@ rb_proc_ractor_make_shareable(VALUE self) rb_proc_t *proc = (rb_proc_t *)RTYPEDDATA_DATA(self); if (proc->block.type != block_type_iseq) rb_raise(rb_eRuntimeError, "not supported yet"); - if (!rb_ractor_shareable_p(vm_block_self(&proc->block))) { + if (rb_vm_iseq_reads_self(iseq) && !rb_ractor_shareable_p(vm_block_self(&proc->block))) { rb_raise(rb_eRactorIsolationError, "Proc's self is not shareable: %" PRIsVALUE, self); -- 2.39.5 (Apple Git-154)