RFR: 8208088: Memory Leak in ControlAcceleratorSupport [v3]
Kevin Rushforth
kcr at openjdk.java.net
Sat Apr 10 15:01:19 UTC 2021
On Fri, 9 Apr 2021 06:15:42 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:
>> The fix looks good.
>>
>> The test looks OK as far as it goes, although I note that it is testing for the leak in an indirect way, since it looks at the `changeListenerMap` rather than checking the listeners of `menuitem.acceleratorProperty()` which is what we really care about. Is there a way to test it more directly? If not then I think this is fine.
>
>> Is there a way to test it more directly?
>
> `ControlTestUtils` provides a way to get the number of listeners added to a property. Please check the updated test. With this direct way, should we keep the indirect way of testing which uses `changeListenerMap` ?
The updated test looks good. I confirm that it fails without the fix and passes with the fix. In order to do that I locally deleted the newly added shim class and comment out all calls to `get_ListenerMapSize`. I'd recommend to get rid of the indirect way of testing, since it will mean you can remove the newly added `ControlAcceleratorSupportShim` class and the `ControlAcceleratorSupport ::testGetListenerMapSize` method.
-------------
PR: https://git.openjdk.java.net/jfx/pull/429
More information about the openjfx-dev
mailing list