On Fri, 23 May 2025 09:07:51 GMT, Mark Sheppard <mshep...@openjdk.org> wrote:

>> Yes, but this is an existing function and it is used in other classes. I 
>> didn't change the functionality
>
> 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.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25333#discussion_r2104238253

Reply via email to