Improve thread safety for pkl:base (#1719)

Stdlib modules are singletons that are shared across multiple evaluators.
This improves thread safety by evaluating its `output.bytes` during
initialization, which initializes truffle nodes (e.g. TypeTestNode),
and also initializes the member cache of the module output of `pkl:base`
This commit is contained in:
Daniel Chao
2026-07-06 11:19:29 -07:00
committed by GitHub
parent 2ec83198f5
commit eaf73af247
2 changed files with 42 additions and 4 deletions
@@ -15,6 +15,8 @@
*/ */
package org.pkl.core.runtime; package org.pkl.core.runtime;
import static org.pkl.core.PClassInfo.pklBaseUri;
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
import java.net.URI; import java.net.URI;
import java.util.List; import java.util.List;
@@ -27,23 +29,33 @@ import org.pkl.core.http.HttpClient;
import org.pkl.core.module.ModuleKeyFactories; import org.pkl.core.module.ModuleKeyFactories;
import org.pkl.core.module.ModuleKeys; import org.pkl.core.module.ModuleKeys;
import org.pkl.core.module.ResolvedModuleKey; import org.pkl.core.module.ResolvedModuleKey;
import org.pkl.core.resource.ResourceReader;
import org.pkl.core.resource.ResourceReaders;
public abstract class StdLibModule { public abstract class StdLibModule {
@TruffleBoundary @TruffleBoundary
protected static void loadModule(URI uri, VmTyped instance) { protected static void loadModule(URI uri, VmTyped instance) {
// evaluate eagerly to increase thread safety doLoad(uri, instance);
// (stdlib module objects are statically shared singletons when running on JVM) }
// and ensure compile-time evaluation in AOT mode
private static void doLoad(URI uri, VmTyped instance) {
VmUtils.createContext( VmUtils.createContext(
() -> { () -> {
var vmContext = VmContext.get(null); var vmContext = VmContext.get(null);
var isPklBaseModule = uri.equals(pklBaseUri);
var resourceReaders =
isPklBaseModule
// needed when initializing `pkl:base` because of
// `read("prop:pkl.outputFormat")`
? List.of(ResourceReaders.externalProperty())
: List.<ResourceReader>of();
vmContext.initialize( vmContext.initialize(
new VmContext.Holder( new VmContext.Holder(
StackFrameTransformers.defaultTransformer, StackFrameTransformers.defaultTransformer,
SecurityManagers.defaultManager, SecurityManagers.defaultManager,
HttpClient.dummyClient(), HttpClient.dummyClient(),
new ModuleResolver(List.of(ModuleKeyFactories.standardLibrary)), new ModuleResolver(List.of(ModuleKeyFactories.standardLibrary)),
new ResourceManager(SecurityManagers.defaultManager, List.of()), new ResourceManager(SecurityManagers.defaultManager, resourceReaders),
Loggers.noop(), Loggers.noop(),
Map.of(), Map.of(),
Map.of(), Map.of(),
@@ -67,6 +79,15 @@ public abstract class StdLibModule {
// (stdlib module objects are statically shared singletons when running on JVM) // (stdlib module objects are statically shared singletons when running on JVM)
// and ensure compile-time evaluation in AOT mode // and ensure compile-time evaluation in AOT mode
instance.force(false, true); instance.force(false, true);
// seed base module's `output` members; `output` contains truffle nodes that
// need to be initialized statically (e.g. LetExprNode, TypeTestNode).
// additionally, its `cachedMembers` is not thread-safe and needs to be initialized
// statically.
if (isPklBaseModule) {
var output = VmUtils.readModuleOutput(instance);
output.force(false, true);
}
}) })
.close(); .close();
} }
@@ -772,6 +772,23 @@ class EvaluatorTest {
.doesNotThrowAnyException() .doesNotThrowAnyException()
} }
@Test
fun `concurrent evals`() {
val exceptions = mutableListOf<Throwable>()
val threads =
(0..10).map {
Thread { Evaluator.preconfigured().use { ev -> ev.evaluateOutputText(text("foo = 1")) } }
.also { t ->
t.uncaughtExceptionHandler = Thread.UncaughtExceptionHandler { _, e ->
synchronized(exceptions) { exceptions.add(e) }
}
t.start()
}
}
for (t in threads) t.join()
exceptions.firstOrNull()?.let { throw it }
}
private fun checkModule(module: PModule) { private fun checkModule(module: PModule) {
assertThat(module.properties.size).isEqualTo(2) assertThat(module.properties.size).isEqualTo(2)
assertThat(module.getProperty("name")).isEqualTo("pigeon") assertThat(module.getProperty("name")).isEqualTo("pigeon")