On Sat, 13 Jun 2026 15:29:34 GMT, Marius Hanl <[email protected]> wrote:
> I agree, but my thought behind was not the GC, but rather this: > > We can change just the skin at runtime. In this case we would still have > stale references to the `disclosureNode` here in the `Map`. And the > `disclosureNode` can still exist after we changed the skin, as the `TableRow` > is unchanged (and the `disclosureNode` comes from it). Then this skin might > not be GC'ed at all. The disclosure node must not strongly reference the skin after the skin is removed. If it does, it's a bug that needs to be fixed, because that would mean that a removed skin will stick around and not be collected. I don't think there is a bug here: it's okay if the skin strongly references the disclosure node; it's just the other way that would be a problem. > Also it is allowed to re-use a skin. So e.g. we could replace the skin of > `TableRow` with another one, and then back to the previous one. I don't think > this will work right now because everything is done in the constructor (and > not in `install()`), but that might be possible in the future. No, it's not allowed to reuse a skin. In the `SkinBase(C)` constructor, the `control` field is assigned to hold the associated control. This field is nulled in `SkinBase.dispose()`, so installing the same skin again on the control will fail with NPE if the `control` field is dereferenced at any point. > I agree both are rare situation and something you most likely don't do. But > since it does not hurt to clear/null the fields here, and we are improving > the skin in case it is not possible to dispose (GC) it immediately, I think > we should rather do it and are safe for whatever a developer might do. It _does_ hurt to null fields of a disposed object, because it suggests that this would do anything meaningful (like fixing a memory leak). But that's not the case: if there was a memory leak (i.e. if the skin wasn't eligible for GC and therefore kept its referenced objects alive), nulling the field would just hide the problem. The solution in that case would be to ensure that a disposed skin is eligible for GC. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2177#discussion_r3408249369
