diff --git a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/EspressoContext.java b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/EspressoContext.java index 0e05673f798d..a48a8b742886 100644 --- a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/EspressoContext.java +++ b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/EspressoContext.java @@ -1095,8 +1095,9 @@ public StaticObject[] getActiveThreads() { return espressoEnv.getThreadRegistry().activeThreads(); } - public void registerThread(Thread host, StaticObject self) { - espressoEnv.getThreadRegistry().registerThread(host, self); + public void registerJavaThread(Thread host, StaticObject self) { + StaticObject guest = espressoEnv.getThreadRegistry().registerThread(host, self); + assert self == guest; if (shouldReportVMEvents()) { espressoEnv.getEventListener().threadStarted(self); } diff --git a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/EspressoThreadLocalState.java b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/EspressoThreadLocalState.java index a41c97918d29..245fddd3ce24 100644 --- a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/EspressoThreadLocalState.java +++ b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/runtime/EspressoThreadLocalState.java @@ -26,6 +26,7 @@ import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.espresso.impl.ClassRegistry; +import com.oracle.truffle.espresso.impl.Field; import com.oracle.truffle.espresso.runtime.staticobject.StaticObject; import com.oracle.truffle.espresso.vm.VM; @@ -244,28 +245,30 @@ assert getHost(currentPlatformThread) == Thread.currentThread() : // assert getHost(t) == Thread.currentThread() : // "Current thread fast access set by non-current thread"; + EspressoContext ctx = EspressoContext.get(null); + Field managedBit = ctx.getMeta().HIDDEN_ESPRESSO_MANAGED; + // Ensure we are not registering multiple guest threads for the same host thread. assert currentPlatformThread == null || currentPlatformThread == t : // /*- Report these threads names */ getHost(currentPlatformThread).getName() + " vs " + getHost(t).getName() + "\n" + /*- Report these threads identities */ - "Guest identities" + System.identityHashCode(currentPlatformThread) + " vs " + System.identityHashCode(t) + "\n" + + "Guest identities: " + System.identityHashCode(currentPlatformThread) + " vs " + System.identityHashCode(t) + "\n" + /*- Checks if our host threads are actually different, or if it is simply a renamed one. */ - "Host identities: " + System.identityHashCode(getHost(currentPlatformThread)) + " vs " + System.identityHashCode(getHost(t)); + "Host identities: " + System.identityHashCode(getHost(currentPlatformThread)) + " vs " + System.identityHashCode(getHost(t)) + "/n" + + /*- Obtain the `managed` bits to know the origin of these threads */ + "Managed by espresso: " + managedBit.getBoolean(currentPlatformThread) + " vs " + managedBit.getBoolean(t); } // @formatter:on - /*- + /* * Current theory for GR-50089: * - * For some reason, when creating a new guest thread, instead of spawning a new host thread - * Truffle gives us back a previously created one that had completed. - * - * If that is the case, then, on failure, we will see in the report: - * - Different guest names and/or guest identities - * - Same host identities + * Lack of synchronization in `ThreadAccess.createJavaThread()` makes it so the polyglot + * thread is started and can reach `EspressoLanguage.initializeThread()` before the store to + * the thread registry can be observed. * - * This may be solved by unregistering a guest thread from the thread local state in - * ThreadAccess.terminate(). + * Now that a `synchronize` block has been added to `EspressoThreadRegistry`, this assertion + * should no longer trigger. */ return true; } diff --git a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/threads/EspressoThreadRegistry.java b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/threads/EspressoThreadRegistry.java index 98c6a113af6b..5791f334bda1 100644 --- a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/threads/EspressoThreadRegistry.java +++ b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/threads/EspressoThreadRegistry.java @@ -117,45 +117,50 @@ private void registerMainThread(Thread thread, StaticObject guest) { public final AtomicLong createdThreadCount = new AtomicLong(); public final AtomicLong peakThreadCount = new AtomicLong(); - public void registerThread(Thread host, StaticObject guest) { - activeThreads.add(guest); - - // Update java.lang.management counters. - createdThreadCount.incrementAndGet(); - peakThreadCount.updateAndGet(new LongUnaryOperator() { - @Override - public long applyAsLong(long oldPeak) { - return Math.max(oldPeak, activeThreads.size()); + public StaticObject registerThread(Thread host, StaticObject guest) { + synchronized (activeThreadLock) { + StaticObject existingThread = getGuestThreadFromHost(host); + if (existingThread != null) { + // There is already a guest thread for this host thread. + return existingThread; } - }); - if (finalizerThreadId == -1) { - if (getMeta().java_lang_ref_Finalizer$FinalizerThread == guest.getKlass()) { - synchronized (activeThreadLock) { + activeThreads.add(guest); + + // Update java.lang.management counters. + createdThreadCount.incrementAndGet(); + peakThreadCount.updateAndGet(new LongUnaryOperator() { + @Override + public long applyAsLong(long oldPeak) { + return Math.max(oldPeak, activeThreads.size()); + } + }); + + if (finalizerThreadId == -1) { + if (getMeta().java_lang_ref_Finalizer$FinalizerThread == guest.getKlass()) { if (finalizerThreadId == -1) { CompilerDirectives.transferToInterpreterAndInvalidate(); finalizerThreadId = getThreadId(host); guestFinalizerThread = guest; - return; + return guest; } } } - } - if (referenceHandlerThreadId == -1) { - if (getMeta().java_lang_ref_Reference$ReferenceHandler == guest.getKlass()) { - synchronized (activeThreadLock) { + if (referenceHandlerThreadId == -1) { + if (getMeta().java_lang_ref_Reference$ReferenceHandler == guest.getKlass()) { if (referenceHandlerThreadId == -1) { CompilerDirectives.transferToInterpreterAndInvalidate(); referenceHandlerThreadId = getThreadId(host); guestReferenceHandlerThread = guest; - return; + return guest; } } } - } - pushThread(Math.toIntExact(getThreadId(host)), guest); - if (host == Thread.currentThread()) { - getContext().registerCurrentThread(guest); + pushThread(Math.toIntExact(getThreadId(host)), guest); + if (host == Thread.currentThread()) { + getContext().registerCurrentThread(guest); + } + return guest; } } @@ -166,18 +171,18 @@ public long applyAsLong(long oldPeak) { */ @SuppressFBWarnings(value = "NN", justification = "Removing a thread from the active set is the state change we need.") public boolean unregisterThread(StaticObject thread) { - if (!activeThreads.remove(thread)) { - // Already unregistered - return false; - } - logger.fine(() -> { - String guestName = getThreadAccess().getThreadName(thread); - long guestId = getThreadAccess().getThreadId(thread); - return String.format("unregisterThread([GUEST:%s, %d])", guestName, guestId); - }); - Thread hostThread = getThreadAccess().getHost(thread); - int id = Math.toIntExact(getThreadId(hostThread)); synchronized (activeThreadLock) { + if (!activeThreads.remove(thread)) { + // Already unregistered + return false; + } + logger.fine(() -> { + String guestName = getThreadAccess().getThreadName(thread); + long guestId = getThreadAccess().getThreadId(thread); + return String.format("unregisterThread([GUEST:%s, %d])", guestName, guestId); + }); + Thread hostThread = getThreadAccess().getHost(thread); + int id = Math.toIntExact(getThreadId(hostThread)); if (id == mainThreadId) { mainThreadId = -1; guestMainThread = null; @@ -264,16 +269,17 @@ public StaticObject createGuestThreadFromHost(Thread hostThread, Meta meta, VM v return null; } synchronized (activeThreadLock) { - StaticObject exisitingThread = getGuestThreadFromHost(hostThread); - if (exisitingThread != null) { + StaticObject existingThread = getGuestThreadFromHost(hostThread); + if (existingThread != null) { // already a live guest thread for this host thread - return exisitingThread; + return existingThread; } StaticObject effectiveThreadGroup = threadGroup; if (effectiveThreadGroup == null || StaticObject.isNull(effectiveThreadGroup)) { effectiveThreadGroup = getContext().getMainThreadGroup(); } vm.attachThread(hostThread); + StaticObject guestThread = meta.java_lang_Thread.allocateInstance(getContext()); // Allow guest Thread.currentThread() to work. @@ -282,7 +288,9 @@ public StaticObject createGuestThreadFromHost(Thread hostThread, Meta meta, VM v } getThreadAccess().setEETopAlive(guestThread); getThreadAccess().initializeHiddenFields(guestThread, hostThread, managedByEspresso); - registerThread(hostThread, guestThread); + // Associate host and guest. + existingThread = registerThread(hostThread, guestThread); + assert existingThread == guestThread; assert getThreadAccess().getCurrentGuestThread() != null; if (name == null) { diff --git a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/threads/ThreadAccess.java b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/threads/ThreadAccess.java index 2f40d84a3435..cd5e58fd5a64 100644 --- a/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/threads/ThreadAccess.java +++ b/espresso/src/com.oracle.truffle.espresso/src/com/oracle/truffle/espresso/threads/ThreadAccess.java @@ -449,18 +449,20 @@ public boolean isManaged(StaticObject guest) { */ public Thread createJavaThread(StaticObject guest, DirectCallNode exit, DirectCallNode dispatch) { Thread host = getContext().getEnv().newTruffleThreadBuilder(new GuestRunnable(getContext(), guest, exit, dispatch)).build(); - initializeHiddenFields(guest, host, true); // Prepare host thread host.setDaemon(isDaemon(guest)); host.setPriority(getPriority(guest)); + String guestName = getContext().getThreadAccess().getThreadName(guest); + host.setName(guestName); if (isInterrupted(guest, false)) { host.interrupt(); } - String guestName = getContext().getThreadAccess().getThreadName(guest); - host.setName(guestName); + // Prepare guest thread + initializeHiddenFields(guest, host, true); getThreadAccess().setEETopAlive(guest); - // Make the thread known to the context - getContext().registerThread(host, guest); + // Associate host and guest + getContext().registerJavaThread(host, guest); + // Thread must be runnable on returning from 'start', so we set it preemptively // here. getThreadAccess().initializeState(guest, ThreadState.DefaultStates.DEFAULT_RUNNABLE_STATE);