Never use the storage of another Fiber, that violates the whole design

* See https://bugs.ruby-lang.org/issues/19078#note-30
This commit is contained in:
Benoit Daloze 2022-12-20 17:59:46 +01:00
parent 88040063d0
commit 45175962a6
Notes: git 2022-12-20 18:32:46 +00:00
5 changed files with 4 additions and 33 deletions

17
cont.c
View File

@ -2185,9 +2185,6 @@ fiber_initialize(VALUE self, VALUE proc, struct fiber_pool * fiber_pool, unsigne
// The default, inherit storage (dup) from the current fiber: // The default, inherit storage (dup) from the current fiber:
storage = inherit_fiber_storage(); storage = inherit_fiber_storage();
} }
else if (storage == Qfalse) {
storage = current_fiber_storage();
}
else /* nil, hash, etc. */ { else /* nil, hash, etc. */ {
fiber_storage_validate(storage); fiber_storage_validate(storage);
storage = rb_obj_dup(storage); storage = rb_obj_dup(storage);
@ -2299,18 +2296,6 @@ rb_fiber_initialize_kw(int argc, VALUE* argv, VALUE self, int kw_splat)
* end.resume * end.resume
* Fiber[:x] # => 1 * Fiber[:x] # => 1
* *
* If the <tt>storage</tt> is <tt>false</tt>, this function uses the current
* fiber's storage by reference. This is used for Enumerator to create
* hidden fiber.
*
* Fiber[:count] = 0
* enumerator = Enumerator.new do |y|
* loop{y << (Fiber[:count] += 1)}
* end
* Fiber[:count] # => 0
* enumerator.next # => 1
* Fiber[:count] # => 1
*
* If the given <tt>storage</tt> is <tt>nil</tt>, this function will lazy * If the given <tt>storage</tt> is <tt>nil</tt>, this function will lazy
* initialize the internal storage, which starts as an empty hash. * initialize the internal storage, which starts as an empty hash.
* *
@ -2322,7 +2307,7 @@ rb_fiber_initialize_kw(int argc, VALUE* argv, VALUE self, int kw_splat)
* Otherwise, the given <tt>storage</tt> is used as the new fiber's storage, * Otherwise, the given <tt>storage</tt> is used as the new fiber's storage,
* and it must be an instance of Hash. * and it must be an instance of Hash.
* *
* Explicitly using `storage: true/false` is currently experimental and may * Explicitly using `storage: true` is currently experimental and may
* change in the future. * change in the future.
*/ */
static VALUE static VALUE

View File

@ -767,8 +767,7 @@ next_init(VALUE obj, struct enumerator *e)
{ {
VALUE curr = rb_fiber_current(); VALUE curr = rb_fiber_current();
e->dst = curr; e->dst = curr;
// We inherit the fiber storage by reference, not by copy, by specifying Qfalse here. e->fib = rb_fiber_new(next_i, obj);
e->fib = rb_fiber_new_storage(next_i, obj, Qfalse);
e->lookahead = Qundef; e->lookahead = Qundef;
} }

View File

@ -45,11 +45,7 @@ VALUE rb_fiber_new(rb_block_call_func_t func, VALUE callback_obj);
* If the given storage is Qundef or Qtrue, this function is equivalent to * If the given storage is Qundef or Qtrue, this function is equivalent to
* rb_fiber_new() which inherits storage from the current fiber. * rb_fiber_new() which inherits storage from the current fiber.
* *
* If the given storage is Qfalse, this function uses the current fiber's * Specifying Qtrue is experimental and may be changed in the future.
* storage by reference.
*
* Specifying either Qtrue or Qfalse is experimental and may be changed in the
* future.
* *
* If the given storage is Qnil, this function will lazy initialize the * If the given storage is Qnil, this function will lazy initialize the
* internal storage which starts of empty (without any inheritance). * internal storage which starts of empty (without any inheritance).

View File

@ -22,14 +22,6 @@ describe "Fiber.new(storage:)" do
fiber.resume.should == {life: 42} fiber.resume.should == {life: 42}
end end
it "creates a fiber with a reference to the storage of the parent fiber" do
fiber = Fiber.new(storage: {life: 42}) do
Fiber.new(storage: false) { Fiber[:life] = 43 }.resume
Fiber.current.storage
end
fiber.resume.should == {life: 43}
end
it "cannot create a fiber with non-hash storage" do it "cannot create a fiber with non-hash storage" do
-> { Fiber.new(storage: 42) {} }.should raise_error(TypeError) -> { Fiber.new(storage: 42) {} }.should raise_error(TypeError)
end end

View File

@ -83,13 +83,12 @@ class TestFiberStorage < Test::Unit::TestCase
Fiber[:count] = 0 Fiber[:count] = 0
enumerator = Enumerator.new do |y| enumerator = Enumerator.new do |y|
# Since the fiber is implementation detail, the storage are shared with the parent:
Fiber[:count] += 1 Fiber[:count] += 1
y << Fiber[:count] y << Fiber[:count]
end end
assert_equal 1, enumerator.next assert_equal 1, enumerator.next
assert_equal 1, Fiber[:count] assert_equal 0, Fiber[:count]
end.resume end.resume
end end