Skip to content

Commit

Permalink
[GR-18682] NPE when instrument asks for TruffleFile in onLanguageCont…
Browse files Browse the repository at this point in the history
…extInitialized with SVM patching.

PullRequest: graal/4551
  • Loading branch information
tzezula committed Oct 9, 2019
2 parents 519cb18 + 0fce1c3 commit 26ca9db
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ public void closeInternalContext(Object impl) {
if (context.isActive()) {
throw new IllegalStateException("The context is currently entered and cannot be closed.");
}
context.closeImpl(false, false);
context.closeImpl(false, false, true);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
import com.oracle.truffle.api.TruffleContext;
import com.oracle.truffle.api.TruffleLanguage;
import com.oracle.truffle.api.impl.Accessor.CastUnsafe;
import com.oracle.truffle.api.nodes.LanguageInfo;
import com.oracle.truffle.polyglot.HostLanguage.HostContext;
import com.oracle.truffle.polyglot.PolyglotEngineImpl.CancelExecution;

Expand Down Expand Up @@ -867,7 +868,7 @@ public Engine getEngineImpl(Context sourceContext) {
@Override
public void close(Context sourceContext, boolean cancelIfExecuting) {
checkCreatorAccess(sourceContext, "closed");
boolean closeCompleted = closeImpl(cancelIfExecuting, cancelIfExecuting);
boolean closeCompleted = closeImpl(cancelIfExecuting, cancelIfExecuting, true);
if (cancelIfExecuting) {
engine.getCancelHandler().cancel(Arrays.asList(this));
} else if (!closeCompleted) {
Expand Down Expand Up @@ -913,7 +914,7 @@ public Value asValue(Object hostValue) {
}

void waitForClose() {
while (!closeImpl(false, true)) {
while (!closeImpl(false, true, true)) {
try {
synchronized (this) {
wait(1000);
Expand Down Expand Up @@ -970,7 +971,7 @@ synchronized void notifyThreadClosed() {
}
}

boolean closeImpl(boolean cancelIfExecuting, boolean waitForPolyglotThreads) {
boolean closeImpl(boolean cancelIfExecuting, boolean waitForPolyglotThreads, boolean notifyInstruments) {
/*
* As a first step we prepare for close by waiting for other threads to finish closing and
* checking whether other threads are still executing. This block performs the following
Expand Down Expand Up @@ -1060,9 +1061,9 @@ boolean closeImpl(boolean cancelIfExecuting, boolean waitForPolyglotThreads) {
assert !closed;
Object prev = engine.enter(this);
try {
closeChildContexts(cancelIfExecuting, waitForPolyglotThreads);
closeChildContexts(cancelIfExecuting, waitForPolyglotThreads, notifyInstruments);

finalizeContext();
finalizeContext(notifyInstruments);

// finalization performed commit close -> no reinitialization allowed

Expand Down Expand Up @@ -1097,7 +1098,7 @@ boolean closeImpl(boolean cancelIfExecuting, boolean waitForPolyglotThreads) {
*/
if (disposedContexts != null) {
for (PolyglotLanguageContext context : disposedContexts) {
context.notifyDisposed();
context.notifyDisposed(notifyInstruments);
}
}

Expand All @@ -1106,14 +1107,16 @@ boolean closeImpl(boolean cancelIfExecuting, boolean waitForPolyglotThreads) {
synchronized (parent) {
parent.childContexts.remove(this);
}
} else {
} else if (notifyInstruments) {
engine.removeContext(this);
}

for (Thread thread : remainingThreads) {
EngineAccessor.INSTRUMENT.notifyThreadFinished(engine, truffleContext, thread);
if (notifyInstruments) {
for (Thread thread : remainingThreads) {
EngineAccessor.INSTRUMENT.notifyThreadFinished(engine, truffleContext, thread);
}
EngineAccessor.INSTRUMENT.notifyContextClosed(engine, truffleContext);
}
EngineAccessor.INSTRUMENT.notifyContextClosed(engine, truffleContext);
if (parent == null) {
if (!this.config.logLevels.isEmpty()) {
EngineAccessor.LANGUAGE.configureLoggers(this, null, getAllLoggers(engine));
Expand All @@ -1126,13 +1129,13 @@ boolean closeImpl(boolean cancelIfExecuting, boolean waitForPolyglotThreads) {
return true;
}

private void closeChildContexts(boolean cancelIfExecuting, boolean waitForPolyglotThreads) {
private void closeChildContexts(boolean cancelIfExecuting, boolean waitForPolyglotThreads, boolean notifyInstruments) {
PolyglotContextImpl[] childrenToClose;
synchronized (this) {
childrenToClose = childContexts.toArray(new PolyglotContextImpl[childContexts.size()]);
}
for (PolyglotContextImpl childContext : childrenToClose) {
childContext.closeImpl(cancelIfExecuting, waitForPolyglotThreads);
childContext.closeImpl(cancelIfExecuting, waitForPolyglotThreads, notifyInstruments);
}
}

Expand Down Expand Up @@ -1160,7 +1163,7 @@ private List<PolyglotLanguageContext> disposeContext() {
}
}

private void finalizeContext() {
private void finalizeContext(boolean notifyInstruments) {
// we need to run finalization at least twice in case a finalization run has
// initialized a new contexts
boolean finalizationPerformed;
Expand All @@ -1172,7 +1175,7 @@ private void finalizeContext() {
PolyglotLanguageContext context = contexts[i];
if (context.isInitialized()) {
try {
finalizationPerformed |= context.finalizeContext();
finalizationPerformed |= context.finalizeContext(notifyInstruments);
} catch (Exception | Error ex) {
throw PolyglotImpl.wrapGuestException(context, ex);
}
Expand Down Expand Up @@ -1229,12 +1232,29 @@ boolean patch(PolyglotContextConfig newConfig) {
return false;
}
}
replayInstrumentationEvents();
} finally {
engine.leave(prev, this);
}
return true;
}

private void replayInstrumentationEvents() {
notifyContextCreated();
for (PolyglotLanguageContext lc : contexts) {
LanguageInfo language = lc.language.info;
if (lc.eventsEnabled && lc.env != null) {
EngineAccessor.INSTRUMENT.notifyLanguageContextCreated(this, truffleContext, language);
if (lc.isInitialized()) {
EngineAccessor.INSTRUMENT.notifyLanguageContextInitialized(this, truffleContext, language);
if (lc.finalized) {
EngineAccessor.INSTRUMENT.notifyLanguageContextFinalized(this, truffleContext, language);
}
}
}
}
}

synchronized void checkSubProcessFinished() {
ProcessHandlers.ProcessDecorator[] processes = subProcesses.toArray(new ProcessHandlers.ProcessDecorator[subProcesses.size()]);
for (ProcessHandlers.ProcessDecorator process : processes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,7 @@ synchronized void ensureClosed(boolean cancelIfExecuting, boolean ignoreCloseFai
for (PolyglotContextImpl context : localContexts) {
assert !Thread.holdsLock(context);
try {
boolean closeCompleted = context.closeImpl(cancelIfExecuting, cancelIfExecuting);
boolean closeCompleted = context.closeImpl(cancelIfExecuting, cancelIfExecuting, true);
if (!closeCompleted && !cancelIfExecuting) {
if (!ignoreCloseFailure) {
throw new IllegalStateException(String.format("One of the context instances is currently executing. " +
Expand Down Expand Up @@ -968,7 +968,6 @@ static PolyglotEngineImpl preInitialize(PolyglotImpl impl, DispatchOutputStream
synchronized (engine) {
try {
engine.preInitializedContext = PolyglotContextImpl.preInitialize(engine);
engine.addContext(engine.preInitializedContext);
} finally {
// Reset language homes from native-image compilatio time, will be recomputed in
// image execution time
Expand Down Expand Up @@ -1277,12 +1276,8 @@ public synchronized Context createContext(OutputStream configOut, OutputStream c
PolyglotContextImpl context = loadPreinitializedContext(config);
if (context == null) {
context = new PolyglotContextImpl(this, config);
addContext(context);
} else {
// don't add contexts for preinitialized contexts as they have been added already
assert Thread.holdsLock(this);
assert contexts.contains(context.weakReference);
}
addContext(context);

if (polyglotLimits != null) {
EngineLimits l = this.limits;
Expand Down Expand Up @@ -1317,7 +1312,7 @@ private PolyglotContextImpl loadPreinitializedContext(PolyglotContextConfig conf
patchResult = context.patch(config);
} finally {
if (!patchResult) {
context.closeImpl(false, false);
context.closeImpl(false, false, false);
context = null;
PolyglotContextImpl.disposeStaticContext(context);
config.fileSystem = oldFileSystem;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,11 +331,11 @@ Env requireEnv() {
return localEnv;
}

boolean finalizeContext() {
boolean finalizeContext(boolean notifyInstruments) {
if (!finalized) {
finalized = true;
LANGUAGE.finalizeContext(env);
if (eventsEnabled) {
if (eventsEnabled && notifyInstruments) {
EngineAccessor.INSTRUMENT.notifyLanguageContextFinalized(context.engine, context.truffleContext, language.info);
}
return true;
Expand Down Expand Up @@ -365,8 +365,8 @@ boolean dispose() {
return false;
}

void notifyDisposed() {
if (eventsEnabled) {
void notifyDisposed(boolean notifyInstruments) {
if (eventsEnabled && notifyInstruments) {
EngineAccessor.INSTRUMENT.notifyLanguageContextDisposed(context.engine, context.truffleContext, language.info);
}
language.freeInstance(lazy.languageInstance);
Expand Down

0 comments on commit 26ca9db

Please sign in to comment.