RFR: 8349206: j.u.l.Handler classes create deadlock risk via synchronized publish() method [v9]
Daniel Fuchs
dfuchs at openjdk.org
Fri Feb 28 09:30:59 UTC 2025
On Thu, 27 Feb 2025 17:15:56 GMT, David Beaumont <duke at 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:
>
> Fix @implNote to @apiNote.
src/java.logging/share/classes/java/util/logging/StreamHandler.java line 240:
> 238: void synchronousPostWriteHook() {
> 239: // Empty by default.
> 240: }
It would be good to have a comment making it clear that this method is called while holding the monitor on this. I know we have a test for that - but it could be good to make it clear in the code as well.
What about:
Suggestion:
void synchronousPostWriteHook() {
// Empty by default. We could do:
// assert Thread.holdsLock(this);
// but this is already covered by unit tests.
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23491#discussion_r1975092107
More information about the core-libs-dev
mailing list