From 82b14c347bba1254a993d8e041a478f5211ddd5f Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Mon, 29 Jan 2024 06:11:54 +0100 Subject: [PATCH] Avoid throwing PolyglotEngineException.illegalState while holding an internal lock --- .../truffle/polyglot/EngineAccessor.java | 6 +- .../truffle/polyglot/PolyglotContextImpl.java | 234 ++++++++++-------- .../polyglot/PolyglotLanguageContext.java | 125 ++++++---- .../PolyglotThreadAccessException.java | 54 ++++ 4 files changed, 253 insertions(+), 166 deletions(-) create mode 100644 truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotThreadAccessException.java diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/EngineAccessor.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/EngineAccessor.java index 0236a2e861069..8dbfbfe944e89 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/EngineAccessor.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/EngineAccessor.java @@ -1150,7 +1150,11 @@ public Thread createThread(Object polyglotLanguageContext, Runnable runnable, Ob threadContext = innerContext.getContext(threadContext.language); } PolyglotThread newThread = new PolyglotThread(threadContext, runnable, group, stackSize, beforeEnter, afterLeave); - threadContext.context.checkMultiThreadedAccess(newThread); + try { + threadContext.context.checkMultiThreadedAccess(newThread); + } catch (PolyglotThreadAccessException ex) { + throw ex.rethrow(threadContext.context); + } return newThread; } diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotContextImpl.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotContextImpl.java index 85db787359136..f0a49362472ec 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotContextImpl.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotContextImpl.java @@ -837,131 +837,145 @@ Object[] enterThreadChanged(boolean enterReverted, boolean pollSafepoint, boolea assert !mustSucceed; throw PolyglotEngineException.illegalState("Context cannot be entered in polyglot thread's beforeEnter or afterLeave notifications."); } - boolean needsInitialization = false; - synchronized (this) { - PolyglotThreadInfo threadInfo = getCurrentThreadInfo(); + PolyglotThreadAccessException threadAccessException = null; + LOOP: for (;;) { + if (threadAccessException != null) { + throw threadAccessException.rethrow(this); + } + boolean needsInitialization = false; + synchronized (this) { + PolyglotThreadInfo threadInfo = getCurrentThreadInfo(); - if (enterReverted && threadInfo.getEnteredCount() == 0) { - threadLocalActions.notifyThreadActivation(threadInfo, false); - if ((state.isCancelling() || state.isExiting() || state == State.CLOSED_CANCELLED || state == State.CLOSED_EXITED) && !threadInfo.isActive()) { - notifyThreadClosed(threadInfo); + if (enterReverted && threadInfo.getEnteredCount() == 0) { + threadLocalActions.notifyThreadActivation(threadInfo, false); + if ((state.isCancelling() || state.isExiting() || state == State.CLOSED_CANCELLED || state == State.CLOSED_EXITED) && !threadInfo.isActive()) { + notifyThreadClosed(threadInfo); + } + if ((state.isInterrupting() || state == State.CLOSED_INTERRUPTED) && !threadInfo.isActive()) { + Thread.interrupted(); + notifyAll(); + } } - if ((state.isInterrupting() || state == State.CLOSED_INTERRUPTED) && !threadInfo.isActive()) { - Thread.interrupted(); - notifyAll(); + if (deactivateSafepoints && threadInfo != PolyglotThreadInfo.NULL) { + threadLocalActions.notifyThreadActivation(threadInfo, false); } - } - if (deactivateSafepoints && threadInfo != PolyglotThreadInfo.NULL) { - threadLocalActions.notifyThreadActivation(threadInfo, false); - } - assert threadInfo != null; - if (!leaveAndEnter) { - checkClosedOrDisposing(mustSucceed); - if (threadInfo.isInLeaveAndEnter()) { - throw PolyglotEngineException.illegalState("Context cannot be entered inside leaveAndEnter."); + assert threadInfo != null; + if (!leaveAndEnter) { + checkClosedOrDisposing(mustSucceed); + if (threadInfo.isInLeaveAndEnter()) { + throw PolyglotEngineException.illegalState("Context cannot be entered inside leaveAndEnter."); + } } - } - - threadInfo = threads.get(current); - if (threadInfo == null) { - threadInfo = createThreadInfo(current, polyglotThreadFirstEnter); - needsInitialization = true; - } - if (singleThreaded) { - /* - * If this is the only thread, then setting the cached thread info to NULL - * is no performance problem. If there is other thread that is just about to - * enter, we are making sure that it initializes multi-threading if this - * thread doesn't do it. - */ - setCachedThreadInfo(PolyglotThreadInfo.NULL); - } - boolean transitionToMultiThreading = isSingleThreaded() && hasActiveOtherThread(true, false); - if (transitionToMultiThreading) { - // recheck all thread accesses - checkAllThreadAccesses(Thread.currentThread(), false); - } + threadInfo = threads.get(current); + if (threadInfo == null) { + try { + threadInfo = createThreadInfo(current, polyglotThreadFirstEnter); + } catch (PolyglotThreadAccessException ex) { + threadAccessException = ex; + continue LOOP; + } + needsInitialization = true; + } + if (singleThreaded) { + /* + * If this is the only thread, then setting the cached thread info to NULL + * is no performance problem. If there is other thread that is just about to + * enter, we are making sure that it initializes multi-threading if this + * thread doesn't do it. + */ + setCachedThreadInfo(PolyglotThreadInfo.NULL); + } + boolean transitionToMultiThreading = isSingleThreaded() && hasActiveOtherThread(true, false); + + if (transitionToMultiThreading) { + try { + // recheck all thread accesses + checkAllThreadAccesses(Thread.currentThread(), false); + } catch (PolyglotThreadAccessException ex) { + threadAccessException = ex; + continue LOOP; + } + } - if (transitionToMultiThreading) { - /* - * We need to do this early (before initializeMultiThreading) as entering or - * local initialization depends on single thread per context. - */ - engine.singleThreadPerContext.invalidate(); - singleThreaded = false; - } + if (transitionToMultiThreading) { + /* + * We need to do this early (before initializeMultiThreading) as entering or + * local initialization depends on single thread per context. + */ + engine.singleThreadPerContext.invalidate(); + singleThreaded = false; + } - if (needsInitialization) { - threads.put(current, threadInfo); - } + if (needsInitialization) { + threads.put(current, threadInfo); + } - if (needsInitialization) { - /* - * Do not enter the thread before initializing thread locals. Creation of - * thread locals might fail. - */ - initializeThreadLocals(threadInfo); - } + if (needsInitialization) { + /* + * Do not enter the thread before initializing thread locals. Creation of + * thread locals might fail. + */ + initializeThreadLocals(threadInfo); + } - prev = threadInfo.enterInternal(); - if (leaveAndEnter) { - threadInfo.setLeaveAndEnterInterrupter(null); - notifyAll(); - } - if (needsInitialization) { - this.threadLocalActions.notifyEnterCreatedThread(); - } - if (closingThread != Thread.currentThread()) { - try { - threadInfo.notifyEnter(engine, this); - } catch (Throwable t) { - threadInfo.leaveInternal(prev); - throw t; + prev = threadInfo.enterInternal(); + if (leaveAndEnter) { + threadInfo.setLeaveAndEnterInterrupter(null); + notifyAll(); } - } - enteredThread = threadInfo; + if (needsInitialization) { + this.threadLocalActions.notifyEnterCreatedThread(); + } + if (closingThread != Thread.currentThread()) { + try { + threadInfo.notifyEnter(engine, this); + } catch (Throwable t) { + threadInfo.leaveInternal(prev); + throw t; + } + } + enteredThread = threadInfo; - // new thread became active so we need to check potential active thread local - // actions and process them. - Set activatedActions = null; - if (enteredThread.getEnteredCount() == 1 && !deactivateSafepoints) { - activatedActions = threadLocalActions.notifyThreadActivation(threadInfo, true); - } + // new thread became active so we need to check potential active thread local + // actions and process them. + Set activatedActions = null; + if (enteredThread.getEnteredCount() == 1 && !deactivateSafepoints) { + activatedActions = threadLocalActions.notifyThreadActivation(threadInfo, true); + } - if (transitionToMultiThreading) { - // we need to verify that all languages give access - // to all threads in multi-threaded mode. - transitionToMultiThreaded(mustSucceed); - } + if (transitionToMultiThreading) { + // we need to verify that all languages give access + // to all threads in multi-threaded mode. + transitionToMultiThreaded(mustSucceed); + } - if (needsInitialization) { - initializeNewThread(enteredThread, mustSucceed); - } + if (needsInitialization) { + initializeNewThread(enteredThread, mustSucceed); + } - if (enteredThread.getEnteredCount() == 1 && !pauseThreadLocalActions.isEmpty()) { - for (Iterator threadLocalActionIterator = pauseThreadLocalActions.iterator(); threadLocalActionIterator.hasNext();) { - PauseThreadLocalAction threadLocalAction = threadLocalActionIterator.next(); - if (!threadLocalAction.isPause()) { - threadLocalActionIterator.remove(); - } else { - if (activatedActions == null || !activatedActions.contains(threadLocalAction)) { - threadLocalActions.submit(new Thread[]{Thread.currentThread()}, PolyglotEngineImpl.ENGINE_ID, threadLocalAction, new HandshakeConfig(true, true, false, false)); + if (enteredThread.getEnteredCount() == 1 && !pauseThreadLocalActions.isEmpty()) { + for (Iterator threadLocalActionIterator = pauseThreadLocalActions.iterator(); threadLocalActionIterator.hasNext();) { + PauseThreadLocalAction threadLocalAction = threadLocalActionIterator.next(); + if (!threadLocalAction.isPause()) { + threadLocalActionIterator.remove(); + } else { + if (activatedActions == null || !activatedActions.contains(threadLocalAction)) { + threadLocalActions.submit(new Thread[]{Thread.currentThread()}, PolyglotEngineImpl.ENGINE_ID, threadLocalAction, new HandshakeConfig(true, true, false, false)); + } } } } - } - - // never cache last thread on close or when closingThread - setCachedThreadInfo(threadInfo); - } - if (needsInitialization) { - EngineAccessor.INSTRUMENT.notifyThreadStarted(engine, creatorTruffleContext, current); + // never cache last thread on close or when closingThread + setCachedThreadInfo(threadInfo); + } + if (needsInitialization) { + EngineAccessor.INSTRUMENT.notifyThreadStarted(engine, creatorTruffleContext, current); + } + return prev; } - - return prev; } finally { /* * We need to always poll the safepoint here in case we already submitted a thread @@ -1023,12 +1037,12 @@ void setCachedThreadInfo(PolyglotThreadInfo info) { } } - synchronized void checkMultiThreadedAccess(PolyglotThread newThread) { + synchronized void checkMultiThreadedAccess(PolyglotThread newThread) throws PolyglotThreadAccessException { boolean singleThread = singleThreaded ? !isActiveNotCancelled() : false; checkAllThreadAccesses(newThread, singleThread); } - private void checkAllThreadAccesses(Thread enteringThread, boolean singleThread) { + private void checkAllThreadAccesses(Thread enteringThread, boolean singleThread) throws PolyglotThreadAccessException { assert Thread.holdsLock(this); List deniedLanguages = null; for (PolyglotLanguageContext context : contexts) { @@ -1350,7 +1364,7 @@ private void transitionToMultiThreaded(boolean mustSucceed) { volatileStatementCounter.getAndAdd(-statementsExecuted); } - private PolyglotThreadInfo createThreadInfo(Thread current, boolean polyglotThreadFirstEnter) { + private PolyglotThreadInfo createThreadInfo(Thread current, boolean polyglotThreadFirstEnter) throws PolyglotThreadAccessException { assert Thread.holdsLock(this); PolyglotThreadInfo threadInfo = new PolyglotThreadInfo(this, current, polyglotThreadFirstEnter); @@ -1375,7 +1389,7 @@ private PolyglotThreadInfo createThreadInfo(Thread current, boolean polyglotThre return threadInfo; } - static RuntimeException throwDeniedThreadAccess(Thread current, boolean accessSingleThreaded, List deniedLanguages) { + static RuntimeException throwDeniedThreadAccess(Thread current, boolean accessSingleThreaded, List deniedLanguages) throws PolyglotThreadAccessException { String message; StringBuilder languagesString = new StringBuilder(""); for (PolyglotLanguage language : deniedLanguages) { @@ -1389,7 +1403,7 @@ static RuntimeException throwDeniedThreadAccess(Thread current, boolean accessSi } else { message = String.format("Multi threaded access requested by thread %s but is not allowed for language(s) %s.", current, languagesString); } - throw PolyglotEngineException.illegalState(message); + throw new PolyglotThreadAccessException(message); } public Object getBindings(String languageId) { diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotLanguageContext.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotLanguageContext.java index 0947a18c402ba..e4d2056fc56a3 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotLanguageContext.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotLanguageContext.java @@ -287,7 +287,7 @@ PolyglotLanguageInstance getLanguageInstance() { return lazy.languageInstance; } - private void checkThreadAccess(Env localEnv) { + private void checkThreadAccess(Env localEnv) throws PolyglotThreadAccessException { assert Thread.holdsLock(context); boolean singleThreaded = context.isSingleThreaded(); Thread firstFailingThread = null; @@ -619,69 +619,84 @@ void ensureCreated(PolyglotLanguage accessingLanguage) { } } - synchronized (context) { - if (!created) { - if (eventsEnabled) { - EngineAccessor.INSTRUMENT.notifyLanguageContextCreate(context.engine, context.creatorTruffleContext, language.info); + boolean wasCreated = false; + try { + PolyglotThreadAccessException threadAccessException = null; + LOOP: for (;;) { + if (threadAccessException != null) { + throw threadAccessException.rethrow(this); } - boolean wasCreated = false; - try { - Env localEnv = LANGUAGE.createEnv(this, languageInstance.spi, contextConfig.out, + synchronized (context) { + if (!created) { + if (eventsEnabled) { + EngineAccessor.INSTRUMENT.notifyLanguageContextCreate(context.engine, context.creatorTruffleContext, language.info); + } + try { + Env localEnv = LANGUAGE.createEnv(this, languageInstance.spi, contextConfig.out, contextConfig.err, contextConfig.in, creatorConfig, contextConfig.getLanguageOptionValues(language).copy(), contextConfig.getApplicationArguments(language)); - Lazy localLazy = new Lazy(languageInstance, contextConfig); - - if (layer.isSingleContext()) { - languageInstance.singleLanguageContext.update(this); - } else { - languageInstance.singleLanguageContext.invalidate(); - } - - checkThreadAccess(localEnv); - - // no more errors after this line - creatingThread = Thread.currentThread(); - env = localEnv; - lazy = localLazy; - assert EngineAccessor.LANGUAGE.getLanguage(env) != null; - - try { - List languageServicesCollector = new ArrayList<>(); - Object contextImpl = LANGUAGE.createEnvContext(localEnv, languageServicesCollector); - language.initializeContextClass(contextImpl); - String errorMessage = verifyServices(language.info, languageServicesCollector, language.cache.getServices()); - if (errorMessage != null) { - throw PolyglotEngineException.illegalState(errorMessage); - } - PolyglotFastThreadLocals.notifyLanguageCreated(this); - this.languageServices = languageServicesCollector; - if (language.isHost()) { - context.initializeHostContext(this, context.config); - } - wasCreated = true; - if (eventsEnabled) { - EngineAccessor.INSTRUMENT.notifyLanguageContextCreated(context.engine, context.creatorTruffleContext, language.info); + Lazy localLazy = new Lazy(languageInstance, contextConfig); + + if (layer.isSingleContext()) { + languageInstance.singleLanguageContext.update(this); + } else { + languageInstance.singleLanguageContext.invalidate(); + } + + try { + checkThreadAccess(localEnv); + } catch (PolyglotThreadAccessException ex) { + threadAccessException = ex; + continue LOOP; + } + + // no more errors after this line + creatingThread = Thread.currentThread(); + env = localEnv; + lazy = localLazy; + assert EngineAccessor.LANGUAGE.getLanguage(env) != null; + + try { + List languageServicesCollector = new ArrayList<>(); + Object contextImpl = LANGUAGE.createEnvContext(localEnv, languageServicesCollector); + language.initializeContextClass(contextImpl); + String errorMessage = verifyServices(language.info, languageServicesCollector, language.cache.getServices()); + if (errorMessage != null) { + throw PolyglotEngineException.illegalState(errorMessage); + } + PolyglotFastThreadLocals.notifyLanguageCreated(this); + this.languageServices = languageServicesCollector; + if (language.isHost()) { + context.initializeHostContext(this, context.config); + } + wasCreated = true; + if (eventsEnabled) { + EngineAccessor.INSTRUMENT.notifyLanguageContextCreated(context.engine, context.creatorTruffleContext, language.info); + } + context.invokeContextLocalsFactory(context.contextLocals, languageInstance.contextLocalLocations); + context.invokeContextThreadLocalFactory(languageInstance.contextThreadLocalLocations); + + languageInstance = null; // commit language use + } catch (Throwable e) { + env = null; + lazy = null; + throw e; + } finally { + creatingThread = null; + } + created = true; + } finally { } - context.invokeContextLocalsFactory(context.contextLocals, languageInstance.contextLocalLocations); - context.invokeContextThreadLocalFactory(languageInstance.contextThreadLocalLocations); - - languageInstance = null; // commit language use - } catch (Throwable e) { - env = null; - lazy = null; - throw e; - } finally { - creatingThread = null; - } - created = true; - } finally { - if (!wasCreated && eventsEnabled) { - EngineAccessor.INSTRUMENT.notifyLanguageContextCreateFailed(context.engine, context.creatorTruffleContext, language.info); } } + break LOOP; + } + } finally { + if (!wasCreated && eventsEnabled) { + EngineAccessor.INSTRUMENT.notifyLanguageContextCreateFailed(context.engine, context.creatorTruffleContext, language.info); } } } diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotThreadAccessException.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotThreadAccessException.java new file mode 100644 index 0000000000000..e0f1046c40d17 --- /dev/null +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotThreadAccessException.java @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2017, 2024, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * The Universal Permissive License (UPL), Version 1.0 + * + * Subject to the condition set forth below, permission is hereby granted to any + * person obtaining a copy of this software, associated documentation and/or + * data (collectively the "Software"), free of charge and under any and all + * copyright rights in the Software, and any and all patent rights owned or + * freely licensable by each licensor hereunder covering either (i) the + * unmodified Software as contributed to or provided by such licensor, or (ii) + * the Larger Works (as defined below), to deal in both + * + * (a) the Software, and + * + * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if + * one is included with the Software each a "Larger Work" to which the Software + * is contributed by such licensors), + * + * without restriction, including without limitation the rights to copy, create + * derivative works of, display, perform, and distribute the Software and make, + * use, sell, offer for sale, import, export, have made, and have sold the + * Software and the Larger Work(s), and to sublicense the foregoing rights on + * either these or other terms. + * + * This license is subject to the following condition: + * + * The above copyright notice and either this complete permission notice or at a + * minimum a reference to the UPL must be included in all copies or substantial + * portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package com.oracle.truffle.polyglot; + +final class PolyglotThreadAccessException extends Exception { + static final long serialVersionUID = 1L; + + PolyglotThreadAccessException(String message) { + super(message); + } + + RuntimeException rethrow(Object noLockCheck) { + assert !Thread.holdsLock(noLockCheck) : "Only rethrow without holding internal lock"; + throw PolyglotEngineException.illegalState(getMessage()); + } +}