Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8245303

InputMap: memory leak due to incomplete cleanup on remove mapping



      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);

                  // 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
                   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:

          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 -> {} );
              assertEquals("sanity: mapping added", 1, inputMap.get().getMappings().size());
              assertEquals("sanity: mapping removed", 0, inputMap.get().getMappings().size());
              assertNull("inputMap must be gc'ed", inputMap.get());


          Issue Links



              • Assignee:
                fastegal Jeanette Winzenburg
                fastegal Jeanette Winzenburg
              • Votes:
                0 Vote for this issue
                1 Start watching this issue


                • Created: