Skip to content

Commit 5f57e95

Browse files
committed
[GR-33681] [GR-33546] Backport: Fix setting the current RubyThread for fibers before TruffleContext#enter() which polls
PullRequest: truffleruby/2962
2 parents cf2bbaa + 162336c commit 5f57e95

File tree

5 files changed

+46
-12
lines changed

5 files changed

+46
-12
lines changed

src/main/java/org/truffleruby/RubyLanguage.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import org.truffleruby.core.exception.RubySyntaxError;
5252
import org.truffleruby.core.exception.RubySystemCallError;
5353
import org.truffleruby.core.exception.RubySystemExit;
54+
import org.truffleruby.core.fiber.FiberPoolThread;
5455
import org.truffleruby.core.fiber.RubyFiber;
5556
import org.truffleruby.core.hash.RubyHash;
5657
import org.graalvm.options.OptionValues;
@@ -177,12 +178,14 @@ public final class RubyLanguage extends TruffleLanguage<RubyContext> {
177178

178179
/** We need an extra indirection added to ContextThreadLocal due to multiple Fibers of different Ruby Threads
179180
* sharing the same Java Thread when using the fiber pool. */
180-
private static final class ThreadLocalState {
181-
private RubyThread rubyThread;
181+
public static final class ThreadLocalState {
182+
public RubyThread rubyThread;
182183
}
183184

184185
private final ContextThreadLocal<ThreadLocalState> threadLocalState = createContextThreadLocal(
185-
(context, thread) -> new ThreadLocalState());
186+
(context, thread) -> thread instanceof FiberPoolThread
187+
? ((FiberPoolThread) thread).threadLocalState
188+
: new ThreadLocalState());
186189

187190
private final CyclicAssumption tracingCyclicAssumption = new CyclicAssumption("object-space-tracing");
188191
@CompilationFinal private volatile Assumption tracingAssumption = tracingCyclicAssumption.getAssumption();

src/main/java/org/truffleruby/core/fiber/FiberManager.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ private void fiberMain(RubyContext context, RubyFiber fiber, RubyProc block, Nod
9393
assert !fiber.isRootFiber() : "Root Fibers execute threadMain() and not fiberMain()";
9494
assertNotEntered("Fibers should start unentered to avoid triggering multithreading");
9595

96-
final Thread thread = Thread.currentThread();
96+
final FiberPoolThread thread = (FiberPoolThread) Thread.currentThread();
9797
final SourceSection sourceSection = block.sharedMethodInfo.getSourceSection();
9898
final String oldName = thread.getName();
9999
thread.setName(NAME_PREFIX + " id=" + thread.getId() + " from " + RubyLanguage.fileLine(sourceSection));
@@ -106,9 +106,14 @@ private void fiberMain(RubyContext context, RubyFiber fiber, RubyProc block, Nod
106106
final FiberMessage message = waitMessage(fiber, currentNode);
107107
final TruffleContext truffleContext = context.getEnv().getContext();
108108

109-
final Object prev = truffleContext.enter(currentNode);
110-
language.setupCurrentThread(thread, fiber.rubyThread);
109+
/* We need to setup the ThreadLocalState before enter(), as that polls and the current thread is needed for
110+
* guest safepoints. It can't just be set in RubyLanguage#initializeThread() as this java.lang.Thread is reused
111+
* for multiple Fibers and multiple Ruby Threads due to the Fiber pool, but initializeThread() is only called
112+
* once per thread. */
111113
fiber.rubyThread.setCurrentFiber(fiber);
114+
thread.threadLocalState.rubyThread = fiber.rubyThread;
115+
116+
final Object prev = truffleContext.enter(currentNode);
112117

113118
FiberMessage lastMessage = null;
114119
try {
@@ -141,8 +146,8 @@ private void fiberMain(RubyContext context, RubyFiber fiber, RubyProc block, Nod
141146
// Make sure that other fibers notice we are dead before they gain control back
142147
fiber.alive = false;
143148
// Leave context before addToMessageQueue() -> parent Fiber starts executing
144-
language.setupCurrentThread(thread, null);
145149
truffleContext.leave(currentNode, prev);
150+
thread.threadLocalState.rubyThread = null;
146151
cleanup(fiber, thread);
147152
thread.setName(oldName);
148153

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Copyright (c) 2021 Oracle and/or its affiliates. All rights reserved. This
3+
* code is released under a tri EPL/GPL/LGPL license. You can use it,
4+
* redistribute it and/or modify it under the terms of the:
5+
*
6+
* Eclipse Public License version 2.0, or
7+
* GNU General Public License version 2, or
8+
* GNU Lesser General Public License version 2.1.
9+
*/
10+
package org.truffleruby.core.fiber;
11+
12+
import org.truffleruby.RubyLanguage.ThreadLocalState;
13+
14+
public final class FiberPoolThread extends Thread {
15+
16+
public final ThreadLocalState threadLocalState;
17+
18+
public FiberPoolThread(Runnable target) {
19+
super(target);
20+
threadLocalState = new ThreadLocalState();
21+
}
22+
23+
}

src/main/java/org/truffleruby/core/thread/ThreadManager.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import org.truffleruby.core.exception.RubyException;
4141
import org.truffleruby.core.exception.RubySystemExit;
4242
import org.truffleruby.core.fiber.FiberManager;
43+
import org.truffleruby.core.fiber.FiberPoolThread;
4344
import org.truffleruby.core.fiber.RubyFiber;
4445
import org.truffleruby.core.klass.RubyClass;
4546
import org.truffleruby.core.string.StringUtils;
@@ -155,7 +156,7 @@ private Thread createFiberJavaThread(Runnable runnable) {
155156
throw new UnsupportedOperationException("fibers should not be created while pre-initializing the context");
156157
}
157158

158-
final Thread thread = new Thread(runnable); // context.getEnv().createUnenteredThread(runnable);
159+
final Thread thread = new FiberPoolThread(runnable); // context.getEnv().createUnenteredThread(runnable);
159160
thread.setName("Ruby-FiberPool-" + thread.getName());
160161
thread.setDaemon(true); // GR-33255
161162
rubyManagedThreads.add(thread); // need to be set before initializeThread()

src/main/java/org/truffleruby/language/SafepointAction.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,17 +58,19 @@ protected final void perform(Access access) {
5858
Thread.currentThread());
5959
}
6060

61+
final RubyLanguage language = RubyLanguage.getCurrentLanguage();
6162
final RubyContext context = RubyLanguage.getCurrentContext();
62-
// Not using language.getCurrentThread() here because it might not be set yet, see fiberMain()
63-
final RubyThread rubyThread = context.getThreadManager().getRubyThreadForJavaThread(access.getThread());
63+
64+
final RubyThread rubyThread = language.getCurrentThread();
65+
final Node node = access.getLocation();
6466
if (filter.test(context, rubyThread, this)) {
65-
run(rubyThread, access.getLocation());
67+
run(rubyThread, node);
6668

6769
if (filter == SafepointPredicate.ALL_THREADS_AND_FIBERS) {
6870
final RubyFiber currentFiber = rubyThread.getCurrentFiber();
6971
for (RubyFiber fiber : rubyThread.runningFibers) {
7072
if (fiber != currentFiber) {
71-
context.fiberManager.safepoint(currentFiber, fiber, this, access.getLocation());
73+
context.fiberManager.safepoint(currentFiber, fiber, this, node);
7274
}
7375
}
7476
}

0 commit comments

Comments
 (0)