RFR: 8295506: ButtonBarSkin: memory leak when changing skin [v2]

Andy Goryachev angorya at openjdk.org
Thu Dec 1 16:43:10 UTC 2022


On Thu, 1 Dec 2022 11:11:31 GMT, Ajit Ghaisas <aghaisas at openjdk.org> wrote:

>> Andy Goryachev has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 23 commits:
>> 
>>  - Merge remote-tracking branch 'origin/master' into
>>    8295506.button.bar.skin
>>  - Merge remote-tracking branch 'origin/master' into
>>    8295506.button.bar.skin
>>  - Merge branch '8294809.listener.helper' into 8295506.button.bar.skin
>>  - 8294809: generics
>>  - Merge branch '8294809.listener.helper' into 8295506.button.bar.skin
>>  - 8294809: is alive
>>  - Revert "8294809: removed weak listeners support"
>>    
>>    This reverts commit 2df4a85db638d76cacaf6c54ba669cdb3dd91a18.
>>  - 8295506: button bar skin
>>  - 8294809: removed weak listeners support
>>  - 8294809: use weak reference correctly this time
>>  - ... and 13 more: https://git.openjdk.org/jfx/compare/0a785ae0...759ecaf4
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ButtonBarSkin.java line 139:
> 
>> 137:         }
>> 138: 
>> 139:         updateButtonListeners(getSkinnable().getButtons(), false);
> 
> Can we use `ListenerHelper` in `updateButtonListeners` method?
> We may need to use `ListernerHelper` of `ButtonSkin`.

good question.

I did not want to make drastic structural changes for risk of introducing regression.  Right now, this skin uses the same pattern when dealing with multiple child items - it adds/removes listeners in the constructor, in the dispose() method, and upon any changes to the observable list.  Perhaps not the most optimal solution, but it works and causes no trouble.  I'd rather keep it as is.

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

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


More information about the openjfx-dev mailing list