Skip to content

Commit 60ad304

Browse files
committed
[GR-64886] Reinforce assertions during thread registration.
PullRequest: graal/20769
2 parents 867ee3c + 326fd90 commit 60ad304

File tree

2 files changed

+61
-4
lines changed

2 files changed

+61
-4
lines changed

espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/EspressoLanguage.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ public final class EspressoLanguage extends TruffleLanguage<EspressoContext> imp
163163

164164
@CompilationFinal private GuestFieldOffsetStrategy guestFieldOffsetStrategy;
165165

166-
private final ContextThreadLocal<EspressoThreadLocalState> threadLocalState = locals.createContextThreadLocal((context, thread) -> new EspressoThreadLocalState(context));
166+
private final ContextThreadLocal<EspressoThreadLocalState> threadLocalState = locals.createContextThreadLocal(EspressoThreadLocalState::new);
167167

168168
public EspressoLanguage() {
169169
// Initialize statically defined symbols and substitutions.

espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/EspressoThreadLocalState.java

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
*/
2323
package com.oracle.truffle.espresso.runtime;
2424

25+
import java.lang.ref.WeakReference;
26+
2527
import com.oracle.truffle.api.CompilerDirectives;
2628
import com.oracle.truffle.espresso.impl.ClassRegistry;
2729
import com.oracle.truffle.espresso.runtime.staticobject.StaticObject;
@@ -47,10 +49,13 @@ public class EspressoThreadLocalState {
4749

4850
private boolean inTransformer;
4951

52+
private WeakReference<Thread> hostThread;
53+
5054
@SuppressWarnings("unused")
51-
public EspressoThreadLocalState(EspressoContext context) {
55+
public EspressoThreadLocalState(EspressoContext context, Thread t) {
5256
typeStack = new ClassRegistry.TypeStack();
5357
privilegedStack = new VM.PrivilegedStack(context);
58+
assert ((hostThread = new WeakReference<>(t)) != null);
5459
}
5560

5661
public StaticObject getPendingExceptionObject() {
@@ -76,8 +81,7 @@ public void clearPendingException() {
7681

7782
public void setCurrentPlatformThread(StaticObject t) {
7883
assert t != null && StaticObject.notNull(t);
79-
assert t.getKlass().getContext().getThreadAccess().getHost(t) == Thread.currentThread() : "Current thread fast access set by non-current thread";
80-
assert currentPlatformThread == null || currentPlatformThread == t : currentPlatformThread + " vs " + t;
84+
assert diagnose(t);
8185
currentPlatformThread = t;
8286
}
8387

@@ -87,6 +91,7 @@ public void setCurrentVirtualThread(StaticObject t) {
8791
}
8892

8993
public void initializeCurrentThread(StaticObject t) {
94+
assert diagnose(t);
9095
setCurrentPlatformThread(t);
9196
setCurrentVirtualThread(t);
9297
}
@@ -213,4 +218,56 @@ public void close() {
213218
inTransformer = false;
214219
}
215220
}
221+
222+
// Helper methods for assertions
223+
private boolean diagnose(StaticObject t) {
224+
// @formatter:off
225+
// Ensure we work from the thread used for this local creation.
226+
assert Thread.currentThread() == hostThread.get() : //
227+
"Initializing current thread in EspressoThreadLocalState for a different thread than current:\n" +
228+
" Current: " + Thread.currentThread() + "\n" +
229+
" Registered: " + hostThread.get();
230+
231+
// Ensure the registered guest thread is and stays linked to the corresponding host thread.
232+
if (currentPlatformThread != null) {
233+
assert getHost(currentPlatformThread) == Thread.currentThread() : //
234+
"Registered platform thread not associated with current host thread.";
235+
}
236+
237+
// Ensure we are consistently registering guest threads.
238+
if (t != null) {
239+
// The thread we are registering is linked to the current thread.
240+
assert getHost(t) == Thread.currentThread() : //
241+
"Current thread fast access set by non-current thread";
242+
243+
// Ensure we are not registering multiple guest threads for the same host thread.
244+
assert currentPlatformThread == null || currentPlatformThread == t : //
245+
/*- Report these threads names */
246+
getHost(currentPlatformThread).getName() + " vs " + getHost(t).getName() + "\n" +
247+
/*- Report these threads identities */
248+
"Guest identities" + System.identityHashCode(currentPlatformThread) + " vs " + System.identityHashCode(t) + "\n" +
249+
/*- Checks if our host threads are actually different, or if it is simply a renamed one. */
250+
"Host identities: " + System.identityHashCode(getHost(currentPlatformThread)) + " vs " + System.identityHashCode(getHost(t));
251+
}
252+
// @formatter:on
253+
/*-
254+
* Current theory for GR-50089:
255+
*
256+
* For some reason, when creating a new guest thread, instead of spawning a new host thread
257+
* Truffle gives us back a previously created one that had completed.
258+
*
259+
* If that is the case, then, on failure, we will see in the report:
260+
* - Different guest names and/or guest identities
261+
* - Same host identities
262+
*
263+
* This may be solved by unregistering a guest thread from the thread local state in
264+
* ThreadAccess.terminate().
265+
*/
266+
return true;
267+
}
268+
269+
private static Thread getHost(StaticObject t) {
270+
assert t != null && StaticObject.notNull(t);
271+
return t.getKlass().getContext().getThreadAccess().getHost(t);
272+
}
216273
}

0 commit comments

Comments
 (0)