On Fri, 23 May 2025 09:49:09 GMT, Daniel Fuchs <[email protected]> wrote:
>> the internal state representation of the server could do with a rethink and
>> overhaul, and representing part of that state in a synchronisation primitive
>> obscures/obfuscates the server state's semantics. I don't think it's "clean
>> code"
>
> I agree that the semantic of `finished` `finishing` is a bit obfuscated. On
> the other hand I don't think we should go so far as overhauling / rethinking
> the states in this PR. If you recall there have been lots of fixes in this
> area (we had some `assert` firing at some point, and it took some time to
> find the root cause and fix).
> That said having both a boolean *and* a countdown latch would be duplicate
> info, and potentially harder to manage consistency.
> If we don't want the countdown latch we might go back to looping with
> Thread.sleep in stop, or use wait/notify, and I am not sure it would be much
> better.
I have a suggestion. Let's do this:
public final boolean finished() {
// we're finished when the latch reaches 0
return finishedLatch.getCount() == 0;
}
public final boolean isFinishing() {
return finished();
}
Then replace things like `if (finished ...` with `if (finished() ...`
That should minimize the changes and make them look less awkward.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2109317990
