On Mon, 6 Jul 2026 17:34:39 GMT, Andy Goryachev <[email protected]> wrote:
>> This PR removes the stage from the active windows list after a destroy >> notification, preventing a possible NPE in `WindowStage::setIconified`. >> >> A system test that fails before the proposed fix, and passes after it, has >> been included. >> >> --------- >> - [X] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassWindowEventHandler.java > line 136: > >> 134: case WindowEvent.DESTROY: >> 135: stage.setPlatformWindowClosed(); >> 136: WindowStage.removeActiveWindow(stage); > > The fix seems to be working (tested on macOS and windows). > > One peculiar thing I noticed is that if I set a breakpoint in WindowStage:582 > in the the master branch and run the test hitting the breakpoint, it won't > fail in macOS (but fails in windows). > > The main question is why `removeActiveWindow()` is called before the > `stage.stageListener.closed()`, somehow it does not seem right. > > I wonder if the order should have been > > case WindowEvent.DESTROY: > stage.stageListener.closed(); > stage.setPlatformWindowClosed(); > > > In fact, the test passes in this case without needing `removeActiveWindow()`, > as it's being called inside `stage.stageListener.closed();` > > What do you think about that? As it is before this PR: When `stage.setPlatformWindowClosed()` is called, it clears `WindowStage::platformWindow`, `stage.stageListener.closed()` ends up calling `WindowStage::handleFocusDisabled`, which takes the last windowStage in the `activeWindows` list, which is not null, but its platformWindow is, causing the NPE. With my proposed fix: `stage.setPlatformWindowClosed()` -> clears `WindowStage::platformWindow`, `WindowStage.removeActiveWindow(stage)` -> removes windowStage from the `activeWindows` list, `stage.stageListener.closed()` -> `WindowStage::handleFocusDisabled` bails out, no active windows, so no NPE. With your suggestion: `stage.stageListener.closed()` -> `WindowStage::handleFocusDisabled` which takes the last windowStage, which still has a valid `platformWindow`, so no NPE, and then `setVisible(false)` removes windowStage from the `activeWindows` list, `stage.setPlatformWindowClosed()` -> does nothing, the platformWindow was already cleared, so this call seems redundant. In both cases the NPE is fixed, but it seems that this will do it with less calls: case WindowEvent.DESTROY: stage.stageListener.closed(); Does that make sense? ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2202#discussion_r3532795103
