Alexandr,
that sounds right.
It took me a while to figure out the exact circumstances to trigger this
memory leak.
In my application where I found this bug, I have a progress bar which is
repeatedly switched from between visible and invisible, and between
determinate and indeterminate.
I could probably work around this issue by altering the sequence in which
setVisible and setIndeterminate is called, but I can just as well fix the
problem in the JDK.
Anyway, adding the isDisplayable checks in the AquaProgressBarUI class
seems to work. Test runs fine when those are added.
I will send a new email to the list tomorrow with a request for a review.
Thanks for your valuable input,
Robin
On Tue, Jul 19, 2016 at 6:06 PM, Alexandr Scherbatiy <
alexandr.scherba...@oracle.com> wrote:
> On 7/19/2016 4:33 PM, Robin Stevens wrote:
>
> Hello Alexandr,
>
> very valid remark.
> Running that same test program on Linux with the metal look and feel
> reveals no memory leak. I have no access to a Windows machine, so I
> couldn't get the Windows specific look and feel.
>
> The other ProgressBarUI implementations seem to extend from
> BasicProgressBarUI, which has the same mechanism of an Animator which uses
> a Timer.
> However, in the test program the Timer does not get started on Linux
> (while it gets started on OS X).
>
> In the BasicProgressBarUI class, all calls to startAnimationTimer are
> wrapped with an if check:
>
> if (progressBar.isDisplayable()) {
> startAnimationTimer();
> }
>
> In the scenario from my test, the isDisplayable method returns false. On
> OS X, this check is missing so the timer is started.
>
> I believe that the changed AquaProgressBarUIMemoryLeakTest where the
> progress bar is visible and indeterminate value is set to true at the end
> should also not have the memory leaks.
>
> Thanks,
> Alexandr.
>
>
> I assume adding that same check in the AquaProgressBarUI will fix the
> problem as well. So that is a third approach to solve the issue.
>
> Robin
>
> On Tue, Jul 19, 2016 at 1:14 PM, Alexandr Scherbatiy <
> alexandr.scherba...@oracle.com> wrote:
>
>> On 7/19/2016 12:27 PM, Robin Stevens wrote:
>>
>>> Hello,
>>>
>>> I wanted to discuss my approach for issue JDK-8161664 (
>>> https://bugs.openjdk.java.net/browse/JDK-8161664) before I started
>>> working on this issue.
>>>
>>> In certain scenarios (see the JIRA issue for an example), the Timer in
>>> the Animator inner class of the AquaProgressBarUI class remains running,
>>> even when the JProgressBar has already been removed from the UI. This
>>> causes a memory leak, as that running Timer avoids that the JProgressBar
>>> can be GC-ed. As long as the Timer is running, the JProgressBar is
>>> referenced through
>>>
>>> Timer -> ActionListener (=Animator inner class) -> AquaProgressBarUI
>>> outer class -> JProgressBar field
>>>
>>>
>>>
>>> I see two possible approaches to fix this:
>>>
>>> 1) I carefully investigate the particular scenario I found, and try to
>>> figure out why the Timer is not stopped and fix this particular scenario.
>>> This offers of course no guarantees that there are no other scenarios which
>>> keep the Timer running.
>>>
>>> 2) I replace one of the hard references with a weak reference, hence
>>> avoiding the memory leak in all cases.
>>> If I do not attach the Animator inner class directly as listener to the
>>> timer, but use another ActionListener which only has a WeakReference to the
>>> Animator class, the memory leak is solved.
>>> The ActionListener could then stop the timer when the timer is fired and
>>> the WeakReference#get returns null.
>>>
>>>
>>>
>>> I prefer the second approach. By cutting the hard reference between the
>>> Timer and the Animator + stopping the Timer when the Animator is GC-ed, I
>>> ensure that the Timer cannot cause a memory leak anymore. This avoids
>>> overlooking certain scenarios.
>>>
>>> Any input on this ? Any preferences for a certain approach, or proposal
>>> for another approach.
>>>
>> Does other L&Fs (for example Metal) have the same memory leak with the
>> JProgressBar? If no, it would be interesting to know what is the difference
>> between them and the AquaProgressBarUI.
>>
>> Thanks,
>> Alexandr.
>>
>>>
>>>
>>> Robin
>>>
>>
>>
>
>
