> 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).
Jose Pereda has updated the pull request incrementally with one additional commit since the last revision: assign back to helper ------------- Changes: - all: https://git.openjdk.org/jfx/pull/2200/files - new: https://git.openjdk.org/jfx/pull/2200/files/3b463c6d..f72724fc Webrevs: - full: https://webrevs.openjdk.org/?repo=jfx&pr=2200&range=02 - incr: https://webrevs.openjdk.org/?repo=jfx&pr=2200&range=01-02 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 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
