On Thu, 9 Jul 2026 19:30:07 GMT, Andy Goryachev <[email protected]> wrote:
>> John Hendrikx has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Fix flakey check on (asynchronous) stderr output
>
> modules/javafx.base/src/main/java/com/sun/javafx/binding/ListenerList.java
> line 178:
>
>> 176: */
>> 177:
>> 178: Logging.getLogger().warning(
>
> rant: it's amazing that we reinvent the wheel all the time instead of making
> a good logging facade a part of java.
>
> in this case, can the log output be disabled? I suspect even when disabled,
> the `formatted()` will be called.
>
> related: is `MessageFormat` a better choice for logging?
These days (well since Java 9) there is a new facade that seems to be preferred
over JUL logging as it is easier to integrate: `System.getLogger(loggerName)`
Also, I don't think we really reinvented the wheel here; we just made a facade
for unit testing specifically (as it is hard to hook into JUL logging I think
for this, given it is asynchronous). We could also directly get the relevant
JUL logger here and not go through the facade and the result would be the same
(just can't be unit tested as easily then).
I can surround this with a "is level warning" check, if you think it would be a
performance issue (this log shouldn't show up at all, and if it does, there may
be something seriously broken with your listeners that would have resulted I
think in a `StackOverflowError` in the system that this PR replaces).
As I think this message should never appear in 99.99% of FX applications, I'm
not worried about the formatting overhead; and if it does appear, the correct
action would be to fix it, not ignore it.
> modules/javafx.base/src/main/java/com/sun/javafx/binding/ListenerManager.java
> line 165:
>
>> 163: setData(instance, list.getInvalidationListener(0));
>> 164: }
>> 165: else if (changeListenersSize == 1) {
>
> ... it must be your style, but it throws my pattern recognition. if no one
> else objects, you can ignore my pain.
It is indeed the style I use, so you'll see this a lot indeed :) Let's focus
on the implementation first I'd say, you brought up some good points already
that I will dive deeper into before answering.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r3558627910
PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r3558641128
