On Fri, 3 Jul 2026 12:36:44 GMT, Jose Pereda <[email protected]> wrote:
> 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). This pull request has now been integrated. Changeset: d460b6a1 Author: Jose Pereda <[email protected]> URL: https://git.openjdk.org/jfx/commit/d460b6a1ba80f192d833ed14614adf120db28cee Stats: 149 lines in 2 files changed: 146 ins; 0 del; 3 mod 8387655: java.lang.NullPointerException: Cannot invoke "java.lang.Boolean.booleanValue()" because "newValue" is null Reviewed-by: jhendrikx, angorya ------------- PR: https://git.openjdk.org/jfx/pull/2200
