RFR: 8293444: Creating ScrollPane with same content component causes memory leak [v3]

[Andy Goryachev]

On Thu, 29 Sep 2022 08:01:55 GMT, Florian Kirmaier <fkirmaier at openjdk.org> wrote:

>> Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8293444: review comments
>
> Of course, you can keep it this way.
> 
> The point of JMemoryBuddy.memoryTest is, that the user doesn't have to fiddle around with WeakReferences.
> For the record, your test might look like this (not tested):
> 
>     @Test
>     public void testScrollPaneObjLeakWhenUsedSameContent() {
>         JMemoryBuddy.memoryTest(() -> {
>             BorderPane bp = new BorderPane();
> 
>             Stage stage = new Stage();
>             stage.setScene(new Scene(bp));
>             stage.show();
> 
>             Label content = new Label("content");
> 
>             ScrollPane sp1 = new ScrollPane(content);
>             bp.setCenter(sp1);
>             Toolkit.getToolkit().firePulse();
> 
>             ScrollPane sp2 = new ScrollPane(content);
>             bp.setCenter(sp2);
>             Toolkit.getToolkit().firePulse();
> 
>             bp.setCenter(null);
>             Toolkit.getToolkit().firePulse();
> 
>             // When the label is still referenced, then only one ScrollPane should stay which is its parent
>             checker.setAsReferenced(label)
>             checker.assertCollectable(sp1)
>             checker.assertNotCollectable(sp2)
>         });
>     }
> 
>     @Test
>     public void testScrollPaneObjLeakWhenUsedSameContent() {
>         JMemoryBuddy.memoryTest(() -> {
>             BorderPane bp = new BorderPane();
> 
>             Stage stage = new Stage();
>             stage.setScene(new Scene(bp));
>             stage.show();
> 
>             Label content = new Label("content");
> 
>             ScrollPane sp1 = new ScrollPane(content);
>             bp.setCenter(sp1);
>             Toolkit.getToolkit().firePulse();
> 
>             ScrollPane sp2 = new ScrollPane(content);
>             bp.setCenter(sp2);
>             Toolkit.getToolkit().firePulse();
> 
>             bp.setCenter(null);
>             Toolkit.getToolkit().firePulse();
> 
>             // IF our label is gone, both scrollpanes shold be collectable
>             checker.assertCollectable(sp1)
>             checker.assertCollectable(sp2)
>         });
>     }
> 
> I think only the first test is the important one.

Thank you for clarifications, @FlorianKirmaier

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

PR: https://git.openjdk.org/jfx/pull/900