On Fri, 23 May 2025 09:49:09 GMT, Daniel Fuchs <dfu...@openjdk.org> 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

Reply via email to