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 <steven...@gmail.com
<mailto:steven...@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.sadhuk...@oracle.com
<mailto:prasanta.sadhuk...@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
