On Thu, 18 Dec 2025 02:19:03 GMT, John Hendrikx <[email protected]> wrote:
>> Fixes a problem with leaked references in XYChart. A few things conspire >> here to make this into a memory leak: >> >> - The weak bindings used by StringBinding leave behind listener "stubs" when >> GC'd; that's just how they work, it is a "strong" listener that wraps a weak >> referenced listener. The strong part remains behind, and is cleaned up when >> a new listener is added/removed by ExpressionHelper (and if that never >> happens, those stubs remain there indefinitely). >> - The fluent bindings (map/flatMap/orElse) use normal listeners, but only >> when they are observed themselves (lazy listeners) >> - The "stub" that is left behind counts as being observed, so the fluent >> bindings don't unsubscribe themselves >> >> The leak has nothing to do with the node or the accessible property, but >> purely by the StringBinding leaving behind stubs on the flat mapped >> properties. >> >> The leak is actually because the Series to which the Data object belongs is >> referencing the Data object. The flatMaps track the series object so they >> added a listener to the series object, and they think they are observed >> indefinitely because of the listener stub. >> >> The easiest solution here is to ensure the Series object is not tracked when >> not needed; this can be achieved by setting the series to `null` in the >> ListChangeListener for the Data list. > > John Hendrikx has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. The pull request contains one new > commit since the last revision: > > Fix memory leak > Excellent work, the memory leak is fixed, thank you so much @hjohn ! > > <img alt="Screenshot 2025-12-18 at 08 36 17" width="706" height="421" > src="https://private-user-images.githubusercontent.com/107069028/528205055-f555a51d-c446-4a55-bbbf-469afddb9f68.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NjYwOTYyNjIsIm5iZiI6MTc2NjA5NTk2MiwicGF0aCI6Ii8xMDcwNjkwMjgvNTI4MjA1MDU1LWY1NTVhNTFkLWM0NDYtNGE1NS1iYmJmLTQ2OWFmZGRiOWY2OC5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUxMjE4JTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MTIxOFQyMjEyNDJaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT03MzcxNTM2MmYwYzg3MDAyZTE5ZmVjMTNmODI4ZmI1YTBiNmUwYWY2YjgxNTdlMWVmYWY5MzZlNTA2ZGJhZjZlJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.2d_UxIX29_PuyyDeziBKZiBvQxklLtPfQKyZHIlWUFc";> > Questions: > > 1. the seriesProperty holds the same object as the series field, can we > remove the field? You added it at some point :) It can be removed, but you'll need use the `get()` method on the property in a lot of places then. I don't see any harm either way, so let me know what you want. > 2. could we apply the same treatment to PieChart (perhaps in a followup, > unless it's an easy fix)? I didn't look very far, I looked at the original change and didn't see any use of `flatMap` there. Is there a problem with it? > 3. perhaps, as a followup, we ought to add a parameterized test to make sure > all other charts don't exhibit the same issue? Yeah perhaps, or we could take a look at least if they were using a fluent binding + weak binding combination anywhere. I only looked at the original change, and I saw XYChart and PieChar were affected, but only XYChart was using new bindings. ------------- PR Comment: https://git.openjdk.org/jfx/pull/2013#issuecomment-3672479279
![https://private-user-images.githubusercontent.com/107069028/528205055-f555a51d-c446-4a55-bbbf-469afddb9f68.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NjYwOTYyNjIsIm5iZiI6MTc2NjA5NTk2MiwicGF0aCI6Ii8xMDcwNjkwMjgvNTI4MjA1MDU1LWY1NTVhNTFkLWM0NDYtNGE1NS1iYmJmLTQ2OWFmZGRiOWY2OC5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUxMjE4JTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MTIxOFQyMjEyNDJaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT03MzcxNTM2MmYwYzg3MDAyZTE5ZmVjMTNmODI4ZmI1YTBiNmUwYWY2YjgxNTdlMWVmYWY5MzZlNTA2ZGJhZjZlJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.2d_UxIX29_PuyyDeziBKZiBvQxklLtPfQKyZHIlWUFc"](https://private-user-images.githubusercontent.com/107069028/528205055-f555a51d-c446-4a55-bbbf-469afddb9f68.png?jwt=eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NjYwOTYyNjIsIm5iZiI6MTc2NjA5NTk2MiwicGF0aCI6Ii8xMDcwNjkwMjgvNTI4MjA1MDU1LWY1NTVhNTFkLWM0NDYtNGE1NS1iYmJmLTQ2OWFmZGRiOWY2OC5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUxMjE4JTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MTIxOFQyMjEyNDJaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT03MzcxNTM2MmYwYzg3MDAyZTE5ZmVjMTNmODI4ZmI1YTBiNmUwYWY2YjgxNTdlMWVmYWY5MzZlNTA2ZGJhZjZlJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.2d_UxIX29_PuyyDeziBKZiBvQxklLtPfQKyZHIlWUFc"){kind=link}