From 12409ad28cf909f84ccae8096bba2beb442266df Mon Sep 17 00:00:00 2001
From: normal <normal@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>
Date: Wed, 12 Sep 2018 20:49:24 +0000
Subject: [PATCH] fiber: fix crash on GC after forking

Remove the remainder of ROOT_FIBER_CONTEXT use and unnecessary
differences between the root and non-root fiber.  This makes
it easier to follow new root fiber at fork time.

Multiple sources of truth often leads to bugs, as in this case.
We can determinte root fiber by checking a fiber against the root_fiber
of its owner thread.  The new `fiber_is_root_p' function
supports that.

Now, we can care only about free-ing/recycling/munmap-ing stacks
as appropriate.

[Bug #15050]

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@64706 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
---
 cont.c                  | 29 +++++++++++++----------------
 test/ruby/test_fiber.rb | 23 +++++++++++++++--------
 2 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/cont.c b/cont.c
index 27f956f5c8..95f2de9b87 100644
--- a/cont.c
+++ b/cont.c
@@ -77,8 +77,7 @@ static long pagesize;
 
 enum context_type {
     CONTINUATION_CONTEXT = 0,
-    FIBER_CONTEXT = 1,
-    ROOT_FIBER_CONTEXT = 2
+    FIBER_CONTEXT = 1
 };
 
 struct cont_saved_vm_stack {
@@ -360,6 +359,12 @@ cont_mark(void *ptr)
     RUBY_MARK_LEAVE("cont");
 }
 
+static int
+fiber_is_root_p(const rb_fiber_t *fib)
+{
+    return fib == fib->cont.saved_ec.thread_ptr->root_fiber;
+}
+
 static void
 cont_free(void *ptr)
 {
@@ -378,24 +383,17 @@ cont_free(void *ptr)
 	/* fiber */
 	const rb_fiber_t *fib = (rb_fiber_t*)cont;
 #ifdef _WIN32
-	if (cont->type != ROOT_FIBER_CONTEXT) {
+        if (!fiber_is_root_p(fib)) {
 	    /* don't delete root fiber handle */
 	    if (fib->fib_handle) {
 		DeleteFiber(fib->fib_handle);
 	    }
 	}
 #else /* not WIN32 */
+        /* fib->ss_sp == NULL is possible for root fiber */
 	if (fib->ss_sp != NULL) {
-	    if (cont->type == ROOT_FIBER_CONTEXT) {
-		rb_bug("Illegal root fiber parameter");
-	    }
 	    munmap((void*)fib->ss_sp, fib->ss_size);
 	}
-	else {
-	    /* It may reached here when finalize */
-	    /* TODO examine whether it is a bug */
-	    /* rb_bug("cont_free: release self"); */
-	}
 #endif
     }
 #else /* not FIBER_USE_NATIVE */
@@ -1526,7 +1524,7 @@ root_fiber_alloc(rb_thread_t *th)
     rb_fiber_t *fib = th->ec->fiber_ptr;
 
     VM_ASSERT(DATA_PTR(fibval) == NULL);
-    VM_ASSERT(fib->cont.type == ROOT_FIBER_CONTEXT);
+    VM_ASSERT(fib->cont.type == FIBER_CONTEXT);
     VM_ASSERT(fib->status == FIBER_RESUMED);
 
     th->root_fiber = fib;
@@ -1555,7 +1553,7 @@ rb_threadptr_root_fiber_setup(rb_thread_t *th)
 {
     rb_fiber_t *fib = ruby_mimmalloc(sizeof(rb_fiber_t));
     MEMZERO(fib, rb_fiber_t, 1);
-    fib->cont.type = ROOT_FIBER_CONTEXT;
+    fib->cont.type = FIBER_CONTEXT;
     fib->cont.saved_ec.fiber_ptr = fib;
     fib->cont.saved_ec.thread_ptr = th;
     fiber_status_set(fib, FIBER_RESUMED); /* skip CREATED */
@@ -1571,7 +1569,7 @@ rb_threadptr_root_fiber_release(rb_thread_t *th)
 	/* ignore. A root fiber object will free th->ec */
     }
     else {
-	VM_ASSERT(th->ec->fiber_ptr->cont.type == ROOT_FIBER_CONTEXT);
+	VM_ASSERT(th->ec->fiber_ptr->cont.type == FIBER_CONTEXT);
 	VM_ASSERT(th->ec->fiber_ptr->cont.self == 0);
 	fiber_free(th->ec->fiber_ptr);
 
@@ -1818,7 +1816,7 @@ rb_fiber_resume(VALUE fibval, int argc, const VALUE *argv)
 {
     rb_fiber_t *fib = fiber_ptr(fibval);
 
-    if (fib->prev != 0 || fib->cont.type == ROOT_FIBER_CONTEXT) {
+    if (fib->prev != 0 || fiber_is_root_p(fib)) {
 	rb_raise(rb_eFiberError, "double resume");
     }
     if (fib->transferred != 0) {
@@ -1997,7 +1995,6 @@ rb_fiber_atfork(rb_thread_t *th)
     if (th->root_fiber) {
         if (&th->root_fiber->cont.saved_ec != th->ec) {
             th->root_fiber = th->ec->fiber_ptr;
-            th->root_fiber->cont.type = ROOT_FIBER_CONTEXT;
         }
         th->root_fiber->prev = 0;
     }
diff --git a/test/ruby/test_fiber.rb b/test/ruby/test_fiber.rb
index c835246fda..1729147d5e 100644
--- a/test/ruby/test_fiber.rb
+++ b/test/ruby/test_fiber.rb
@@ -260,18 +260,25 @@ class TestFiber < Test::Unit::TestCase
   end
 
   def test_fork_from_fiber
-    begin
-      pid = Process.fork{}
-    rescue NotImplementedError
-      return
-    else
-      Process.wait(pid)
-    end
+    skip 'fork not supported' unless Process.respond_to?(:fork)
+    pid = nil
     bug5700 = '[ruby-core:41456]'
     assert_nothing_raised(bug5700) do
       Fiber.new do
         pid = fork do
-          Fiber.new {}.transfer
+          xpid = nil
+          Fiber.new {
+            xpid = fork do
+              # enough to trigger GC on old root fiber
+              10000.times do
+                Fiber.new {}.transfer
+                Fiber.new { Fiber.yield }
+              end
+              exit!(0)
+            end
+          }.transfer
+          _, status = Process.waitpid2(xpid)
+          exit!(status.success?)
         end
       end.resume
     end