Begin the shutdown process before releasing the last waiter.

Once we push the final result on the resume queue the waiting thread may immediately resume, and see a Fiber that appears to be alive but which will actually soon be dead. By moving one of the death conditions before the final push, we will at least ensure the Fiber appears dead. There is an opposite side effect here, in that now a Fiber may appear to be dead a brief moment before it returns its last value, but this can only be observed from a third thread, which could not atomically know that both the final value had returned and the Fiber had declared itself dead. Fixes #4838.
jruby · Nov 21, 2017 · ce61601 · ce61601
1 parent 8d5b2ca
commit ce61601
Showing 1 changed file with 9 additions and 4 deletions.
diff --git a/core/src/main/java/org/jruby/ext/fiber/ThreadFiber.java b/core/src/main/java/org/jruby/ext/fiber/ThreadFiber.java
@@ -275,10 +275,19 @@ public void run() {
                                 result = block.yieldArray(context, init, null);
                             }
 
+                            // Clear ThreadFiber's thread since we're on the way out and need to appear non-alive?
+                            // Waiting thread can proceed immediately after push below but before we are truly dead.
+                            // See https://github.com/jruby/jruby/issues/4838
+                            ThreadFiber tf = data.fiber.get();
+                            if (tf != null) tf.thread = null;
+
                             data.prev.data.queue.push(context, new IRubyObject[]{result});
                         } finally {
+                            // Ensure we do everything for shutdown now
                             data.queue.shutdown();
                             runtime.getThreadService().disposeCurrentThread();
+                            ThreadFiber tf = data.fiber.get();
+                            if (tf != null) tf.thread = null;
                         }
                     } catch (JumpException.FlowControlException fce) {
                         if (data.prev != null) {
@@ -304,10 +313,6 @@ public void run() {
                         if (data.prev != null) {
                             data.prev.thread.raise(JavaUtil.convertJavaToUsableRubyObject(runtime, t));
                         }
-                    } finally {
-                        // clear reference to the fiber's thread
-                        ThreadFiber tf = data.fiber.get();
-                        if (tf != null) tf.thread = null;
                     }
                 }
             });