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

ChoiceBox/ToolBarSkin: misbehavior on switching skin

    Details

      Description

      turned up in binch testing (see JDK-8244531), the issues are

      - memory leak
      - NPE thrown in old skin if items added after replacing the skin

      // test snippets

          @Test
          public void testMemoryLeakAlternativeSkin() {
              installDefaultSkin(control);
              WeakReference<?> weakRef = new WeakReference<>(control.getSkin());
              assertNotNull(weakRef.get());
              setAlternativeSkin(control);
              attemptGC(weakRef);
              assertEquals("Skin must be gc'ed", null, weakRef.get());
          }
          
          @Test
          public void testSideEffectAlternativeSkin() {
              if (sideEffect == null) return;
              installDefaultSkin(control);
              setAlternativeSkin(control);
              sideEffect.accept(control);
          }
          
      with sideEffect:

          (Consumer<Control>) c -> {
                ChoiceBox box = (ChoiceBox) c;
                       box.getItems().add("added");
           }

      culprits are manually added listeners to items / selectionModel.selectedIndex that are not removed

      Fix is to remove:

          /** {@inheritDoc} */
          @Override public void dispose() {
              // removing the content listener fixes NPE from listener
              if (choiceBoxItems != null) {
                  choiceBoxItems.removeListener(weakChoiceBoxItemsListener);
                  choiceBoxItems = null;
              }
              // removing the path listener fixes the memory leak on replacing skin
              if (selectionModel != null) {
                  selectionModel.selectedIndexProperty().removeListener(selectionChangeListener);
                  selectionModel = null;
              }
             ...
       

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                fastegal Jeanette Winzenburg
                Reporter:
                fastegal Jeanette Winzenburg
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: