Other |
---|
tbdUnresolved |
Blocks :
|
|
Relates :
|
Below is a test that creates an inputMap, adds and removes a single mapping, the inputMap should be eligible for gc but isn't. This seems to be a left-over of JDK-8150636, according to a code comment in removeMapping private void removeMapping(Mapping<?> mapping) { EventType<?> et = mapping.getEventType(); if (this.eventTypeMappings.containsKey(et)) { List<?> _eventTypeMappings = this.eventTypeMappings.get(et); _eventTypeMappings.remove(mapping); // TODO remove the event handler in the root if there are no more mappings of this type // anywhere in the input map tree } } Guessing a bit: the todo wasn't handled due using a similar (incorrect) pattern for adding/removing the eventHandler as adding/removing a listener in ButtonBehavior JDK-8245282: // addMapping -> calls rootInputMap.addEventHandler(type) // the latter is implemented as void addEventHandler(type) { ... final EventHandler<? super Event> eventHandler = this::handle; node.addEventHandler(et, eventHandler); } Note the lambda, each time a new instance even though the intention is (afaics) to handle all types with the same handler, that is the InputMap itself. A way might be the same as in ButtonBehavior: have a single instance for the handler and add/remove that instance on add/remove mappings: final EventHandler<? super Event> eventHandler = this::handle; void addEventHandler(type) { ... if (not-yet-added) node.addEventHandler(type, eventHandler); } void removeMapping(mapping) { if (has-mapping) { // cleanup internal bookkeeping eventTypeMappings.remove(mapping) node.removeEventHandler(type, eventHandler) } With the change the test below passes. Needs further evaluation, though, all tests are commented (and some use api that didn't make it into the release). The failing test: @Test public void testInputMapMemoryLeak() { Label label = new Label(); WeakReference<InputMap<?>> inputMap = new WeakReference<>(new InputMap<Label>(label)); // do-nothing mapping KeyMapping mapping = new KeyMapping(SPACE, KeyEvent.KEY_PRESSED, e -> {} ); inputMap.get().getMappings().add(mapping); assertEquals("sanity: mapping added", 1, inputMap.get().getMappings().size()); inputMap.get().getMappings().remove(mapping); assertEquals("sanity: mapping removed", 0, inputMap.get().getMappings().size()); attemptGC(inputMap); assertNull("inputMap must be gc'ed", inputMap.get()); }