<Swing Dev> [10] RFR JDK-8175968: The javax.swing.filechooser.FileSystemView constructor consumes memory by adding a PropertyChangeListener that is never removed.

[Robin Stevens]

I don't need the credit. Feel free to simply adjust your fix.

Robin

On Wed, Jul 5, 2017 at 7:42 AM, Prasanta Sadhukhan <
prasanta.sadhukhan at oracle.com> wrote:

> Ok. Will you mind preparing a patch for your thought on this fix for us to
> credit you for the fix?
> Regards
> Prasanta
>
> On 7/4/2017 3:32 PM, Robin Stevens wrote:
>
> What I forgot to mention in my previous mail.
>
> The code of that PropertyChangeListener already does an attempt to be a
> weak listener, but the implementation is wrong.
> Because it is an anonymous class, the PropertyChangeListener itself keeps
> a hard reference to the FileSystemView instance.
> As such, the WeakReference used in the PropertyChangeListener never
> becomes null, and the listener and FileSystemView never get GC-ed.
>
> The anonymous class should be converted to a static inner class before
> this mechanism can work.
> Doing so will at least ensure that the PropertyChangeListener does not
> keep a hard reference to the FileSystemView class.
>
> Of course, in the scenario from the reported bug the
> PropertyChangeListeners will remain present on the UIManager until a
> PropertyChangeEvent is fired by the UIManager.. Not sure whether that is an
> acceptable trade-off for not having the dispose method in the API.
>
> On Tue, Jul 4, 2017 at 11:54 AM, Robin Stevens <stevensro at gmail.com>
> wrote:
>
>> - There should be no need to create a public dispose method and shift the
>> responsibility of disposing the listener to the user of the FileSystemView
>> class.
>>
>> You can simply convert the PropertyChangeListener to a so-called weak
>> listener, and keep this PropertyChangeListener an implementation detail
>> (see the WeakChangeListener in javafx.beans.value package for an example of
>> what I mean with a weak listener, or http://www.jroller.com/santhos
>> h/entry/use_weak_listeners_to_avoid).
>>
>> - The javadoc of that new dispose method is unclear. How do you suppose
>> somebody somebody will know what "the PropertyChangeListener" is. This
>> remark of course becomes irrelevant if you remove that method from the
>> public API.
>>
>> Robin
>>
>> On Tue, Jul 4, 2017 at 11:40 AM, Prasanta Sadhukhan <
>> prasanta.sadhukhan at oracle.com> wrote:
>>
>>> Hi All,
>>>
>>> Please review a fix for a memory leak issue where PropertyChangeListener
>>> object added by FileSystemView constructor is never removed.
>>> Proposed fix is to add dispose() method to be called by app when they
>>> want to remove this resource.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8175968
>>> webrev: http://cr.openjdk.java.net/~psadhukhan/8175968/webrev.00/
>>>
>>> Regards
>>> Prasanta
>>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20170705/88aaedff/attachment.html>