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