ruby-changes:71467
From: nagachika <ko1@a...>
Date: Mon, 21 Mar 2022 16:53:53 +0900 (JST)
Subject: [ruby-changes:71467] a72b7b898c (ruby_3_0): merge revision(s) d0d6227a0da5925acf946a09191f172daf53baf2,fff1edf23ba28267bf57097c269f7fa87530e3fa: [Backport #17529]
https://git.ruby-lang.org/ruby.git/commit/?id=a72b7b898c From a72b7b898c69a116d754d599e8bb061761015255 Mon Sep 17 00:00:00 2001 From: nagachika <nagachika@r...> Date: Mon, 21 Mar 2022 16:23:30 +0900 Subject: merge revision(s) d0d6227a0da5925acf946a09191f172daf53baf2,fff1edf23ba28267bf57097c269f7fa87530e3fa: [Backport #17529] alen should be actions number on ractor_select() alen was number of rs, but it should be actions number (taking ractors + receiving + yielding). --- ractor.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) fix Ractor.yield(obj, move: true) Ractor.yield(obj, move: true) and Ractor.select(..., yield_value: obj, move: true) tried to yield a value with move semantices, but if the trial is faild, the obj should not become a moved object. To keep this rule, `wait_moving` wait status is introduced. New yield/take process: (1) If a ractor tried to yield (move:true), make taking racotr's wait status `wait_moving` and make a moved object by `ractor_move(obj)` and wakeup taking ractor. (2) If a ractor tried to take a message from a ractor waiting fo yielding (move:true), wakeup the ractor and wait for (1). --- bootstraptest/test_ractor.rb | 25 +++++++++++++++ ractor.c | 73 +++++++++++++++++++++++++++++++++++--------- ractor_core.h | 1 + 3 files changed, 84 insertions(+), 15 deletions(-) --- bootstraptest/test_ractor.rb | 25 ++++++++++++++ ractor.c | 78 ++++++++++++++++++++++++++++++++++---------- ractor_core.h | 1 + version.h | 2 +- 4 files changed, 87 insertions(+), 19 deletions(-) diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index e4e476d560..7d920c31b5 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -760,6 +760,31 @@ assert_equal 'hello', %q{ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_ractor.rb#L760 end } +# yield/move should not make moved object when the yield is not succeeded +assert_equal '"str"', %q{ + R = Ractor.new{} + M = Ractor.current + r = Ractor.new do + s = 'str' + selected_r, v = Ractor.select R, yield_value: s, move: true + raise if selected_r != R # taken from R + M.send s.inspect # s should not be a moved object + end + + Ractor.receive +} + +# yield/move can fail +assert_equal "allocator undefined for Thread", %q{ + r = Ractor.new do + obj = Thread.new{} + Ractor.yield obj + rescue => e + e.message + end + r.take +} + # Access to global-variables are prohibited assert_equal 'can not access global variables $gv from non-main Ractors', %q{ $gv = 1 diff --git a/ractor.c b/ractor.c index de3ab8c16f..bf31b545cf 100644 --- a/ractor.c +++ b/ractor.c @@ -917,7 +917,7 @@ static VALUE ractor_move(VALUE obj); // in this file https://github.com/ruby/ruby/blob/trunk/ractor.c#L917 static VALUE ractor_copy(VALUE obj); // in this file static void -ractor_basket_setup(rb_execution_context_t *ec, struct rb_ractor_basket *basket, VALUE obj, VALUE move, bool exc, bool is_will) +ractor_basket_setup(rb_execution_context_t *ec, struct rb_ractor_basket *basket, VALUE obj, VALUE move, bool exc, bool is_will, bool is_yield) { basket->sender = rb_ec_ractor_ptr(ec)->pub.self; basket->exception = exc; @@ -936,7 +936,13 @@ ractor_basket_setup(rb_execution_context_t *ec, struct rb_ractor_basket *basket, https://github.com/ruby/ruby/blob/trunk/ractor.c#L936 } else { basket->type = basket_type_move; - basket->v = ractor_move(obj); + + if (is_yield) { + basket->v = obj; // call ractor_move() when yielding timing. + } + else { + basket->v = ractor_move(obj); + } } } @@ -944,7 +950,7 @@ static VALUE https://github.com/ruby/ruby/blob/trunk/ractor.c#L950 ractor_send(rb_execution_context_t *ec, rb_ractor_t *r, VALUE obj, VALUE move) { struct rb_ractor_basket basket; - ractor_basket_setup(ec, &basket, obj, move, false, false); + ractor_basket_setup(ec, &basket, obj, move, false, false, false); ractor_send_basket(ec, r, &basket); return r->pub.self; } @@ -959,17 +965,23 @@ ractor_try_take(rb_execution_context_t *ec, rb_ractor_t *r) https://github.com/ruby/ruby/blob/trunk/ractor.c#L965 RACTOR_LOCK(r); { - if (ractor_wakeup(r, wait_yielding, wakeup_by_take)) { + if (ractor_sleeping_by(r, wait_yielding)) { + MAYBE_UNUSED(bool) wakeup_result; VM_ASSERT(r->sync.wait.yielded_basket.type != basket_type_none); - basket = r->sync.wait.yielded_basket; - ractor_basket_clear(&r->sync.wait.yielded_basket); + + if (r->sync.wait.yielded_basket.type == basket_type_move) { + wakeup_result = ractor_wakeup(r, wait_yielding, wakeup_by_retry); + } + else { + wakeup_result = ractor_wakeup(r, wait_yielding, wakeup_by_take); + basket = r->sync.wait.yielded_basket; + ractor_basket_clear(&r->sync.wait.yielded_basket); + } + VM_ASSERT(wakeup_result); } else if (r->sync.outgoing_port_closed) { closed = true; } - else { - // not reached. - } } RACTOR_UNLOCK(r); @@ -986,6 +998,12 @@ ractor_try_take(rb_execution_context_t *ec, rb_ractor_t *r) https://github.com/ruby/ruby/blob/trunk/ractor.c#L998 } } +static VALUE +ractor_yield_move_body(VALUE v) +{ + return ractor_move(v); +} + static bool ractor_try_yield(rb_execution_context_t *ec, rb_ractor_t *cr, struct rb_ractor_basket *basket) { @@ -1010,8 +1028,34 @@ ractor_try_yield(rb_execution_context_t *ec, rb_ractor_t *cr, struct rb_ractor_b https://github.com/ruby/ruby/blob/trunk/ractor.c#L1028 RACTOR_LOCK(r); { - if (ractor_wakeup(r, wait_taking, wakeup_by_yield)) { + if (ractor_sleeping_by(r, wait_taking)) { VM_ASSERT(r->sync.wait.taken_basket.type == basket_type_none); + + if (basket->type == basket_type_move) { + enum ractor_wait_status prev_wait_status = r->sync.wait.status; + r->sync.wait.status = wait_moving; + + RACTOR_UNLOCK(r); + { + int state; + VALUE moved_value = rb_protect(ractor_yield_move_body, basket->v, &state); + if (state) { + r->sync.wait.status = prev_wait_status; + rb_jump_tag(state); + } + else { + basket->v = moved_value; + } + } + RACTOR_LOCK(r); + + if (!ractor_wakeup(r, wait_moving, wakeup_by_yield)) { + // terminating? + } + } + else { + ractor_wakeup(r, wait_taking, wakeup_by_yield); + } r->sync.wait.taken_basket = *basket; } else { @@ -1036,7 +1080,7 @@ ractor_try_yield(rb_execution_context_t *ec, rb_ractor_t *cr, struct rb_ractor_b https://github.com/ruby/ruby/blob/trunk/ractor.c#L1080 // select(r1, r2, r3, receive: true, yield: obj) static VALUE -ractor_select(rb_execution_context_t *ec, const VALUE *rs, int alen, VALUE yielded_value, bool move, VALUE *ret_r) +ractor_select(rb_execution_context_t *ec, const VALUE *rs, const int rs_len, VALUE yielded_value, bool move, VALUE *ret_r) { rb_ractor_t *cr = rb_ec_ractor_ptr(ec); VALUE crv = cr->pub.self; @@ -1045,7 +1089,7 @@ ractor_select(rb_execution_context_t *ec, const VALUE *rs, int alen, VALUE yield https://github.com/ruby/ruby/blob/trunk/ractor.c#L1089 bool interrupted = false; enum ractor_wait_status wait_status = 0; bool yield_p = (yielded_value != Qundef) ? true : false; - const int rs_len = alen; + const int alen = rs_len + (yield_p ? 1 : 0); struct ractor_select_action { enum ractor_select_action_type { @@ -1054,7 +1098,7 @@ ractor_select(rb_execution_context_t *ec, const VALUE *rs, int alen, VALUE yield https://github.com/ruby/ruby/blob/trunk/ractor.c#L1098 ractor_select_action_yield, } type; VALUE v; - } *actions = ALLOCA_N(struct ractor_select_action, alen + (yield_p ? 1 : 0)); + } *actions = ALLOCA_N(struct ractor_select_action, alen); VM_ASSERT(cr->sync.wait.status == wait_none); VM_ASSERT(cr->sync.wait.wakeup_status == wakeup_none); @@ -1062,7 +1106,7 @@ ractor_select(rb_execution_context_t *ec, const VALUE *rs, int alen, VALUE yield https://github.com/ruby/ruby/blob/trunk/ractor.c#L1106 VM_ASSERT(cr->sync.wait.yielded_basket.type == basket_type_none); // setup actions - for (i=0; i<alen; i++) { + for (i=0; i<rs_len; i++) { VALUE v = rs[i]; if (v == crv) { @@ -1087,9 +1131,7 @@ ractor_select(rb_execution_context_t *ec, const VALUE *rs, int alen, VALUE yield https://github.com/ruby/ruby/blob/trunk/ractor.c#L1131 actions[rs_len].type = ractor_select_action_yield; actions[rs_len].v = Qundef; wait_status |= wait_yielding; - alen++; - - ractor_basket_setup(ec, &cr->sync.wait.yielded_basket, yielded_value, move, false, false); + ractor_basket_setup(ec, &cr->sync.wait.yielded_basket, yielded_value, move, false, false, true); } // TODO: shuffle actions @@ -1576,7 +1618,7 @@ ractor_yield_atexit(rb_execution_context_t *ec, rb_ractor_t *cr, VALUE v, bool e https://github.com/ruby/ruby/blob/trunk/ractor.c#L1618 ASSERT_ractor_unlocking(cr); struct rb_ractor_basket basket; - ractor_basket_setup(ec, &basket, v, Qfalse, exc, true); + ractor_basket_setup(ec, &basket, v, Qfalse, exc, true, true /* this flag is ignored because m (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/