On Tue, 25 Feb 2025 01:12:55 GMT, Stuart Marks <[email protected]> wrote:
>> David Beaumont has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Tweaking @implNote for better rendering.
>
> src/java.logging/share/classes/java/util/logging/Handler.java line 62:
>
>> 60: * {@link #isLoggable(LogRecord)} while allowing code synchronized on the
>> 61: * {@code Handler} instance to set a new log level.
>> 62: * <p>
>
> The above discussion is mostly internal to Handler implementations so I don't
> think it belongs in public javadoc. The implementation comments at line 91
> below cover this (though they could be clarified a bit) but I think the
> mutables-volatile/unlocked-get/locked-set convention is mostly orthogonal to
> the deadlock issue we're trying to solve here. I'd recommend simply removing
> this material.
>
> As an aside I think using volatiles this way is quite error-prone and I don't
> like advertising it in the javadoc.
Fair enough. Logging is a weird space in which certain
synchronization/consistency strategies become quite important for performance,
so I was thinking it would be useful to explain it a bit more. I agree it's not
a "neat" solution, but it can can be used safely. There are definitely a few
pitfalls here though.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23491#discussion_r1969556294
