Re: RFR: 8235480: Regression: [RTL] Arrow keys navigation doesn't respect TableView orientation
On Sat, 15 Feb 2020 14:54:50 GMT, Kevin Rushforth wrote:
>> thanks, Kevin - that's what I tried, but it refused to accept the second ..
>> so sticking with single comments until I feel like digging into this
>> weirdness.
>
> Regarding the fix, it seems that you need to eliminate the `rtl` variable you
> added earlier, and move the test into the listener, like this:
>
> new KeyMapping(LEFT, e -> isRtl() ? selectRightCell() : selectLeftCell()),
@kevinrushforth, this new KeyMapping suggestion does not compile.
We need something like -
`new KeyMapping(LEFT, e -> { if(isRTL()) selectRightCell(); else
selectLeftCell();}),`
-
PR: https://git.openjdk.java.net/jfx/pull/114
Re: [jfx14] [Rev 02] RFR: 8228867: Fix mistakes in FX API docs
On Tue, 18 Feb 2020 21:59:07 GMT, Nir Lisker wrote: >> Docs fixes for OpenJFX 14. We can wait with integrating this in case there >> are last minute fixes, but reviews can start. > > The pull request has been updated with 2 additional commits. Marked as reviewed by jvos (Reviewer). - PR: https://git.openjdk.java.net/jfx/pull/110
Re: [Rev 02] RFR: 8235772: Remove use of deprecated finalize method from PiscesRenderer and AbstractSurface
On Mon, 10 Feb 2020 13:33:41 GMT, Ambarish Rapte wrote: >> The finalize() method is deprecated in JDK9. See [Java 9 deprecated >> features](https://www.oracle.com/technetwork/java/javase/9-deprecated-features-3745636.html). >> And so the >> [PiscesRenderer.finalize()](https://github.com/openjdk/jfx/blob/71ca899fbfc74d825c29b20a778d6d9eb243f90f/modules/javafx.graphics/src/main/java/com/sun/pisces/PiscesRenderer.java#L439) >> and >> [AbstractSurface.finalize()](https://github.com/openjdk/jfx/blob/71ca899fbfc74d825c29b20a778d6d9eb243f90f/modules/javafx.graphics/src/main/java/com/sun/pisces/AbstractSurface.java#L100) >> method should be removed. >> >> The change is, >> 1. Remove finalize method from PiscesRenderer and AbstractSurface classes. >> 2. Use >> [Disposer](https://github.com/openjdk/jfx/blob/71ca899fbfc74d825c29b20a778d6d9eb243f90f/modules/javafx.graphics/src/main/java/com/sun/prism/impl/Disposer.java#L48) >> class for cleanup instead of finalize(). For SW pipeline the cleanup is >> initiated in >> [here](https://github.com/openjdk/jfx/blob/71ca899fbfc74d825c29b20a778d6d9eb243f90f/modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/UploadingPainter.java#L195) >> in UploadingPainer.java. >> 3. Added new JNI methods to JPiscesRenderer.c and JAbstractSurface.c and >> removed the ones which were used previously with finalize() method. >> >> Verification: >> Verified that the newly added dispose() methods get invoked as required and >> no OOM occurs. >> 1. Create a sample program which adds and removes non 3D shapes and/or >> controls to stage in a loop. >> 2. Run the program using -Dprism.order=sw, -Dprism.verbose=true. (optional >> -Xmx50m) >> Expected Behavior: >> a. Maximum memory consumption after this change should reduce or remain same >> as that of before this change. >> b. No OOM should occur with this change or without this change. > > The pull request has been updated with 1 additional commit. Marked as reviewed by jvos (Reviewer). - PR: https://git.openjdk.java.net/jfx/pull/66
Re: [Integrated] RFR: 8228867: Fix mistakes in FX API docs
Changeset: 35a01caf Author:Nir Lisker Date: 2020-02-19 14:35:56 + URL: https://git.openjdk.java.net/jfx/commit/35a01caf 8228867: Fix mistakes in FX API docs Reviewed-by: jvos, kcr ! modules/javafx.base/src/main/java/javafx/collections/FXCollections.java ! modules/javafx.graphics/src/main/java/javafx/animation/Animation.java ! modules/javafx.graphics/src/main/java/javafx/application/Platform.java ! modules/javafx.graphics/src/main/java/javafx/scene/Node.java ! modules/javafx.graphics/src/main/native-glass/win/OleUtils.h
RFR: 8239454: LLIntData : invalid opcode returned for 16 and 32 bit wide instructions
With new bytecode format introduced in Webkit 608.1, the unlinked and linked bytecodes were replaced by narrow (1-byte operand) and wide (4-byte operand) bytecodes. These were extended to narrow, wide16 and wide32 bytecodes in WebKit 609.1. In narrow instructions, each argument of the opcode has a fixed size of 1-byte. The same applies for wide 16 and wide32 with 2-byte and 4-byte, respectively. In the Low Level Intepreter (LLInt), each opcode has a corresponding ID assigned for narrow, wide16 and wide32 implementation, and the variable `numOpcodeIDs` is used to denote the total number of opcodes. The narrow opcode IDs are mapped from 0 to (`numOpcodeIDs` - 1). The next `numOpcodeIDs` opcode IDs are mapped to wide16 opcodes, and similarly, the next `numOpcodeIDs` to wide32 opcodes. The same can be found in [LowLevelInterpreter.cpp](https://github.com/openjdk/jfx/blob/master/modules/javafx.web/src/main/native/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp#L265) and also in the autogenerated file `Bytecodes.h`. Bug: When `getOpcodeWide(id)` is called in `LLIntData.h`, the value (`id` - `numOpcodesIDs`) is returned. Fix: It's modified to (`id` + `numOpcodesIDs`) in `getOpcodeWide16()` and (`id` + `numOpcodesIDs`*2) in `getOpcodeWide32()`. - Commits: - eed20431: 8239454: LLIntData : invalid opcode returned for 16 and 32 bit wide instructions Changes: https://git.openjdk.java.net/jfx/pull/115/files Webrev: https://webrevs.openjdk.java.net/jfx/115/webrev.00 Issue: https://bugs.openjdk.java.net/browse/JDK-8239454 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jfx/pull/115.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/115/head:pull/115 PR: https://git.openjdk.java.net/jfx/pull/115
Question ad running the "systemTests" module tests ...
"build.gradle" contains the project ":systemTests" which defines tests that each run on a separate JVM and use their own Assert methods defined in respective Util.java. They signal success according to the values defined in "Constants.java". The module test "testapp7" that I wrote mimickries the existing "testapp6" module in its layout. "testapp7" has been successfully added to the list of test modules and got compiled successfully, such that it is possible to run the "testapp7" module from the command line and it behaves as expected given the data and assertions carried out. However, when running "./gradlew systemTests test" that module test "testapp7" does not get executed (and perhaps none of the other module tests) it seems, although "FULL_TEST" was set to "true" in "build.gradle" such that the module tests should get "enabled" therefore. Is there something else to take into account or is there some other task that I need to use? Any help, pointer highly appreciated! ---rony
Re: RFR: 8176499: Dependence on java.util.Timer freezes screen when OS time resets backwards
On Wed, 19 Feb 2020 19:39:30 GMT, Kevin Rushforth wrote: >> I don't see any stray commits, so it looks like your branch is based off of >> master correctly. >> >> One thing I would ask you to change is that the title of this PR should >> exactly match the title of the JBS bug. So can you change it to: >> >> 8176499: Dependence on java.util.Timer freezes screen when OS time resets >> backwards > > Seems a simple enough fix. Probably @johanvos can review it. > I don't see any stray commits, so it looks like your branch is based off of > master correctly. > > One thing I would ask you to change is that the title of this PR should > exactly match the title of the JBS bug. So can you change it to: > > ``` > 8176499: Dependence on java.util.Timer freezes screen when OS time resets > backwards > ``` apologies, all done - PR: https://git.openjdk.java.net/jfx/pull/117
Re: RFR: 8176499: Remove MonocleTimer dependency on system time
On Wed, 19 Feb 2020 19:17:03 GMT, Dell Green
wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8176499
>>
>> This pull request fixes a long standing issue in the MonocleTimer class
>> whereby it has a dependency on the java.uti.Timer class which is dependent
>> on system time and can cause UI freezes for seconds/minutes/hours/days/years
>> dependent upon how far back in time the system clock is set by either a user
>> manually or NTP. This looks like it is because the Timer class will wait for
>> (executionTime - currentTime) before proceeding if a task hasn't fired yet.
>>
>> https://hg.openjdk.java.net/jdk10/master/file/be620a591379/src/java.base/share/classes/java/util/Timer.java#l553
>>
>> For a long running embedded device with a UI like IOT devices this is pretty
>> disastrous.
>> We recently re-discovered this issue whilst testing such a device before
>> going into production.
>>
>> The MonocleTimer class is used by MonocleApplication and QuantumToolkit
>> class to create its pulseTimer for emebdded systems and sets it up to fire
>> periodically inline with the requested pulse frequency which by default is
>> 60Hz resulting in a pulse interval of 16ms.
>>
>> It is well documented that for implementations that wish to measure elapsed
>> time ScheduledThreadPoolExecutor should be used as a replacement for
>> java.util.Timer class.
>>
>> Java Concurrency In Practice:
>> https://pdfs.semanticscholar.org/3650/4bc31d3b2c5c00e5bfee28ffc5d403cc8edd.pdf
>> (page 77)
>>
>> "The Timer facility manages the execution of deferred ("run this task in 100
>> ms") and periodic ("run this task every 10ms") tasks. However, Timer has
>> some drawbacks, and ScheduledThreadPoolExecutor should be thought of as its
>> replacement."
>>
>> With the original implementation, if I set the date.time back 8 years for
>> example the UI freezes up indefinitely (I cant wait 8 years). Repeating the
>> same test with the proposed implementation has no affect on the UI and it
>> runs as normal.
>>
>> The proposed solution has been tested on an Arm iMX6 board.
>>
>> Whist testing in isolation the MonocleTimer class with no work to do on each
>> pulse, it looks like the change from Timer class to
>> ScheduledThreadPoolExecutor also has brought with it a greater accuracy of
>> when the pulses are fired.
>>
>> The following results were observed when running MonocleTimer at 60Hz for 1
>> minute. It appears that we get a higher frequency of pulses hitting the 16ms
>> mark with the replacement solution
>>
>>
>> x86-64 linux desktop:
>>
>> Timer class
>> NumSamples: 3599
>> Mean: 16.230063906640734
>> StdDev: 0.45021901536403147
>> Median: 16
>> Mode: 16, freq: 2714, perc: 75.40983606557377%
>>
>>
>> Scheduler class
>> NumSamples: 3599
>> Mean: 16.0
>> StdDev: 0.0
>> Median: 16
>> Mode: 16, freq: 3599, perc: 100.0%
>>
>>
>>
>> Arm linux iMX6:
>>
>> Timer class
>> NumSamples: 3599
>> Mean: 16.182272853570435
>> StdDev: 0.4224950723394174
>> Median: 16
>> Mode: 16, freq: 2837, perc: 78.82745207001946%
>>
>>
>> Scheduler class
>> NumSamples: 3599
>> Mean: 15.995554320644624
>> StdDev: 0.3666906549602725
>> Median: 16
>> Mode: 16, freq: 3468, perc: 96.3601000277855%
>
> @kevinrushforth apologies for previous, my local repos seem to be messed up
> when i changed remotes from old javafx github repo to new one, as that rogue
> commit didnt exist my side for some reason. looks like its fixed now
I don't see any stray commits, so it looks like your branch is based off of
master correctly.
One thing I would ask you to change is that the title of this PR should exactly
match the title of the JBS bug. So can you change it to:
8176499: Dependence on java.util.Timer freezes screen when OS time resets
backwards
-
PR: https://git.openjdk.java.net/jfx/pull/117
Re: RFR: 8176499: Remove MonocleTimer dependency on system time
On Wed, 19 Feb 2020 19:37:41 GMT, Kevin Rushforth wrote: >> @kevinrushforth apologies for previous, my local repos seem to be messed up >> when i changed remotes from old javafx github repo to new one, as that rogue >> commit didnt exist my side for some reason. looks like its fixed now > > I don't see any stray commits, so it looks like your branch is based off of > master correctly. > > One thing I would ask you to change is that the title of this PR should > exactly match the title of the JBS bug. So can you change it to: > > 8176499: Dependence on java.util.Timer freezes screen when OS time resets > backwards Seems a simple enough fix. Probably @johanvos can review it. - PR: https://git.openjdk.java.net/jfx/pull/117
Re: RFR: 8176499: Remove MonocleTimer dependency on system time
On Wed, 19 Feb 2020 19:14:08 GMT, Dell Green
wrote:
> https://bugs.openjdk.java.net/browse/JDK-8176499
>
> This pull request fixes a long standing issue in the MonocleTimer class
> whereby it has a dependency on the java.uti.Timer class which is dependent on
> system time and can cause UI freezes for seconds/minutes/hours/days/years
> dependent upon how far back in time the system clock is set by either a user
> manually or NTP. This looks like it is because the Timer class will wait for
> (executionTime - currentTime) before proceeding if a task hasn't fired yet.
>
> https://hg.openjdk.java.net/jdk10/master/file/be620a591379/src/java.base/share/classes/java/util/Timer.java#l553
>
> For a long running embedded device with a UI like IOT devices this is pretty
> disastrous.
> We recently re-discovered this issue whilst testing such a device before
> going into production.
>
> The MonocleTimer class is used by MonocleApplication and QuantumToolkit class
> to create its pulseTimer for emebdded systems and sets it up to fire
> periodically inline with the requested pulse frequency which by default is
> 60Hz resulting in a pulse interval of 16ms.
>
> It is well documented that for implementations that wish to measure elapsed
> time ScheduledThreadPoolExecutor should be used as a replacement for
> java.util.Timer class.
>
> Java Concurrency In Practice:
> https://pdfs.semanticscholar.org/3650/4bc31d3b2c5c00e5bfee28ffc5d403cc8edd.pdf
> (page 77)
>
> "The Timer facility manages the execution of deferred ("run this task in 100
> ms") and periodic ("run this task every 10ms") tasks. However, Timer has some
> drawbacks, and ScheduledThreadPoolExecutor should be thought of as its
> replacement."
>
> With the original implementation, if I set the date.time back 8 years for
> example the UI freezes up indefinitely (I cant wait 8 years). Repeating the
> same test with the proposed implementation has no affect on the UI and it
> runs as normal.
>
> The proposed solution has been tested on an Arm iMX6 board.
>
> Whist testing in isolation the MonocleTimer class with no work to do on each
> pulse, it looks like the change from Timer class to
> ScheduledThreadPoolExecutor also has brought with it a greater accuracy of
> when the pulses are fired.
>
> The following results were observed when running MonocleTimer at 60Hz for 1
> minute. It appears that we get a higher frequency of pulses hitting the 16ms
> mark with the replacement solution
>
>
> x86-64 linux desktop:
>
> Timer class
> NumSamples: 3599
> Mean: 16.230063906640734
> StdDev: 0.45021901536403147
> Median: 16
> Mode: 16, freq: 2714, perc: 75.40983606557377%
>
>
> Scheduler class
> NumSamples: 3599
> Mean: 16.0
> StdDev: 0.0
> Median: 16
> Mode: 16, freq: 3599, perc: 100.0%
>
>
>
> Arm linux iMX6:
>
> Timer class
> NumSamples: 3599
> Mean: 16.182272853570435
> StdDev: 0.4224950723394174
> Median: 16
> Mode: 16, freq: 2837, perc: 78.82745207001946%
>
>
> Scheduler class
> NumSamples: 3599
> Mean: 15.995554320644624
> StdDev: 0.3666906549602725
> Median: 16
> Mode: 16, freq: 3468, perc: 96.3601000277855%
@kevinrushforth apologies for previous, my local repos seem to be messed up
when i changed remotes from old javafx github repo to new one, as that rogue
commit didnt exist my side for some reason. looks like its fixed now
-
PR: https://git.openjdk.java.net/jfx/pull/117
RFR: 8176499: Remove MonocleTimer dependency on system time
https://bugs.openjdk.java.net/browse/JDK-8176499
This pull request fixes a long standing issue in the MonocleTimer class whereby
it has a dependency on the java.uti.Timer class which is dependent on system
time and can cause UI freezes for seconds/minutes/hours/days/years dependent
upon how far back in time the system clock is set by either a user manually or
NTP. This looks like it is because the Timer class will wait for (executionTime
- currentTime) before proceeding if a task hasn't fired yet.
https://hg.openjdk.java.net/jdk10/master/file/be620a591379/src/java.base/share/classes/java/util/Timer.java#l553
For a long running embedded device with a UI like IOT devices this is pretty
disastrous.
We recently re-discovered this issue whilst testing such a device before going
into production.
The MonocleTimer class is used by MonocleApplication and QuantumToolkit class
to create its pulseTimer for emebdded systems and sets it up to fire
periodically inline with the requested pulse frequency which by default is 60Hz
resulting in a pulse interval of 16ms.
It is well documented that for implementations that wish to measure elapsed
time ScheduledThreadPoolExecutor should be used as a replacement for
java.util.Timer class.
Java Concurrency In Practice:
https://pdfs.semanticscholar.org/3650/4bc31d3b2c5c00e5bfee28ffc5d403cc8edd.pdf
(page 77)
"The Timer facility manages the execution of deferred ("run this task in 100
ms") and periodic ("run this task every 10ms") tasks. However, Timer has some
drawbacks, and ScheduledThreadPoolExecutor should be thought of as its
replacement."
With the original implementation, if I set the date.time back 8 years for
example the UI freezes up indefinitely (I cant wait 8 years). Repeating the
same test with the proposed implementation has no affect on the UI and it runs
as normal.
The proposed solution has been tested on an Arm iMX6 board.
Whist testing in isolation the MonocleTimer class with no work to do on each
pulse, it looks like the change from Timer class to ScheduledThreadPoolExecutor
also has brought with it a greater accuracy of when the pulses are fired.
The following results were observed when running MonocleTimer at 60Hz for 1
minute. It appears that we get a higher frequency of pulses hitting the 16ms
mark with the replacement solution
x86-64 linux desktop:
Timer class
NumSamples: 3599
Mean: 16.230063906640734
StdDev: 0.45021901536403147
Median: 16
Mode: 16, freq: 2714, perc: 75.40983606557377%
Scheduler class
NumSamples: 3599
Mean: 16.0
StdDev: 0.0
Median: 16
Mode: 16, freq: 3599, perc: 100.0%
Arm linux iMX6:
Timer class
NumSamples: 3599
Mean: 16.182272853570435
StdDev: 0.4224950723394174
Median: 16
Mode: 16, freq: 2837, perc: 78.82745207001946%
Scheduler class
NumSamples: 3599
Mean: 15.995554320644624
StdDev: 0.3666906549602725
Median: 16
Mode: 16, freq: 3468, perc: 96.3601000277855%
-
Commits:
- 3c22d205: 8176499: Remove MonocleTimer dependency on system time
Changes: https://git.openjdk.java.net/jfx/pull/117/files
Webrev: https://webrevs.openjdk.java.net/jfx/117/webrev.00
Issue: https://bugs.openjdk.java.net/browse/JDK-8176499
Stats: 17 lines in 1 file changed: 3 ins; 8 del; 6 mod
Patch: https://git.openjdk.java.net/jfx/pull/117.diff
Fetch: git fetch https://git.openjdk.java.net/jfx pull/117/head:pull/117
PR: https://git.openjdk.java.net/jfx/pull/117
Re: RFR: 8239454: LLIntData : invalid opcode returned for 16 and 32 bit wide instructions
On Wed, 19 Feb 2020 12:09:50 GMT, Arun Joseph wrote: > With new bytecode format introduced in Webkit 608.1, the unlinked and linked > bytecodes were replaced by narrow (1-byte operand) and wide (4-byte operand) > bytecodes. These were extended to narrow, wide16 and wide32 bytecodes in > WebKit 609.1. In narrow instructions, each argument of the opcode has a fixed > size of 1-byte. The same applies for wide 16 and wide32 with 2-byte and > 4-byte, respectively. > > In the Low Level Intepreter (LLInt), each opcode has a corresponding ID > assigned for narrow, wide16 and wide32 implementation, and the variable > `numOpcodeIDs` is used to denote the total number of opcodes. The narrow > opcode IDs are mapped from 0 to (`numOpcodeIDs` - 1). The next `numOpcodeIDs` > opcode IDs are mapped to wide16 opcodes, and similarly, the next > `numOpcodeIDs` to wide32 opcodes. The same can be found in > [LowLevelInterpreter.cpp](https://github.com/openjdk/jfx/blob/master/modules/javafx.web/src/main/native/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp#L265) > and also in the autogenerated file `Bytecodes.h`. > > Bug: When `getOpcodeWide(id)` is called in `LLIntData.h`, the value (`id` - > `numOpcodesIDs`) is returned. > > Fix: It's modified to (`id` + `numOpcodesIDs`) in `getOpcodeWide16()` and > (`id` + `numOpcodesIDs`*2) in `getOpcodeWide32()`. Marked as reviewed by ghb (Reviewer). - PR: https://git.openjdk.java.net/jfx/pull/115
Re: RFR: 8239454: LLIntData : invalid opcode returned for 16 and 32 bit wide instructions
On Wed, 19 Feb 2020 12:09:50 GMT, Arun Joseph wrote: > With new bytecode format introduced in Webkit 608.1, the unlinked and linked > bytecodes were replaced by narrow (1-byte operand) and wide (4-byte operand) > bytecodes. These were extended to narrow, wide16 and wide32 bytecodes in > WebKit 609.1. In narrow instructions, each argument of the opcode has a fixed > size of 1-byte. The same applies for wide 16 and wide32 with 2-byte and > 4-byte, respectively. > > In the Low Level Intepreter (LLInt), each opcode has a corresponding ID > assigned for narrow, wide16 and wide32 implementation, and the variable > `numOpcodeIDs` is used to denote the total number of opcodes. The narrow > opcode IDs are mapped from 0 to (`numOpcodeIDs` - 1). The next `numOpcodeIDs` > opcode IDs are mapped to wide16 opcodes, and similarly, the next > `numOpcodeIDs` to wide32 opcodes. The same can be found in > [LowLevelInterpreter.cpp](https://github.com/openjdk/jfx/blob/master/modules/javafx.web/src/main/native/Source/JavaScriptCore/llint/LowLevelInterpreter.cpp#L265) > and also in the autogenerated file `Bytecodes.h`. > > Bug: When `getOpcodeWide(id)` is called in `LLIntData.h`, the value (`id` - > `numOpcodesIDs`) is returned. > > Fix: It's modified to (`id` + `numOpcodesIDs`) in `getOpcodeWide16()` and > (`id` + `numOpcodesIDs`*2) in `getOpcodeWide32()`. Fix looks good. Testing looks good. - Marked as reviewed by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/115
Re: RFR: 8239109: Update SQLite to version 3.31.1
On Thu, 20 Feb 2020 05:58:03 GMT, Guru Hb wrote: > Updating SQLite to version 3.31.1 (Currently used version is 3.30.1). Marked as reviewed by ajoseph (Author). - PR: https://git.openjdk.java.net/jfx/pull/119
RFR: 8239109: Update SQLite to version 3.31.1
Updating SQLite to version 3.31.1 (Currently used version is 3.30.1). - Commits: - d4e17340: 8239109: Update SQLite to version 3.31.1 Changes: https://git.openjdk.java.net/jfx/pull/119/files Webrev: https://webrevs.openjdk.java.net/jfx/119/webrev.00 Issue: https://bugs.openjdk.java.net/browse/JDK-8239109 Stats: 9862 lines in 4 files changed: 4400 ins; 774 del; 4688 mod Patch: https://git.openjdk.java.net/jfx/pull/119.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/119/head:pull/119 PR: https://git.openjdk.java.net/jfx/pull/119
Re: RFR: 8196586: Remove use of deprecated finalize methods from javafx property objects
On Tue, 18 Feb 2020 20:09:59 GMT, Kevin Rushforth wrote: >>> the referent to the created property object has been garbage collected >>> before the `finalize` method is called >> >> I'm confused. What is that referent exactly and why is it guaranteed to have >> been GC'd before finalization? > >> > the referent to the created property object has been garbage collected >> > before the `finalize` method is called >> >> I'm confused. What is that referent exactly and why is it guaranteed to have >> been GC'd before finalization? > > I meant the referent of the `WeakReference` to the constructed `Property` > object on which the `finalize` method is being called. > > The `asObject` and `xProperty` methods create a bidirectional binding, > which uses weak references to both the original `Property` object and the > newly constructed `Property` object. The `finalize` method is on the > newly-constructed `Property`. By the time that finalize method is called, the > JVM guarantees that no other reference to that `Property` object exists. > From the API docs of `Object::finalize`: > > _The general contract of finalize is that it is invoked if and when the Java™ > virtual machine has determined that there is no longer any means by which > this object can be accessed by any thread that has not yet died, except as a > result of an action taken by the finalization of some other object or class > which is ready to be finalized._ The logic seems correct. The weak reference to the newly created property in the bidirectional binding must be cleared before `finalize` is called. The unbinding attempt only interferes with GC. I still have to go over the tests. I note that with the removal of the finalize methods, the method `BidirectionalBinding#unbindNumber` is left unused and can be removed. Also, the changed property classes and `BidirectionalBinding` can use some cleanup that I can do after this is fixed, so I can remove that method at that time if you don't want to do it here. - PR: https://git.openjdk.java.net/jfx/pull/113
