diff options
| -rw-r--r-- | cont.c | 102 | ||||
| -rw-r--r-- | spec/ruby/core/fiber/resume_spec.rb | 37 | ||||
| -rw-r--r-- | spec/ruby/library/fiber/current_spec.rb | 16 | ||||
| -rw-r--r-- | spec/ruby/library/fiber/resume_spec.rb | 21 | ||||
| -rw-r--r-- | spec/ruby/library/fiber/transfer_spec.rb | 56 | ||||
| -rw-r--r-- | spec/ruby/shared/fiber/resume.rb | 21 | ||||
| -rw-r--r-- | test/ruby/test_fiber.rb | 103 |
7 files changed, 267 insertions, 89 deletions
@@ -208,6 +208,7 @@ typedef struct rb_context_struct { /* * Fiber status: + * Fiber status: * [Fiber.new] ------> FIBER_CREATED * | [Fiber#resume] * v @@ -235,14 +236,11 @@ struct rb_fiber_struct { rb_context_t cont; VALUE first_proc; struct rb_fiber_struct *prev; - BITFIELD(enum fiber_status, status, 2); - /* If a fiber invokes by "transfer", - * then this fiber can't be invoked by "resume" any more after that. - * You shouldn't mix "transfer" and "resume". - */ - unsigned int transferred : 1; + VALUE resuming_fiber; + BITFIELD(enum fiber_status, status, 2); /* Whether the fiber is allowed to implicitly yield. */ + unsigned int yielding : 1; unsigned int blocking : 1; struct coroutine_context context; @@ -919,11 +917,13 @@ cont_mark(void *ptr) RUBY_MARK_LEAVE("cont"); } +#if 0 static int fiber_is_root_p(const rb_fiber_t *fiber) { return fiber == fiber->cont.saved_ec.thread_ptr->root_fiber; } +#endif static void cont_free(void *ptr) @@ -2010,25 +2010,33 @@ fiber_current(void) } static inline rb_fiber_t* -return_fiber(void) +return_fiber(bool terminate) { rb_fiber_t *fiber = fiber_current(); rb_fiber_t *prev = fiber->prev; - if (!prev) { + if (prev) { + fiber->prev = NULL; + prev->resuming_fiber = Qnil; + return prev; + } + else { + if (!terminate) { + rb_raise(rb_eFiberError, "attempt to yield on a not resumed fiber"); + } + rb_thread_t *th = GET_THREAD(); rb_fiber_t *root_fiber = th->root_fiber; VM_ASSERT(root_fiber != NULL); - if (root_fiber == fiber) { - rb_raise(rb_eFiberError, "can't yield from root fiber"); + // search resuming fiber + for (fiber = root_fiber; + RTEST(fiber->resuming_fiber); + fiber = fiber_ptr(fiber->resuming_fiber)) { } - return root_fiber; - } - else { - fiber->prev = NULL; - return prev; + + return fiber; } } @@ -2073,7 +2081,7 @@ fiber_store(rb_fiber_t *next_fiber, rb_thread_t *th) } static inline VALUE -fiber_switch(rb_fiber_t *fiber, int argc, const VALUE *argv, int is_resume, int kw_splat) +fiber_switch(rb_fiber_t *fiber, int argc, const VALUE *argv, int kw_splat, VALUE resuming_fiber, bool yielding) { VALUE value; rb_context_t *cont = &fiber->cont; @@ -2120,11 +2128,21 @@ fiber_switch(rb_fiber_t *fiber, int argc, const VALUE *argv, int is_resume, int VM_ASSERT(FIBER_RUNNABLE_P(fiber)); - if (is_resume) { + rb_fiber_t *current_fiber = fiber_current(); + + VM_ASSERT(!RTEST(current_fiber->resuming_fiber)); + if (RTEST(resuming_fiber)) { + current_fiber->resuming_fiber = resuming_fiber; fiber->prev = fiber_current(); + fiber->yielding = 0; } - if (fiber_current()->blocking) { + VM_ASSERT(!current_fiber->yielding); + if (yielding) { + current_fiber->yielding = 1; + } + + if (current_fiber->blocking) { th->blocking -= 1; } @@ -2134,7 +2152,7 @@ fiber_switch(rb_fiber_t *fiber, int argc, const VALUE *argv, int is_resume, int value = fiber_store(fiber, th); - if (is_resume && FIBER_TERMINATED_P(fiber)) { + if (RTEST(resuming_fiber) && FIBER_TERMINATED_P(fiber)) { fiber_stack_release(fiber); } @@ -2152,7 +2170,7 @@ fiber_switch(rb_fiber_t *fiber, int argc, const VALUE *argv, int is_resume, int VALUE rb_fiber_transfer(VALUE fiber_value, int argc, const VALUE *argv) { - return fiber_switch(fiber_ptr(fiber_value), argc, argv, 0, RB_NO_KEYWORDS); + return fiber_switch(fiber_ptr(fiber_value), argc, argv, RB_NO_KEYWORDS, Qfalse, false); } VALUE @@ -2181,29 +2199,38 @@ rb_fiber_terminate(rb_fiber_t *fiber, int need_interrupt) fiber->cont.machine.stack = NULL; fiber->cont.machine.stack_size = 0; - next_fiber = return_fiber(); + next_fiber = return_fiber(true); if (need_interrupt) RUBY_VM_SET_INTERRUPT(&next_fiber->cont.saved_ec); - fiber_switch(next_fiber, 1, &value, 0, RB_NO_KEYWORDS); + fiber_switch(next_fiber, 1, &value, RB_NO_KEYWORDS, Qfalse, false); } VALUE rb_fiber_resume_kw(VALUE fiber_value, int argc, const VALUE *argv, int kw_splat) { rb_fiber_t *fiber = fiber_ptr(fiber_value); + rb_fiber_t *current_fiber = fiber_current(); if (argc == -1 && FIBER_CREATED_P(fiber)) { rb_raise(rb_eFiberError, "cannot raise exception on unborn fiber"); } - - if (fiber->prev != 0 || fiber_is_root_p(fiber)) { - rb_raise(rb_eFiberError, "double resume"); + else if (FIBER_TERMINATED_P(fiber)) { + rb_raise(rb_eFiberError, "attempt to resume a terminated fiber"); } - - if (fiber->transferred != 0) { - rb_raise(rb_eFiberError, "cannot resume transferred Fiber"); + else if (fiber == current_fiber) { + rb_raise(rb_eFiberError, "attempt to resume the current fiber"); + } + else if (fiber->prev != NULL) { + rb_raise(rb_eFiberError, "attempt to resume a resumed fiber (double resume)"); + } + else if (RTEST(fiber->resuming_fiber)) { + rb_raise(rb_eFiberError, "attempt to resume a resuming fiber"); + } + else if (fiber->prev == NULL && + (!fiber->yielding && fiber->status != FIBER_CREATED)) { + rb_raise(rb_eFiberError, "attempt to resume a transferring fiber"); } - return fiber_switch(fiber, argc, argv, 1, kw_splat); + return fiber_switch(fiber, argc, argv, kw_splat, fiber_value, false); } VALUE @@ -2215,13 +2242,13 @@ rb_fiber_resume(VALUE fiber_value, int argc, const VALUE *argv) VALUE rb_fiber_yield_kw(int argc, const VALUE *argv, int kw_splat) { - return fiber_switch(return_fiber(), argc, argv, 0, kw_splat); + return fiber_switch(return_fiber(false), argc, argv, kw_splat, Qfalse, true); } VALUE rb_fiber_yield(int argc, const VALUE *argv) { - return fiber_switch(return_fiber(), argc, argv, 0, RB_NO_KEYWORDS); + return fiber_switch(return_fiber(false), argc, argv, RB_NO_KEYWORDS, Qfalse, true); } void @@ -2362,8 +2389,13 @@ static VALUE rb_fiber_m_transfer(int argc, VALUE *argv, VALUE fiber_value) { rb_fiber_t *fiber = fiber_ptr(fiber_value); - fiber->transferred = 1; - return fiber_switch(fiber, argc, argv, 0, rb_keyword_given_p()); + if (RTEST(fiber->resuming_fiber)) { + rb_raise(rb_eFiberError, "attempt to transfer to a resuming fiber"); + } + if (fiber->yielding) { + rb_raise(rb_eFiberError, "attempt to transfer to a yielding fiber"); + } + return fiber_switch(fiber, argc, argv, rb_keyword_given_p(), Qfalse, false); } /* @@ -2411,8 +2443,8 @@ fiber_to_s(VALUE fiber_value) const rb_proc_t *proc; char status_info[0x20]; - if (fiber->transferred) { - snprintf(status_info, 0x20, " (%s, transferred)", fiber_status_name(fiber->status)); + if (RTEST(fiber->resuming_fiber)) { + snprintf(status_info, 0x20, " (%s by resuming)", fiber_status_name(fiber->status)); } else { snprintf(status_info, 0x20, " (%s)", fiber_status_name(fiber->status)); diff --git a/spec/ruby/core/fiber/resume_spec.rb b/spec/ruby/core/fiber/resume_spec.rb index 97495c5059..273bc866af 100644 --- a/spec/ruby/core/fiber/resume_spec.rb +++ b/spec/ruby/core/fiber/resume_spec.rb @@ -6,9 +6,40 @@ describe "Fiber#resume" do end describe "Fiber#resume" do - it "raises a FiberError if the Fiber tries to resume itself" do - fiber = Fiber.new { fiber.resume } - -> { fiber.resume }.should raise_error(FiberError, /double resume/) + it "runs until Fiber.yield" do + obj = mock('obj') + obj.should_not_receive(:do) + fiber = Fiber.new { 1 + 2; Fiber.yield; obj.do } + fiber.resume + end + + it "resumes from the last call to Fiber.yield on subsequent invocations" do + fiber = Fiber.new { Fiber.yield :first; :second } + fiber.resume.should == :first + fiber.resume.should == :second + end + + it "sets the block parameters to its arguments on the first invocation" do + first = mock('first') + first.should_receive(:arg).with(:first).twice + + fiber = Fiber.new { |arg| first.arg arg; Fiber.yield; first.arg arg; } + fiber.resume :first + fiber.resume :second + end + + ruby_version_is '3.0' do + it "raises a FiberError if the Fiber tries to resume itself" do + fiber = Fiber.new { fiber.resume } + -> { fiber.resume }.should raise_error(FiberError, /current fiber/) + end + end + + ruby_version_is '' ... '3.0' do + it "raises a FiberError if the Fiber tries to resume itself" do + fiber = Fiber.new { fiber.resume } + -> { fiber.resume }.should raise_error(FiberError, /double resume/) + end end it "returns control to the calling Fiber if called from one" do diff --git a/spec/ruby/library/fiber/current_spec.rb b/spec/ruby/library/fiber/current_spec.rb index 52dff3dea1..e67d7d050a 100644 --- a/spec/ruby/library/fiber/current_spec.rb +++ b/spec/ruby/library/fiber/current_spec.rb @@ -42,10 +42,22 @@ describe "Fiber.current" do fiber3 = Fiber.new do states << :fiber3 fiber2.transfer - flunk + ruby_version_is '3.0' do + states << :fiber3_terminated + end + ruby_version_is '' ... '3.0' do + flunk + end end fiber3.resume - states.should == [:fiber3, :fiber2, :fiber] + + ruby_version_is "" ... "3.0" do + states.should == [:fiber3, :fiber2, :fiber] + end + + ruby_version_is "3.0" do + states.should == [:fiber3, :fiber2, :fiber, :fiber3_terminated] + end end end diff --git a/spec/ruby/library/fiber/resume_spec.rb b/spec/ruby/library/fiber/resume_spec.rb index dae717c8a1..39d14bdda0 100644 --- a/spec/ruby/library/fiber/resume_spec.rb +++ b/spec/ruby/library/fiber/resume_spec.rb @@ -3,10 +3,21 @@ require_relative '../../spec_helper' require 'fiber' describe "Fiber#resume" do - it "raises a FiberError if the Fiber has transferred control to another Fiber" do - fiber1 = Fiber.new { true } - fiber2 = Fiber.new { fiber1.transfer; Fiber.yield } - fiber2.resume - -> { fiber2.resume }.should raise_error(FiberError) + ruby_version_is '' ... '3.0' do + it "raises a FiberError if the Fiber has transferred control to another Fiber" do + fiber1 = Fiber.new { true } + fiber2 = Fiber.new { fiber1.transfer; Fiber.yield } + fiber2.resume + -> { fiber2.resume }.should raise_error(FiberError) + end + end + + ruby_version_is '3.0' do + it "can work with Fiber#transfer" do + fiber1 = Fiber.new { true } + fiber2 = Fiber.new { fiber1.transfer; Fiber.yield 10 ; Fiber.yield 20; raise } + fiber2.resume.should == 10 + fiber2.resume.should == 20 + end end end diff --git a/spec/ruby/library/fiber/transfer_spec.rb b/spec/ruby/library/fiber/transfer_spec.rb index d13053666c..7af548da1a 100644 --- a/spec/ruby/library/fiber/transfer_spec.rb +++ b/spec/ruby/library/fiber/transfer_spec.rb @@ -11,7 +11,13 @@ describe "Fiber#transfer" do it "transfers control from one Fiber to another when called from a Fiber" do fiber1 = Fiber.new { :fiber1 } fiber2 = Fiber.new { fiber1.transfer; :fiber2 } - fiber2.resume.should == :fiber1 + + ruby_version_is '' ... '3.0' do + fiber2.resume.should == :fiber1 + end + ruby_version_is '3.0' do + fiber2.resume.should == :fiber2 + end end it "returns to the root Fiber when finished" do @@ -34,12 +40,24 @@ describe "Fiber#transfer" do states.should == [:start, :end] end - it "can transfer control to a Fiber that has transferred to another Fiber" do - states = [] - fiber1 = Fiber.new { states << :fiber1 } - fiber2 = Fiber.new { states << :fiber2_start; fiber1.transfer; states << :fiber2_end} - fiber2.resume.should == [:fiber2_start, :fiber1] - fiber2.transfer.should == [:fiber2_start, :fiber1, :fiber2_end] + ruby_version_is '' ... '3.0' do + it "can transfer control to a Fiber that has transferred to another Fiber" do + states = [] + fiber1 = Fiber.new { states << :fiber1 } + fiber2 = Fiber.new { states << :fiber2_start; fiber1.transfer; states << :fiber2_end} + fiber2.resume.should == [:fiber2_start, :fiber1] + fiber2.transfer.should == [:fiber2_start, :fiber1, :fiber2_end] + end + end + + ruby_version_is '3.0' do + it "can not transfer control to a Fiber that has suspended by Fiber.yield" do + states = [] + fiber1 = Fiber.new { states << :fiber1 } + fiber2 = Fiber.new { states << :fiber2_start; Fiber.yield fiber1.transfer; states << :fiber2_end} + fiber2.resume.should == [:fiber2_start, :fiber1] + -> { fiber2.transfer }.should raise_error(FiberError) + end end it "raises a FiberError when transferring to a Fiber which resumes itself" do @@ -83,4 +101,28 @@ describe "Fiber#transfer" do thread.join states.should == [0, 1, 2, 3] end + + ruby_version_is "" ... "3.0" do + it "runs until Fiber.yield" do + obj = mock('obj') + obj.should_not_receive(:do) + fiber = Fiber.new { 1 + 2; Fiber.yield; obj.do } + fiber.transfer + end + + it "resumes from the last call to Fiber.yield on subsequent invocations" do + fiber = Fiber.new { Fiber.yield :first; :second } + fiber.transfer.should == :first + fiber.transfer.should == :second + end + + it "sets the block parameters to its arguments on the first invocation" do + first = mock('first') + first.should_receive(:arg).with(:first).twice + + fiber = Fiber.new { |arg| first.arg arg; Fiber.yield; first.arg arg; } + fiber.transfer :first + fiber.transfer :second + end + end end diff --git a/spec/ruby/shared/fiber/resume.rb b/spec/ruby/shared/fiber/resume.rb index d3dc438ae2..f3477804ad 100644 --- a/spec/ruby/shared/fiber/resume.rb +++ b/spec/ruby/shared/fiber/resume.rb @@ -35,32 +35,11 @@ describe :fiber_resume, shared: true do fiber.send(@method) end - it "runs until Fiber.yield" do - obj = mock('obj') - obj.should_not_receive(:do) - fiber = Fiber.new { 1 + 2; Fiber.yield; obj.do } - fiber.send(@method) - end - - it "resumes from the last call to Fiber.yield on subsequent invocations" do - fiber = Fiber.new { Fiber.yield :first; :second } - fiber.send(@method).should == :first - fiber.send(@method).should == :second - end - it "accepts any number of arguments" do fiber = Fiber.new { |a| } -> { fiber.send(@method, *(1..10).to_a) }.should_not raise_error end - it "sets the block parameters to its arguments on the first invocation" do - first = mock('first') - first.should_receive(:arg).with(:first).twice - fiber = Fiber.new { |arg| first.arg arg; Fiber.yield; first.arg arg; } - fiber.send(@method, :first) - fiber.send(@method, :second) - end - it "raises a FiberError if the Fiber is dead" do fiber = Fiber.new { true } fiber.send(@method) diff --git a/test/ruby/test_fiber.rb b/test/ruby/test_fiber.rb index 4d103a7f76..dc46b558f0 100644 --- a/test/ruby/test_fiber.rb +++ b/test/ruby/test_fiber.rb @@ -184,6 +184,34 @@ class TestFiber < Test::Unit::TestCase assert_equal([:baz], ary) end + def test_terminate_transferred_fiber + root_fiber = Fiber.current + log = [] + fa1 = fa2 = fb1 = r1 = nil + + fa1 = Fiber.new{ + fa2 = Fiber.new{ + log << :fa2_terminate + } + fa2.resume + log << :fa1_terminate + } + fb1 = Fiber.new{ + fa1.transfer + log << :fb1_terminate + } + + r1 = Fiber.new{ + fb1.transfer + log << :r1_terminate + } + + r1.resume + log << :root_terminate + + assert_equal [:fa2_terminate, :fa1_terminate, :r1_terminate, :root_terminate], log + end + def test_tls # def tvar(var, val) @@ -278,29 +306,72 @@ class TestFiber < Test::Unit::TestCase assert_instance_of(Class, Fiber.new(&Class.new.method(:undef_method)).resume(:to_s), bug5083) end - def test_prohibit_resume_transferred_fiber + def test_prohibit_transfer_to_resuming_fiber + root_fiber = Fiber.current + assert_raise(FiberError){ - root_fiber = Fiber.current - f = Fiber.new{ - root_fiber.transfer - } - f.transfer - f.resume + fiber = Fiber.new{ root_fiber.transfer } + fiber.resume + } + + fa1 = Fiber.new{ + fa2 = Fiber.new{ root_fiber.transfer } + } + fb1 = Fiber.new{ + fb2 = Fiber.new{ root_fiber.transfer } + } + fa1.transfer + fb1.transfer + + assert_raise(FiberError){ + fa1.transfer } assert_raise(FiberError){ - g=nil - f=Fiber.new{ - g.resume - g.resume - } - g=Fiber.new{ - f.resume - f.resume + fb1.transfer + } + end + + def test_prohibit_transfer_to_yielding_fiber + root_fiber = Fiber.current + f1 = f2 = f3 = nil + + f1 = Fiber.new{ + f2 = Fiber.new{ + f3 = Fiber.new{ + p f3: Fiber.yield + } + f3.resume } - f.transfer + f2.resume } + f1.resume + + assert_raise(FiberError){ f3.transfer 10 } end + def test_prohibit_resume_to_transferring_fiber + root_fiber = Fiber.current + + assert_raise(FiberError){ + Fiber.new{ + root_fiber.resume + }.transfer + } + + f1 = f2 = nil + f1 = Fiber.new do + f2.transfer + end + f2 = Fiber.new do + f1.resume # attempt to resume transferring fiber + end + + assert_raise(FiberError){ + f1.transfer + } + end + + def test_fork_from_fiber skip 'fork not supported' unless Process.respond_to?(:fork) pid = nil |