This PR captures the `currentValue` of the fired value change event in 
`ExpressionHelper$Generic::fireValueChangedEvent`,  before entering the 
notification loop, in order to pass it down to all the change listeners. 

This prevents a potential issue that could happen if all the change listeners 
are removed before the notification loop ends: the `currentValue` is nullified, 
and the pending listeners would unexpectedly receive null, instead of the value 
that was correctly sent to the previous listeners.

Some unit tests have been included, all of them but one pass before and after 
the proposed fix, and the last one fails before the fix and passes after it. 

The scenario that describes this failing test is: 
- A property has attached a few InvalidationListeners and a few ChangeListeners 
(so the Generic path is followed, and not the SingleChange). 
- At some point the property changes to a non-null `newValue`, and all the 
listeners get notified, one by one, in the same order as they were registered. 
- If, for any reason, and _while_ the different change listeners are being 
notified, all the change listeners are removed,  the pending listeners will, 
unexpectedly,  receive `null`, though the listeners that were notified before 
that happens received the expected `newValue`.

For simplicity, the test simulates that exact moment by removing all the change 
listeners when `listenerA` receives `newValue` from the `changed` notification. 
Then the `currentValue` gets nullified, and then the pending listeners in the 
notification list `curChangeList` (`listenerB` and `listenerC`, as they were 
registered after `listenerA`) get `null`. There could be other triggers, as 
long as they happen on the same thread, as part of the same call stack that 
started `fireValueChangedEvent`.

With the fix, by capturing the `currentValue` into `newValue` before entering 
the loop, then `B` and `C` will still receive `newValue`, regardless any 
possible change done to `currentValue` in the process.

---------
- [X] I confirm that I make this contribution in accordance with the [OpenJDK 
Interim AI Policy](https://openjdk.org/legal/ai).

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

Commit messages:
 - Capture currentValue before entering the notification loop, including 
unitary tests

Changes: https://git.openjdk.org/jfx/pull/2200/files
  Webrev: https://webrevs.openjdk.org/?repo=jfx&pr=2200&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8387626
  Stats: 149 lines in 2 files changed: 146 ins; 0 del; 3 mod
  Patch: https://git.openjdk.org/jfx/pull/2200.diff
  Fetch: git fetch https://git.openjdk.org/jfx.git pull/2200/head:pull/2200

PR: https://git.openjdk.org/jfx/pull/2200

Reply via email to