From eb0c33d730c32de86e9e339077de733ca590fb91 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Wed, 6 Oct 2021 12:38:12 -0400 Subject: [PATCH] Fix Ractor.make_shareable changing locals for Procs env_copy() uses rb_ary_delete_at() with a loop counting up while iterating through the list of read only locals. rb_ary_delete_at() can shift elements in the array to an index lesser than the loop index, causing locals to be missed and being set to Qfalse in the returned environment. Iterate through the list of locals in reverse instead, this way we always remove the last element and not cause elements in the array to shift. This ensures that all read only locals are handled. [Bug #18023] --- bootstraptest/test_ractor.rb | 22 ++++++++++++++++++++++ vm.c | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index 01d02dce775e8b..4da348df67e120 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -187,6 +187,28 @@ }.map(&:take) } +# Ractor.make_shareable issue for locals in proc [Bug #18023] +assert_equal '[:a, :b, :c, :d, :e]', %q{ + v1, v2, v3, v4, v5 = :a, :b, :c, :d, :e + closure = Proc.new { [v1, v2, v3, v4, v5] } + + Ractor.make_shareable(closure).call +} + +# Ractor.make_shareable issue for locals in proc [Bug #18023] +assert_equal '[:a, :b, :c, :d, :e, :f, :g]', %q{ + a = :a + closure = -> { + b, c, d = :b, :c, :d + -> { + e, f, g = :e, :f, :g + -> { [a, b, c, d, e, f, g] } + }.call + }.call + + Ractor.make_shareable(closure).call +} + ### ### # Ractor still has several memory corruption so skip huge number of tests diff --git a/vm.c b/vm.c index 5a1756e33a3689..d5e984ce61abc3 100644 --- a/vm.c +++ b/vm.c @@ -1006,7 +1006,7 @@ env_copy(const VALUE *src_ep, VALUE read_only_variables) volatile VALUE prev_env = Qnil; if (read_only_variables) { - for (int i=0; i=0; i--) { ID id = SYM2ID(rb_str_intern(RARRAY_AREF(read_only_variables, i))); for (unsigned int j=0; jiseq->body->local_table_size; j++) {