[Rev 01] RFR: 8241840: Memoryleak: Closed focused Stages are not collected with Monocle.

Ambarish Rapte arapte at openjdk.java.net
Fri Apr 17 13:00:19 UTC 2020


On Tue, 14 Apr 2020 11:15:32 GMT, Florian Kirmaier <fkirmaier at openjdk.org> wrote:

>> Closed focused Stages are not collected with Monocle
>> 
>> This commit adds a unit-test and a fix for collecting focused closed stages.
>> 
>> ticket: https://bugs.openjdk.java.net/browse/JDK-8241840
>
> Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision:
> 
>   JDK-8241840
>   Some code cleanups

Changes requested by arapte (Reviewer).

modules/javafx.graphics/src/main/java/com/sun/glass/ui/monocle/MonocleWindowManager.java line 124:

> 123:         int index = getWindowIndex(window);
> 124:         if (index != -1 && window.isVisible()) {
> 125:             focusedWindow = window;

This change is sufficient for the test to pass, are there any other scenarios for which the changes in Window.java are
needed. If so could you please add the relevant tests.

tests/system/src/test/java/test/javafx/stage/FocusedWindowTest.java line 103:

> 102:         assertCollectable(closedFocusedStageWeak);
> 103:     }
> 104:

The current test does not assure that stage is shown when `close()` is called or it is closed when `requestFocus()` is
called, I would recommend the test to be as,

    @Test
    public void testClosedFocusedStageLeak() throws Exception {
        CountDownLatch latch = new CountDownLatch(1);
        Util.runAndWait(() -> {
            closedFocusedStage = new Stage();
            closedFocusedStage.setTitle("Focused Stage");
            closedFocusedStageWeak = new WeakReference<>(closedFocusedStage);
            TextField textField = new TextField();
            closedFocusedStage.setScene(new Scene(textField));
            closedFocusedStage.setOnShown(l -> {
                latch.countDown();
            });
            closedFocusedStage.show();
        });
        Assert.assertTrue("Timeout waiting for closedFocusedStage to show`",
                      latch.await(15, TimeUnit.MILLISECONDS));

        CountDownLatch hideLatch = new CountDownLatch(1);
        closedFocusedStage.setOnHidden(a -> {
            hideLatch.countDown();
        });
        Util.runAndWait(() -> closedFocusedStage.close());
        Assert.assertTrue("Timeout waiting for closedFocusedStage to hide`",
                      hideLatch.await(15, TimeUnit.MILLISECONDS));

        closedFocusedStage.requestFocus();
        closedFocusedStage = null;
        assertCollectable(closedFocusedStageWeak);
    }

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

PR: https://git.openjdk.java.net/jfx/pull/153


More information about the openjfx-dev mailing list