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

[Nir Lisker]

On Thu, 20 Feb 2020 20:43:22 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> 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.

Marked as reviewed by nlisker (Committer).

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

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