On Fri, 10 Jul 2026 22:04:33 GMT, John Hendrikx <[email protected]> wrote:
>> Good one; this test is to make sure that dead weak listeners stubs don't
>> disappear at unexpected moments. Some code (see
>> `ObjectBinding#removeListener` for example) tracks whether the listener list
>> is empty through overrides of `addListener` and `removeListener`:
>>
>>
>> public void removeListener(ChangeListener<? super T> listener) {
>> observed = !LISTENER_MANAGER.removeListener(this,
>> (ChangeListener<Object>) listener);
>> }
>>
>> Or the old code:
>>
>> public void removeListener(ChangeListener<? super T> listener) {
>> helper = ExpressionHelper.removeListener(helper, listener);
>> observed = helper != null;
>> }
>>
>> To not break such tracking, we can only remove dead listeners when some
>> other list modification occurs; either an addition (in which case we know
>> there is at least one new listener, so we can remove any dead listeners), or
>> a removal (in which case if the list becomes empty, we return `true` or with
>> the old code, we check if `helper == null`.)
>>
>> The `observed` flag is very important for the lazy binding code (that powers
>> `ObservableValue#map/flatMap/orElse`), but can also be very handy when you
>> want to remove or clean-up your own listeners if you yourself are not
>> observed anymore (if nobody observes you, then why would you bother updating
>> yourself :)).
>
> I realize I didn't truly answer your question. This is not specified anywhere
> (the whole weak listener removal mechanism is basically internal stuff);
> there is only an indirect expectation that if you override add/remove
> listeners in order to track listeners (and by that I only mean empty or not
> empty, not a counter) that you would get the correct result.
>
> So this tests the implementation I did, to ensure it is compatible with
> existing JavaFX code that tracks an `observed` flag.
>
> I think improving this system somehow (more eagerly removing dead listeners)
> would require also a way to notify interested parties whether the list has
> become empty.
>
> I believe an earlier iteration tried to do this by also returning a `boolean`
> from the notify call (ie. it would return true if all listeners were removed
> during notification), but I think that ran into a wall. I think the problem
> with that approach was that JavaFX code really doesn't like it when a simple
> notify (normally harmless) has a lot of consequences (like all kinds of
> listeners getting removed in a whole chain because something went from
> observed to unobserved state).
the reason I asked this question is that we probably should test the public
APIs, not the implementation specifics. if, for example, one day there is some
change in the implementation that still conforms to the spec, the test
might/will fail.
in this case, the implementation might be changed to eagerly clear empty weak
references, then what? there might still be some value in having this test,
perhaps a comment should be added for anyone who comes after us to know that
this test assumes a particular internal implementation.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1081#discussion_r3562278188
