On Wed, 2 Apr 2025 13:39:22 GMT, David Beaumont <d...@openjdk.org> wrote:
>> 8349206: j.u.l.Handler classes create deadlock risk via synchronized
>> publish() method.
>>
>> 1. Remove synchronization of calls to publish() in Handlers in
>> java.util.logging package.
>> 2. Add explanatory comments to various affected methods.
>> 3. Add a test to ensure deadlocks no longer occur.
>>
>> Note that this change does not address issue in MemoryHandler (see
>> JDK-8349208).
>
> David Beaumont has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Reworking user warnings about synchronization and deadlocking based on
> Joe's comments.
src/java.logging/share/classes/java/util/logging/Handler.java line 61:
> 59: * their {@link #publish(LogRecord)} method, or calls to {@code
> 60: * super.publish()}, since these are always expected to need to process
> and
> 61: * format user-supplied arguments.
I had to read this paragraph twice to realize it didn't say that subclasses
shouldn't call
super.publish(). What about slightly changing the wording as follows:
Suggestion:
* As such, general purpose {@code Handler} subclasses should not synchronize
* their {@link #publish(LogRecord)} method, or call {@code
* super.publish()} while holding locks, since these are typically expected to
need to process and
* format user-supplied arguments.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23491#discussion_r2024881280
