On Fri, 17 Apr 2026 10:12:22 GMT, Marius Hanl <[email protected]> wrote:
>> Hi,
>>
>> From what I can see this type of behavior is inherent for an implementation
>> in which when event is "fired" it is delivered right in place to listeners
>> like in below pseudo code:
>>
>> fireXXX()...
>> {
>> for each listener invoke onXXX(....)
>> }
>>
>> It is common for all awt and swing listeners and may, in plenty of
>> conditions when nested firing occurs, lead to change in ordering of events.
>> Yet, as far as I investigated it, and I did put a lot of work into it more
>> than decade ago, it is a nature of in-place events firing.
>>
>> As I do see it, the very crucial assumption behind in-place firing is:
>> _"when fireXXX returns, all listeners are notified"_. This is a strong
>> indication of state and data consistency and only in-place firing may do it.
>> Like in this pseudo code:
>>
>> my_function()
>> {
>> fireXXX()
>> _here I can assume that all listeners reacted to XXX_
>> fireYYY()
>> _here I can assume that all listeners reacted to XXX and YYY_
>> }
>>
>> I can assume it regardless if `my_function()` was invoked in response to
>> event or not.
>>
>> This assumption is in a strict and conceptual conflict with _"when fireXXX
>> is invoked the events reach all listeners in the same order as in which
>> events were fired even if events are fired from inside of listener event
>> handler"_.
>>
>> Because now if my example function is invoked inside a listener those
>> assumptions may be false.
>>
>> Any attempt to achieve the goal of delivering events in their order of
>> appearance will break the first assumption and it will lead to corruption of
>> existing programs. In many, many funny ways.
>>
>> Also any reliance on _"if listener A was registered before listener B, then
>> A will get called before B"_ is a horribly bad idea. Please, do not even try
>> to suggest it. Simply do not. There is no way to ensure that ordering in any
>> system of some major complexity.
>>
>> Any removing of "not necessary changes" in notification process is begging
>> for troubles. It did change, and it did changed again. Preventing that first
>> change from reaching the listeners is horrible idea, as listener is by
>> definition getting every change, not just "some".
>>
>> In my practice, if I am worried about nested events and ordering problem, I
>> do consciously decide to "decouple" them. Instead of firing an event in
>> place I do, consciously, wrap them and delegate to the event pump (in case
>> of awt - with invokeLater). Then I do know that they will reach the
>> destination in order of firing, but I also do now that at...
>
>> Also any reliance on _"if listener A was registered before listener B, then
>> A will get called before B"_ is a horribly bad idea. Please, do not even try
>> to suggest it. Simply do not. There is no way to ensure that ordering in any
>> system of some major complexity.
>
> This is the case right now and will be after this PR.
>
>> My recommendation is: if You really plan to change in-place firing to some
>> more sophisticated algorithm **do not do it**. Not in a stable production
>> code. Recommend, inform, educate about possible side effect, alternatively
>> add like "orderedFireXXX" methods or something, but do not change such an
>> essential algorithm. As you can see above delegation is a simple method
>> which allows programmers using the library to decide what is important from
>> them and what is not. If they see problems with ordering delegation solves
>> it. If however you will build it in, they will have no way to ensure that if
>> they have problems with:
>
> I still don't get your point. This is about nested changes and that the old
> value is misleading or simply incorrect. You talked about state consistency,
> but currently, this can happen:
>
>
> myProperty.addListener((_, oldValue, newValue) -> {
> // Here, oldValue MIGHT not be the last set value, but there value before
> that.
> // Instead of A -> B -> C
> // oldValue can be A and newValue be C. Which is very bad when you want
> to remove/unregister the old value.
> }
>
> This PR corrects that to BE consistent, so the `oldValue` will be B in this
> example. This is consistency for me.
> @Maran23
>
> My point was, that events broadcasting strategy is a critical point of the
> system which is build around event oriented programming and model-observer
> design. A system which exists for about thirty years now and is deployed in
> millions of places. Touching it should be done with absolute care and
> attention.
This was done with care and attention, and fixes real problems with systems
relying on correct old values (like listener removal). The current state of
affairs is that the old value is unreliable in scenario's that can arise when
notifications trigger further notifications which may unwittingly lead to a
re-entrant call.
> I wished to make an impression, that what one person sees as an obvious
> benefit may be a complete drama for others. And this is obviously a mission
> critical point. It should not be touched to make it better. An alternative
> method shall be provided, the old might become deprecated, but such a
> critical subsystem shall not change its behavior.
If you have a problem with these changes, then feel free to test and refute
what is done here. Your statement applies to any change, no matter how
insignificant, however, what matters here is whether it aligns with what
guarantees are made in the documentation. Anything beyond that is relying on
undefined behavior.
> My second observation was, that if one really needs proper ordering of
> events, one may resort to `invokeLater` or override some methods.
We're not changing the order of events. Events were always delivered in order
of registration. Even though this is unspecified, it is something we do not
wish to touch as that was deemed to be far too risky to change. It is sort of
implied by the fact that we're talking about listener lists, and the fact one
can register (and must remove) multiple instances of the same listeners.
> Consistency depends on point of view.
Sure. In this case it depends on what guarantees are given in the
documentation's point of view.
> For an example, can you give a warranty that in any nested condition when
> event is fired, and the firing method returns, regardless if it was a nested
> firing or not, that specific event was delivered to listeners? If not, it
> will be a critical change, as a delivery time consistency becomes broken.
No such guarantees were given before, and none are part of the specification.
Relying on any specific change notifications order or timing is undefined
behavior.
This PR however promises that **if** an event is delivered, it will have
correct old/new values, that match up with the previous event received, and
will match up with the next event received.
> Did you consider how will it behave when one changed multiple properties on
> multiple objects, of which some broadcast will be nested and some will be not?
No, there are no guarantees anywhere in JavaFX that work across multiple
properties. The way to deal with this is how for example layout is done. One
sets multiple values, and invalidation listeners register that there was a
change but don't act on it immediately. Instead, they trigger a single action
that reads the changes and updates the system. This is how layout ensures that
not every property change triggers a layout each time; instead they're
"buffered" and only acted upon on the next layout pass.
> Then, some applications might rely on counting the changes, logging them or
> something like that. Will they still see everything?
Change notifications are not an event delivery system. No guarantees are or
were made that every change to a property will result in a change notification.
For example, change listeners are free to elide non-changes (from A to A). They
also don't send a notification when the instances are equal, but different
instances. Subsequently, if a listener changes a value back to the previous
value, there is and was never a guarantee that subsequent listeners would be
notified as there is no longer a change (oldValue equals newValue).
> Did you consider the consistency of the event with a value retrieved from the
> getters?
If you take the time to read the comments on this PR, then you will find that
this was indeed considered.
> Sure, the in-place event firing has a lot of down-sides. The getters may
> produce value inconsistent with what event sends, the events order may flip,
> a deadlock possibilities are horrible is one will use `synchronized` inside a
> listener. A complete horror.
Sorry what? FX is a single threaded system, and so are its properties. None of
their internal structures are synchronized or suitable for multi-threaded use.
It would indeed be a "complete horror" to use these in a multi-threaded
system...
> I do appreciate, that you did notice the problem, because very few people can
> see it. This is great. But I would rather see a safer approach with, for an
> example, pluggable listener manager or something like that, when I could put
> it in if I want it, or stay with an old style if I do not. This would be
> nice, because I could then plug in my own solution to my own problems. Yet,
> for those I can override some methods, right?
Feel free to post to the mailinglist so this can be discussed in a broader
forum, and so we can hear the opinions of other developers on this suggestion.
However, I would suggest if you have specific needs for event delivery to use a
solution suited for that purpose instead of trying to use JavaFX properties for
this.
> Where I see the problem, it is that the concept of having both getter and an
> event sending the new value is inherently flawed. Either getter and "changed"
> without to what, or no getter and only "streaming of changes" - only those
> can ensure the consistency.
It is not flawed at all. The change listener provides both the old and new
value, which is highly useful when you need to track a property's state to
remove a listener on the old value, and register the listener on the new value.
If the old value is unreliable (in the current system) then this breaks,
resulting in duplicate listeners registered or listeners never getting removed.
For your use case, you can use invalidation listeners and do a `get` call in
your callback. Alternatively, if you are only interested in the current value,
then consider just using `subscribe(Consumer)`.
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1081#issuecomment-4293584695
