Skip to content

Commit

Permalink
Fix multi-context uninitialization if an engine is stored multiple ti…
Browse files Browse the repository at this point in the history
…mes.
  • Loading branch information
chumer committed Nov 2, 2020
1 parent debc432 commit 3e29bb3
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ public void preinitializeContext() {
GraalRuntimeAccessor.ENGINE.preinitializeContext(this.polyglotEngine);
}

public void finalizeStore() {
GraalRuntimeAccessor.ENGINE.finalizeStore(this.polyglotEngine);
}

public Object getEngineLock() {
return GraalRuntimeAccessor.ENGINE.getEngineLock(this.polyglotEngine);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,8 @@ public abstract <T, G> Iterator<T> mergeHostGuestFrames(StackTraceElement[] host

public abstract void preinitializeContext(Object polyglotEngine);

public abstract void finalizeStore(Object polyglotEngine);

public abstract Object getEngineLock(Object polyglotEngine);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,11 @@ public void preinitializeContext(Object polyglotEngine) {
((PolyglotEngineImpl) polyglotEngine).preInitialize();
}

@Override
public void finalizeStore(Object polyglotEngine) {
((PolyglotEngineImpl) polyglotEngine).finalizeStore();
}

@Override
public Object getEngineLock(Object polyglotEngine) {
return ((PolyglotEngineImpl) polyglotEngine).lock;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,11 @@ boolean patch(DispatchOutputStream newOut,
this.engineOptionValues = engineOptions;
this.logLevels = newLogConfig.logLevels;
this.storeEngine = RUNTIME.isStoreEnabled(engineOptions);

if (storeEngine && boundEngine && singleContext.isValid()) {
singleContext.invalidate();
}

INSTRUMENT.patchInstrumentationHandler(instrumentationHandler, newOut, newErr, newIn);

Map<PolyglotLanguage, Map<String, String>> languagesOptions = new HashMap<>();
Expand Down Expand Up @@ -526,23 +531,6 @@ private static void registerShutDownHook() {
}
}

void uninitializeMultiContext() {
assert Thread.holdsLock(this.lock);

/*
* If we store an engine we force initialize multi context to avoid language to do any
* context related references in the AST, but after, at least for context bound engines we
* can restore the single context assumption.
*/
if (storeEngine && boundEngine && !singleContext.isValid()) {
singleContext = Truffle.getRuntime().createAssumption("Single context after preinitialization.");
}

// much more things should be done here, like trying to use single context references
// for stored engines and then patch them later on.

}

void initializeMultiContext(PolyglotContextImpl existingContext) {
synchronized (this.lock) {
if (singleContext.isValid()) {
Expand Down Expand Up @@ -1211,9 +1199,11 @@ public Set<Source> getCachedSources() {
}
}
}
for (PolyglotLanguage language : idToLanguage.values()) {
for (PolyglotLanguageInstance instance : language.getInstancePool()) {
instance.listCachedSources(sources);
synchronized (lock) {
for (PolyglotLanguage language : idToLanguage.values()) {
for (PolyglotLanguageInstance instance : language.getInstancePool()) {
instance.listCachedSources(sources);
}
}
}
return sources;
Expand Down Expand Up @@ -1246,10 +1236,25 @@ OptionDescriptors getAllOptions() {
void preInitialize() {
synchronized (this.lock) {
this.preInitializedContext.set(PolyglotContextImpl.preInitialize(this));
this.uninitializeMultiContext();
}
}

void finalizeStore() {
assert Thread.holdsLock(this.lock);

/*
* If we store an engine we force initialize multi context to avoid language to do any
* context related references in the AST, but after, at least for context bound engines we
* can restore the single context assumption.
*/
if (storeEngine && boundEngine && !singleContext.isValid()) {
singleContext = Truffle.getRuntime().createAssumption("Single context after preinitialization.");
}

// much more things should be done here, like trying to use single context references
// for stored engines and then patch them later on.
}

/**
* Clears the pre-initialized engines. The TruffleFeature needs to clean emitted engines during
* Feature.cleanup.
Expand Down

0 comments on commit 3e29bb3

Please sign in to comment.