RFR: 8255248: NullPointerException in JFXPanel due to race condition in HostContainer [v7]

[Kevin Rushforth]

On Wed, 19 Nov 2025 12:52:51 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> NPE is seen while accessing transient "scenePeer" variable between reads..
>> Fix is made to store it in a temp variable rather than reading it twice since the value can change between successive reads in many places it is accessed.
>> Also some debug logs added to be enabled via `jfxpanel.debug` property
>
> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Review comment

The fix now looks good with one question about the change in `EmbeddedScene`. I also looked at the implementation of the `EmbeddedStage` and `EmbeddedScene` methods, and they all do the right thing in that they forward the requests to the FX thread.

I do want to see a follow-up issue filed to consider redesigning the threading model, but I think this PR is a good workaround for the NPEs.

As for the newly added test, it passes with the fix and with no exception messages (good). However, at least one of the times I ran the test without the fix (using the existing 100 msec sleep), it printed a couple exceptions and passed anyway. It seems that some of the exceptions that can occur will cause the test to fail but others will not. You might consider using the `test.javafx.util.OutputRedirect` utility instead of relying on the `UncaughtExceptionHandler`. This might not be a problem in practice if you reduce the sleep time, but will make the test more likely to catch any problems.

modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/EmbeddedScene.java line 160:

> 158:                 if (getSceneState() != null) {
> 159:                     updateSceneState();
> 160:                 }

None of the other calls to `updateSceneState()` check for a null `sceneState`. Why is this is the only one that needs to? If it is needed, it might be safer to move the null check into `updateSceneState` itself.

tests/system/src/test/java/test/javafx/embed/swing/JFXPanelNPETest.java line 103:

> 101:             Thread.sleep(100);
> 102:             Platform.runLater(() -> contentPane.setScene(webView.getScene()));
> 103:             Thread.sleep(100);

With a `sleep(100)` this only occasionally gets an exception when I run it without your fix. I recommend `sleep(1)` which is what the manual test does. The test will take less time and also be a better stress test.

-------------

PR Review: https://git.openjdk.org/jfx/pull/1968#pullrequestreview-3482862662
PR Review Comment: https://git.openjdk.org/jfx/pull/1968#discussion_r2542150648
PR Review Comment: https://git.openjdk.org/jfx/pull/1968#discussion_r2542248037