RFR: 8371386: Fix potential NPE in SwingNode
Kevin Rushforth
kcr at openjdk.org
Thu Nov 6 22:11:19 UTC 2025
On Thu, 6 Nov 2025 03:57:51 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:
> There can be potential NPEs in SwingNode which is fixed
The changes look reasonable, for a start. I decided to look more closely at the threads the methods are being called on, and I found two related threading problems and one more place where you should locally capture `lwFrame`.
### Methods called on wrong thread
#### notifyNativeHandle
The `notifyNativeHandle` method is meant to be called on the FX thread. Two of the three call sites are correct, but one of them incorrectly calls it from the EDT:
/*
* Called on EDT
*/
private void setContentImpl(JComponent content) {
...
if (getScene() != null) {
notifyNativeHandle(getScene().getWindow());
}
Since, the very next line of code in this method executes a block on the FX app thread, it should be as simple as moving the above block into the body of the `runOnFxThread` immediately below it.
#### locateLwFrame
The `locateLwFrame` method is meant to be called on the FX thread. Three of the four call sites are correct, but one of them incorrectly calls it from the EDT:
SwingNodeHelper.runOnEDT(() -> {
if (lwFrame != null) {
locateLwFrame();
}
});
For some reason this code goes out of its way to jump on the EDT to call a method that is called from the FX thread in all other cases. Removing the runOnEDT call should be the right fix. Also, the check for `lwFrame != null` is unnecessary here. `locateLwFrame` already checks for null (and none of the other 3 callers of that method check prior to calling).
### Local capture of `lwFrame` needed
The `windowHiddenHandler` lambda is called on the FX thread. As such, the following is one more place where you should capture `lwFrame` into a local variable so you can test it for non-null and then be sure you are using that still non-null value:
if (isThreadMerged) {
swNodeIOP.overrideNativeWindowHandle(lwFrame, 0L, null);
Suggestion:
if (isThreadMerged) {
var hFrame = lwFrame;
If (hFrame != null) {
swNodeIOP.overrideNativeWindowHandle(hFrame, 0L, null);
}
-------------
PR Review: https://git.openjdk.org/jfx/pull/1965#pullrequestreview-3430748790
More information about the openjfx-dev
mailing list