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