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

ToggleButton.setToggleGroup causes memory leak when button is removed via ToggleGroup.getToggles()

    Details

    • Subcomponent:
    • CPU:
      generic
    • OS:
      generic

      Description

      FULL PRODUCT VERSION :
      java version "1.8.0_141"
      Java(TM) SE Runtime Environment (build 1.8.0_141-b15)
      Java HotSpot(TM) 64-Bit Server VM (build 25.141-b15, mixed mode)

      ADDITIONAL OS VERSION INFORMATION :
      Linux [...] 3.10.0-693.17.1.el7.x86_64 #1 SMP Thu Jan 25 20:13:58 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux


      A DESCRIPTION OF THE PROBLEM :
      When ToggleButtons are added to a ToggleGroup using ToggleButton.setToggleGroup, but removed wihtout using ToggleButton.setToggleGroup (e.g. by calling ToggleGroup.getToggles().clear(), strong references to the buttons are retained and prevent the buttons from being garbage collected.

      As far as I can tell, the culprit is [ToggleButton.class, method toggleGroupProperty(), line 221] a listener registered by ToggleButton on the ToggleGroup's selectedToggleProperty() when ToggleButton.setToggleGroup is used - or more specifically, when the ToggleGroup of the ToggleButton is changed to a non-null ToggleGroup and the button is not yet part of ToggleGroup's toggles. That listener is only ever de-registered if the button's ToggleGroup is set to null. However, when toggles are removed from a ToggleGroup by working directly on ToggleGroup.getToggles(), the button's ToggleGroup is never reset (see ToggleGroup.java, definition of field toggles, line 76).

      There appear to be no issues if toggles are only added/removed using the same approach (either using ToggleButton.setToggleGroup OR working directly on ToggleGroup.getToggles()), or when adding the button directly to ToggleGroup.getToggles() but removing it via ToggleButton.setToggleGroup.

      I don't think the documentation clarifies anywhere that mixing both approaches as described can lead to memory leaks (and I think it should not happen any way).

      STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
      (See code example.)

      1. Create some ToggleButtons, store references to them in a WeakHashMap (to verify leak).
      2. Create a ToggleGroup. Add ToggleButtons to it using ToggleButton.setToggleGroup.
      3. Remove the ToggleButtons from the ToggleGroup by using methods of ToggleGroup.getToggles() , e.g. ToggleGroup.getToggles().clear() .
      4. Make sure you have no further strong references to the ToggleButtons.
      5. Call System.gc()
      6. Check if the WeakHashMap still contains references to the ToggleButtons.

      EXPECTED VERSUS ACTUAL BEHAVIOR :
      EXPECTED -
      All strong references to the ToggleButtons should have been removed, so the WeakHashMap should not contain any entries.
      ACTUAL -
      The WeakHashMap still contains entries for all ToggleButtons that were added to the ToggleGroup using ToggleButton.setToggleGroup .

      REPRODUCIBILITY :
      This bug can be reproduced always.

      ---------- BEGIN SOURCE ----------
       final ToggleGroup tg = new ToggleGroup();

              final WeakHashMap<ToggleButton, Object> weakMap = new WeakHashMap<>();

              // create some strong reference to toggle buttons
              ToggleButton tbNoLeak1 = new ToggleButton("TB_NO_LEAK_1");
              ToggleButton tbNoLeak2 = new ToggleButton("TB_NO_LEAK_2");
              ToggleButton tbLeak1 = new ToggleButton("TB_LEAK_1");
              ToggleButton tbLeak2 = new ToggleButton("TB_LEAK_2");

              // store some weak references to the buttons to check for leak
              weakMap.put(tbNoLeak1, null);
              weakMap.put(tbNoLeak2, null);
              weakMap.put(tbLeak1, null);
              weakMap.put(tbLeak2, null);

              System.out.println("weak map before gc: " + weakMap);

              // add some buttons to toggle group:

              // if we use the following line, everything is fine / no leak
              // tg.getToggles().addAll(tbLeak1, tbLeak2);

              // if we use the following lines, we get leaks:
              tbLeak1.setToggleGroup(tg);
              tbLeak2.setToggleGroup(tg);

              // get rid of all other strong references
              tbNoLeak1 = null;
              tbNoLeak2 = null;
              tbLeak1 = null;
              tbLeak2 = null;

              // clear toggles from group
              tg.getToggles().clear();

              // trigger garbage collection
              // expected result: there should be no more strong references to the buttons at this point, they should be gc'ed
              // actual result: the buttons that were added to the toggle group remain in memory
              System.gc();

              System.out.println("weak map after gc: " + weakMap);
      ---------- END SOURCE ----------

      CUSTOMER SUBMITTED WORKAROUND :
      In my tests, no leaks occurred if I always added ToggleButtons to a ToggleGroup the same way (either ONLY with ToggleButton.setToggleGroup, or ONLY with adding/removing buttons from ToggleGroup.getToggles().

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                arapte Ambarish Rapte
                Reporter:
                webbuggrp Webbug Group
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: