On Tue, 21 Jul 2026 19:48:41 GMT, Andy Goryachev <[email protected]> wrote:
> still think `unlock()` should be moved to the `finally` block, otherwise
> looks good to me.
Hey Andy, I tried this, but I think it doesn't get any prettier. It would have
to be something like this:
boolean notifyListeners(ObservableValue<? extends T> observableValue, T
oldValue) {
boolean wasLocked = isLocked();
if (!wasLocked) {
lock();
}
boolean modifiedWhileLocked = false;
try {
notifyWhileLocked(observableValue, oldValue, wasLocked);
}
finally {
if (wasLocked) {
modifiedWhileLocked = unlock();
}
}
return modifiedWhileLocked;
}
Also, the exception (if somehow one bubbles through, which would be a bug in
the implementation as there really shouldn't be one) would mean that the caller
of `notifyListeners` doesn't get a chance to clean up properly. I can do the
above, but I think we should catch and log any exception thrown instead of
letting it bubble up (again, that should already be the case as all calls to
user code go through these:
static final void callInvalidationListener(ObservableValue<?> instance,
InvalidationListener listener) {
try {
listener.invalidated(instance);
}
catch (Exception e) {
Thread.currentThread().getUncaughtExceptionHandler().uncaughtException(Thread.currentThread(),
e);
}
}
static final <T> void callChangeListener(ObservableValue<? extends T>
instance, ChangeListener<T> changeListener, T oldValue, T newValue) {
try {
changeListener.changed(instance, oldValue, newValue);
}
catch (Exception e) {
Thread.currentThread().getUncaughtExceptionHandler().uncaughtException(Thread.currentThread(),
e);
}
}
Let me know if you prefer this solution!
-------------
PR Comment: https://git.openjdk.org/jfx/pull/1081#issuecomment-5038779356
