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