On Thu, 6 Nov 2025 03:57:51 GMT, Prasanta Sadhukhan <[email protected]>
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
