RFR: 8263402: MemoryLeak: Node hardreferences it's previous Parent after csslayout and getting removed from the scene [v3]

Ambarish Rapte arapte at openjdk.java.net
Tue Mar 23 09:59:42 UTC 2021


On Mon, 22 Mar 2021 14:30:58 GMT, Florian Kirmaier <fkirmaier at openjdk.org> wrote:

>> Fixing a memory leak. 
>> A node hard references its old parent after CSS layout and getting removed. 
>> This shouldn't be the case, this is very counterintuitive.
>> 
>> The fix uses a WeakReference in CSSStyleHelper for firstStyleableAncestor.
>> This should be fine because the CSS should only depend on it if it's still the real parent. 
>> In that case, it doesn't get collected.
>
> Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8263402
>   Rewrote the style memoryleak test

Provided few comments on test.

tests/system/src/test/java/test/javafx/scene/StyleMemoryLeakTest.java line 48:

> 46: import static org.junit.Assert.fail;
> 47: import static org.junit.Assert.assertTrue;
> 48: 

The packages Application, Node and Parent are not used.

tests/system/src/test/java/test/javafx/scene/StyleMemoryLeakTest.java line 59:

> 57:             startupLatch.countDown();
> 58:         });
> 59:         assertTrue("Timeout waiting for FX runtime to start", startupLatch.await(15, TimeUnit.SECONDS));

The runnable passed to `Platform.startup()` is executed on JavaFX Application thread once it is started. `Platform.setImplicitExit(false);` can be called from any thread. So we do not need to pass the runnable and do not need CountDownLatch.
This block can be replaced by,
Platform.startup(null);
Platform.setImplicitExit(false);

tests/system/src/test/java/test/javafx/scene/StyleMemoryLeakTest.java line 100:

> 98:         Platform.runLater(() -> {
> 99:             Platform.exit();
> 100:         });

`Platform.exit()` can be called from any thread. So `Platform.runLater` is not required.

tests/system/src/test/java/test/javafx/scene/StyleMemoryLeakTest.java line 102:

> 100:         });
> 101:     }
> 102: }

Include a new line at end of file.

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

Changes requested by arapte (Reviewer).

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


More information about the openjfx-dev mailing list