[Rev 03] RFR: 8196586: Remove use of deprecated finalize methods from javafx property objects

Kevin Rushforth kcr at openjdk.java.net
Thu Feb 20 20:43:22 UTC 2020


> This patch removes the `finalize` methods from the `Property` classes in the `javafx.base` module.
> 
> The `{Boolean,Double,Float,Int,Long}Property` classes each have a pair of methods -- `asObject` and `xxxxxProperty` -- that create a `Property<Xxxxx>` object from a primitive `XxxxxProperty` object and vice versa. Each of the methods bidirectionally binds the original property object to the created property object. All of the bidirectional bindings in question use `WeakReference`s to refer to the pair of objects being bound to each other, so that they won't cause a memory leak.
> 
> The `finalize` methods were added in an attempt to cleanup the bidirectional bindings when the created object goes away. I say attempt, because it is entirely ineffective in doing so. The logic that removes the binding creates a new one from the same pair of objects, but fails to match the original binding because the referent to the created property object has been garbage collected before the `finalize` method is called; the `WeakReference` in the binding is cleared and no longer matches. Fortunately, the only impact of this ineffective logic is that it can delay the removal of the binding (which is a small object that just contains the two weak references) from the original property object until it (the original property) is either garbage collected or modified (the binding logic already looks for a referent that has been GCed and removes the binding).
> 
> Given that the `finalize` methods don't do anything useful today, and that there is no memory leak in the first place, the proposed fix is to just remove the `finalize` methods entirely with no replacement needed. There will be no changes in behavior as a result of this.
> 
> Existing tests of the methods in question are sufficient to ensure no functional regressions, although there is no existing test for leaks, so I created new tests to verify that there is no leak. Since there is no existing leak, those tests will pass both with and without this fix.

The pull request has been updated with 1 additional commit.

-------------

Added commits:
 - 225e6011: Remove now-unused BidirectionalBinding::unbindNumber method per review

Changes:
  - all: https://git.openjdk.java.net/jfx/pull/113/files
  - new: https://git.openjdk.java.net/jfx/pull/113/files/c8d04111..225e6011

Webrevs:
 - full: https://webrevs.openjdk.java.net/jfx/113/webrev.03
 - incr: https://webrevs.openjdk.java.net/jfx/113/webrev.02-03

  Stats: 11 lines in 1 file changed: 0 ins; 11 del; 0 mod
  Patch: https://git.openjdk.java.net/jfx/pull/113.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/113/head:pull/113

PR: https://git.openjdk.java.net/jfx/pull/113


More information about the openjfx-dev mailing list