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

[Prasanta Sadhukhan]

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 
> <mailto: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
>     <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
>     <mailto: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
>         <https://bugs.openjdk.java.net/browse/JDK-8175968>
>         webrev:
>         http://cr.openjdk.java.net/~psadhukhan/8175968/webrev.00/
>         <http://cr.openjdk.java.net/%7Epsadhukhan/8175968/webrev.00/>
>
>         Regards
>         Prasanta
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20170705/8813935c/attachment.html>