akashchamp opened a new pull request, #396: URL: https://github.com/apache/commons-scxml/pull/396
Fixes SCXML-290: https://issues.apache.org/jira/browse/SCXML-290 ## Problem `JSEvaluator#initGlobalsScript` is a `private static String` that caches the contents of the `init_global.js` classpath resource. It is written exactly once, inside the per-instance `synchronized initEngine()` method: ```java protected synchronized void initEngine() { if (engine == null) { engine = new ScriptEngineManager().getEngineByName("JavaScript"); if (initGlobalsScript == null) { initGlobalsScript = IOUtils.toString(...); } } } ``` but it is read without any synchronization elsewhere, e.g. in `getScriptContext()`: ```java getEngine().eval(initGlobalsScript, scriptContext); ``` `initEngine()` synchronizes on the individual `JSEvaluator` instance (`this`). Separate `JSEvaluator` instances (one per SCXML instance, per the class's own Javadoc) are routinely constructed and used concurrently by different threads, so the write on one instance's monitor and a read via another instance's `getScriptContext()`/`initEngine()` do not share a common monitor. The Java Memory Model therefore does not guarantee that a thread will observe the script content published by another thread's `initEngine()` call, and may see a stale or `null` value. ## Fix Mark `initGlobalsScript` `volatile`. This establishes the required happens-before edge between any write and any subsequent read of the field, regardless of which instance or monitor performed the write, closing the visibility gap directly at its source without changing the existing per-instance locking structure. ## Tests Added `JSEvaluatorTest#testConcurrentEvaluatorInitialization`, which constructs and evaluates 32 `JSEvaluator` instances concurrently from a thread pool, exercising the previously-racy lazy-initialization path under real thread contention and failing loudly on any exception or incorrect result. Note: a Java Memory Model visibility bug like this one is inherently non-deterministic to reproduce as a hard test failure in a single JVM run (it depends on the JIT, CPU cache behavior, and scheduling, none of which a portable unit test controls). The added test provides concurrency coverage of the affected code path but cannot itself prove the pre-fix race; the fix is otherwise a direct, minimal application of the standard JMM remedy for this exact pattern (write under one lock, read without a shared lock). ## Verification - `mvn clean verify` — **BUILD SUCCESS**, 241 tests run, 0 failures, 0 errors, 0 skipped (includes RAT license check and Jacoco coverage check). - `mvn test -Dtest=JSEvaluatorTest` — 12/12 tests pass (11 existing + 1 new). --- - [x] Read the [contribution guidelines](CONTRIBUTING.md) for this project. - [x] Read the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) if you use Artificial Intelligence (AI). - [x] I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute? — Claude (Anthropic), model Claude Sonnet 5, operated under my direction end-to-end: root-cause analysis, the code and test changes, and this description. I reviewed the diff and ran the verification above myself before opening this PR. - [x] Run a successful build using the default [Maven](https://maven.apache.org/) goal with `mvn`; that's `mvn` on the command line by itself. - [x] Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice. (See the Tests/Verification notes above on the limits of testing a JMM visibility bug deterministically.) - [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why. - [x] Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
