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

* See https://bugs.ruby-lang.org/issues/19078#note-30

5 files changed, 4 insertions, 33 deletions

diff --git a/cont.c b/cont.c
index 72331fcc93..a1cb4112d2 100644
--- a/cont.c
+++ b/cont.c
@@ -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:
         storage = inherit_fiber_storage();
     }
-    else if (storage == Qfalse) {
-        storage = current_fiber_storage();
-    }
     else /* nil, hash, etc. */ {
         fiber_storage_validate(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
  *    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
  *  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,
  *  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.
  */
 static VALUE
diff --git a/enumerator.c b/enumerator.c
index 5cbaaee04b..a4e9d9a5c8 100644
--- a/enumerator.c
+++ b/enumerator.c
@@ -767,8 +767,7 @@ next_init(VALUE obj, struct enumerator *e)
 {
     VALUE curr = rb_fiber_current();
     e->dst = curr;
-    // We inherit the fiber storage by reference, not by copy, by specifying Qfalse here.
-    e->fib = rb_fiber_new_storage(next_i, obj, Qfalse);
+    e->fib = rb_fiber_new(next_i, obj);
     e->lookahead = Qundef;
 }
 
diff --git a/include/ruby/internal/intern/cont.h b/include/ruby/internal/intern/cont.h
index 1ec78793b7..32647f48aa 100644
--- a/include/ruby/internal/intern/cont.h
+++ b/include/ruby/internal/intern/cont.h
@@ -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
  * rb_fiber_new() which inherits storage from the current fiber.
  *
- * If the given storage is Qfalse, this function uses the current fiber's
- * storage by reference.
- *
- * Specifying either Qtrue or Qfalse is experimental and may be changed in the
- * future.
+ * Specifying Qtrue is experimental and may be changed in the future.
  *
  * If the given storage is Qnil, this function will lazy initialize the
  * internal storage which starts of empty (without any inheritance).
diff --git a/spec/ruby/core/fiber/storage_spec.rb b/spec/ruby/core/fiber/storage_spec.rb
index 4adfd30d2b..c0958d98ee 100644
--- a/spec/ruby/core/fiber/storage_spec.rb
+++ b/spec/ruby/core/fiber/storage_spec.rb
@@ -22,14 +22,6 @@ describe "Fiber.new(storage:)" do
       fiber.resume.should == {life: 42}
     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
       -> { Fiber.new(storage: 42) {} }.should raise_error(TypeError)
     end
diff --git a/test/fiber/test_storage.rb b/test/fiber/test_storage.rb
index 0a6942617c..d5f1f10a68 100644
--- a/test/fiber/test_storage.rb
+++ b/test/fiber/test_storage.rb
@@ -83,13 +83,12 @@ class TestFiberStorage < Test::Unit::TestCase
       Fiber[:count] = 0
 
       enumerator = Enumerator.new do |y|
-        # Since the fiber is implementation detail, the storage are shared with the parent:
         Fiber[:count] += 1
         y << Fiber[:count]
       end
 
       assert_equal 1, enumerator.next
-      assert_equal 1, Fiber[:count]
+      assert_equal 0, Fiber[:count]
     end.resume
   end