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

[Robin Stevens]

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/
> santhosh/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/20170704/84e5aa9f/attachment.html>