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

[Robin Stevens]

Hello,

thank you both for the review.

Updated webrev: http://cr.openjdk.java.net/~rstevens/8161664/webrev.01/

I have adjusted the test as suggested by Sergey:

- I moved it to the javax/swing/JProgressBar package, as it is no longer
specific for the Aqua look and feel
- The test now loops over all installed look and feels. For that, I copied
code from the test in javax/swing/JTab;e/7124218/SelectEditTableCell.java
- The test now uses Util.generateOOME instead of the System.gc +
System.finalization call


Robin

On Mon, Jul 25, 2016 at 10:32 PM, Sergey Bylokhov <
Sergey.Bylokhov at oracle.com> wrote:

> 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.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160726/aa6f7c1c/attachment.html>