Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions src/main/java/org/truffleruby/core/fiber/FiberManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
1 change: 0 additions & 1 deletion src/main/java/org/truffleruby/core/fiber/RubyFiber.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/org/truffleruby/core/thread/ThreadManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
Loading