[GR-59531] Use Thread#join() to wait for Fibers instead of a CountDow…

…nLatch PullRequest: truffleruby/4386
oracle · Nov 1, 2024 · 07b978e · 07b978e
2 parents fe19a02 + 732fa69
commit 07b978e
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 11 deletions.
diff --git a/src/main/java/org/truffleruby/core/fiber/FiberManager.java b/src/main/java/org/truffleruby/core/fiber/FiberManager.java
@@ -176,6 +176,7 @@ private void afterLeave(RubyFiber fiber) {
             fiber.returnFiber = null;
             fiber.lastMessage = null;
         }
+        fiber.thread = null;
     }
 
     public RubyFiber getReturnFiber(RubyFiber currentFiber, Node currentNode, InlinedBranchProfile errorProfile) {
@@ -351,10 +352,6 @@ public void cleanup(RubyFiber fiber, Thread javaThread) {
         fiber.status = FiberStatus.TERMINATED;
 
         fiber.rubyThread.runningFibers.remove(fiber);
-
-        fiber.thread = null;
-
-        fiber.finishedLatch.countDown();
     }
 
     @TruffleBoundary
@@ -388,9 +385,16 @@ private void doKillOtherFibers(RubyThread thread) throws InterruptedException {
             if (!fiber.isRootFiber()) {
                 addToMessageQueue(fiber, new FiberShutdownMessage());
 
-                // Wait for the Fiber to finish so we only run one Fiber at a time
-                final CountDownLatch finishedLatch = fiber.finishedLatch;
-                finishedLatch.await();
+                /* Wait for the Fiber to finish, so we only run one Fiber at a time. If fiber.thread is null it means
+                 * the Fiber never started and shouldn't ever start since we are killing all fibers of this Ruby thread
+                 * and so no more Ruby code (which could resume that Fiber) should run there. Adding a
+                 * FiberShutdownMessage above won't cause the Fiber to start, because the only thing causing the Fiber
+                 * to create a thread are callers of resumeAndWait(), which are Fiber#resume, Fiber#transfer,
+                 * Fiber#raise and Fiber.yield. */
+                Thread fiberThread = fiber.thread;
+                if (fiberThread != null) {
+                    fiberThread.join();
+                }
 
                 final Throwable uncaughtException = fiber.uncaughtException;
                 if (uncaughtException != null) {

diff --git a/src/main/java/org/truffleruby/core/fiber/RubyFiber.java b/src/main/java/org/truffleruby/core/fiber/RubyFiber.java
@@ -80,7 +80,6 @@ public enum FiberStatus {
     public final RubyBasicObject fiberLocals;
     public final RubyArray catchTags;
     public final CountDownLatch initializedLatch = new CountDownLatch(1);
-    public CountDownLatch finishedLatch = new CountDownLatch(1);
     final BlockingQueue<FiberManager.FiberMessage> messageQueue = newMessageQueue();
     public final RubyThread rubyThread;
     // @formatter:off

diff --git a/src/main/java/org/truffleruby/core/thread/ThreadManager.java b/src/main/java/org/truffleruby/core/thread/ThreadManager.java
@@ -134,7 +134,6 @@ public void restartMainThread(Thread mainJavaThread) {
 
         final RubyFiber rootFiber = rootThread.getRootFiber();
         rootFiber.restart();
-        rootFiber.finishedLatch = new CountDownLatch(1);
 
         PRNGRandomizerNodes.resetSeed(context, rootThread.randomizer);
     }
@@ -200,8 +199,6 @@ private static Thread.UncaughtExceptionHandler uncaughtExceptionHandler(RubyFibe
 
                 // the Fiber is not yet initialized, unblock the caller and rethrow the exception to it
                 fiber.initializedLatch.countDown();
-                // the Fiber thread is dying, unblock the caller
-                fiber.finishedLatch.countDown();
             } catch (Throwable t) { // exception inside this UncaughtExceptionHandler
                 t.initCause(throwable);
                 printInternalError(t);
@@ -423,6 +420,7 @@ public void cleanupThreadState(RubyThread thread, Thread javaThread) {
 
         unregisterThread(thread);
         thread.thread = null;
+        thread.getRootFiber().thread = null;
 
         if (Thread.currentThread() == javaThread) {
             for (ReentrantLock lock : thread.ownedLocks) {