On Thu, 19 Dec 2019 13:23:25 GMT, Jeanette Winzenburg <faste...@openjdk.org> wrote:
>> Hi everyone, >> >> ticket: https://bugs.openjdk.java.net/browse/JDK-8236259 >> >> The fix itself is quite straight forward. >> It basically just removed the listener which causes the leak. >> >> The unit-test for the fix is a bit more complicated. >> >> I added a library JMemoryBuddy https://github.com/Sandec/JMemoryBuddy >> (written by myself), which simplifies testing for memory leaks. >> I think there are many places in the JavaFX-codebase that could highly >> benefit from this library. >> It could also simplify many of the already existing unit tests. >> It makes testing for memory-leaks readably and reliable. >> It would also be possible to just copy the code of the library into the >> JavaFX-codebase. >> It only contains a single class. >> >> I also had to make a method public, to write the test. I'm open to ideas, >> how I could solve it differently. > > modules/javafx.controls/src/main/java/javafx/scene/control/skin/ProgressIndicatorSkin.java > line 521: > >> 520: registerChangeListener(text.fontProperty(), >> (Consumer<ObservableValue<?>>) fontListener); >> 521: >> 522: // The circular background for the progress pie piece > > wondering why you add a (strong) field reference here? > > Thought that registerChangeListener(observableValue, consumer) takes care of > handling storage/cleanup - provided its counter-method > unregisterChangeListener(observableValue) is called, which is missing, so > seems to be the only part that has to be added. Or not? Yes, that doesn't make a lot of sense. It's probably an artifact because I expected a different API do unregister the listener. I'm gonna change it back! ------------- PR: https://git.openjdk.java.net/jfx/pull/71
