<Swing Dev> [9] Review Request: 8161664: Memory leak in com.apple.laf.AquaProgressBarUI: removed progress bar still referenced

[Sergey Bylokhov]

Hi, Robin.
These calls do no guaranties that all weak references will be collected.
56     System.runFinalization();
57     System.gc();

I suggest to generate OOM explicitly. see for example:
javax/swing/UIDefaults/6795356/bug6795356.java

Note that on other platforms it will cover Metal L&F but probably others 
can be tested as well. So it will be good to test all installed L&F in 
the loop.

On 20.07.16 14:26, Alexandr Scherbatiy wrote:
>
> The fix looks good to me.
>
> Thanks,
> Alexandr.
>
> On 7/20/2016 11:37 AM, Robin Stevens wrote:
>> Hello,
>>
>> please review this patch for issue JDK-8161664:
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8161664
>> Patch: http://cr.openjdk.java.net/~rstevens/8161664/webrev.00/
>> <http://cr.openjdk.java.net/%7Erstevens/8161664/webrev.00/>
>>
>>
>> The problem is that in certain scenarios, the Timer in the Animator of
>> the AquaProgressBarUI does not get stopped when the progress bar is
>> removed from the Swing hierarchy.
>>
>> Several approaches were discussed in another thread
>> (http://mail.openjdk.java.net/pipermail/swing-dev/2016-July/006330.html).
>> In the linked webrev, I opted to use the same approach as what is done
>> in the BasicProgressBarUI class: only start the timer when the
>> progress bar is displayable.
>>
>> To achieve this, I wrapped all calls to startAnimationTimer with an if
>> statement that checks the displayable state of the progress bar.
>>
>> There is one call to startAnimationTimer that is not adjusted. That
>> call is only triggered from the paint method, which in turn is only
>> triggered if the progress bar is displayable. As such, the if check
>> was not needed there (imo).
>>
>> The patch includes a test, which fails without the fix and succeeds
>> afterwards.
>>
>> Thanks,
>>
>> Robin
>


-- 
Best regards, Sergey.