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

[Kevin Rushforth]

On Fri, 26 Mar 2021 12:02:40 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
>   Minor cleanup based on codereview

The fix looks OK, although I left a question below about ensuring that there cannot be an NPE.

The test doesn't catch the bug on my system (Windows 10). Even when I run it without your fix, it passes. Is it platform-specific?

modules/javafx.graphics/src/main/java/javafx/scene/CssStyleHelper.java line 355:

> 353:     /* This is the first Styleable parent (of Node this StyleHelper belongs to)
> 354:      * having a valid StyleHelper */
> 355:     private WeakReference<Node> firstStyleableAncestor = null;

Since `firstStyleableAncestor` is initialized to `null`, rather than `new WeakReference(null)`, it might be possible for a call to `firstStyleableAncestor.get()` to throw an NPE. Can you comment on what analysis and testing you've done to ensure that this won't happen in some situations? A quick look shows that in the two places a `CssStyleHelper` object is created, `firstStyleableAncestor` is set shortly after, but it wouldn't hurt to take a closer look.

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

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