diff --git a/src/main/java/org/apache/commons/scxml2/env/javascript/JSEvaluator.java b/src/main/java/org/apache/commons/scxml2/env/javascript/JSEvaluator.java index cde6dc763..4810bb188 100644 --- a/src/main/java/org/apache/commons/scxml2/env/javascript/JSEvaluator.java +++ b/src/main/java/org/apache/commons/scxml2/env/javascript/JSEvaluator.java @@ -83,7 +83,7 @@ public String getSupportedDatamodel() { + "expression, Context must be a org.apache.commons.scxml2.env.javascript.JSContext"; /** Nashorn Global initialization script, loaded from {@code init_global.js} classpath resource */ - private static String initGlobalsScript; + private static volatile String initGlobalsScript; /** Nashorn ScriptEngine **/ private transient ScriptEngine engine; diff --git a/src/test/java/org/apache/commons/scxml2/env/javascript/JSEvaluatorTest.java b/src/test/java/org/apache/commons/scxml2/env/javascript/JSEvaluatorTest.java index 0eadd07fd..c997661cd 100644 --- a/src/test/java/org/apache/commons/scxml2/env/javascript/JSEvaluatorTest.java +++ b/src/test/java/org/apache/commons/scxml2/env/javascript/JSEvaluatorTest.java @@ -24,7 +24,13 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.StringReader; +import java.util.List; import java.util.Map; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import org.apache.commons.scxml2.Context; import org.apache.commons.scxml2.Evaluator; @@ -140,6 +146,49 @@ void testBasic() throws SCXMLExpressionException { assertTrue((Boolean) evaluator.eval(context, "1+1 == 2")); } + /** + * SCXML-290: {@code JSEvaluator#initGlobalsScript} is a static field that is only ever written + * inside the per-instance {@code synchronized initEngine()} method, but was read without any + * synchronization elsewhere. Since the write and the read do not share a common monitor, the Java + * Memory Model does not guarantee that a thread constructing/using a fresh {@link JSEvaluator} + * instance will observe the value published by another instance's {@code initEngine()} call. + *
+ * This test exercises many {@link JSEvaluator} instances concurrently performing their first + * (lazy) engine initialization and evaluation. It cannot deterministically force the JMM + * visibility gap to manifest in a single JVM run, but it does exercise the previously-racy + * code path under real concurrency and fails loudly (instead of silently passing) if + * initialization throws or produces an incorrect result on any thread. + *
+ */ + @Test + void testConcurrentEvaluatorInitialization() throws Exception { + final int threadCount = 32; + final ExecutorService executor = Executors.newFixedThreadPool(threadCount); + final CountDownLatch ready = new CountDownLatch(threadCount); + final CountDownLatch start = new CountDownLatch(1); + final List