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.sadhuk...@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 <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/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.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 >>> webrev: http://cr.openjdk.java.net/~psadhukhan/8175968/webrev.00/ >>> >>> Regards >>> Prasanta >>> >> >> > >
