On Fri, 19 Jan 2024 22:16:46 GMT, Phil Race <[email protected]> wrote:

>> When using a TreeCellRenterer which creates new components in 
>> getTreeCellRendererComponent() in a JTree that is not visible, changes to 
>> the nodes cause a memory leak.
>> When a node is changed, the Method getNodeDimensions() is called to 
>> calculate the new dimensions for the node. In this method, 
>> getTreeCellRendererComponent() is called to obtain the renderer component 
>> (what else...) and [this component is added to 
>> rendererPane](https://github.com/openjdk/jdk/blob/36f4b34f1953af736706ec67192204727808bc6c/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTreeUI.java#L3283-L3284).
>>  It is not removed from the rendererPane afterwards. 
>> Only when the tree is painted, the paint() method does a removeAll on the 
>> rendererPane [in this 
>> code](https://github.com/openjdk/jdk/blob/36f4b34f1953af736706ec67192204727808bc6c/src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTreeUI.java#L1500)
>> 
>> FIx is added to remove the components from rendererPane when the JTree UI is 
>> changed/uninstalled only when tree is not visible since they are already 
>> removed when tree is painted in paint() method..
>
> src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTreeUI.java line 
> 1338:
> 
>> 1336:         if(rendererPane != null) {
>> 1337:             if (!tree.isVisible()) {
>> 1338:                 rendererPane.removeAll();
> 
> if( -> if (
> 
> Is it actually necessary to check visibility ? 
> 
> But more importantly, in the case of the test in the bug, how and when does 
> this uninstallComponents() method get called ? It isn't clear to me how it 
> changes the reported behaviour of that test.
> 
> I thought uninstallComponents() is just part of uninstallUI() like when 
> tearing down and freeing the tree, or at least switching L&F .. whereas the 
> submitter seems to be implying nodeChanged() is the problem

Yes, the submitter mentioned during nodeChanged() but I mentioned it before [in 
this comment](https://github.com/openjdk/jdk/pull/17458#issuecomment-1897684665)
that the comment present, implied it's not an issue, if the component is not 
removed if UI is not changed (as it is in the case when tree is not 
visible)...I am not sure why it was added and when but should we ignore that 
comment and try to remove the component even if tree is not visible (and UI is 
not changing)? 

I went with the thought that the coder meant what he said and I freed the 
components at the end (as you told during tearing down) so that there is no 
leak..

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17458#discussion_r1461398341

Reply via email to