On Mon, 10 Mar 2025 10:41:25 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:

>> modules/javafx.base/src/main/java/com/sun/javafx/binding/ListenerList.java 
>> line 174:
>> 
>>> 172:                  */
>>> 173: 
>>> 174:                 throw new StackOverflowError("non-converging value 
>>> detected in value modifying listeners on " + observableValue + "; original 
>>> value was: " + oldValue);
>> 
>> We shouldn't throw errors, even if that's what the old implementation would 
>> implicitly do (not that it matters, no sane application would ever try to 
>> catch a `StackOverflowError` as part of its normal control flow). I think a 
>> `RuntimeException` is in order.
>> 
>> Also maybe make the message a bit clearer:
>> `<observableValue> was changed during the invocation of multiple listeners, 
>> but the new values diverge [original value: <oldValue>]`
>
> Maybe it should only be a warning log. Any exception a listener may throw is 
> currently only logged so other listeners may still continue to work 
> correctly.  Throwing an `Error` was a nice way around that, but does 
> potentially leave some listeners uninformed of a change or invalidation (same 
> as `ExpressionHelper` in that regard).
> 
> If we go for a runtime exception, I'd need to allow the exception to pass 
> through, but the same thing still applies that breaking it off with an 
> exception may leave some listeners uninformed.  So I think we need to decide, 
> how "bad" is this problem, and does it warrant terminating the application.

Is it possible to ignore the listener that incompatibly changed the value from 
Y back to X?

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r1987297519

Reply via email to