RFR: 8349206: j.u.l.Handler classes create deadlock risk via synchronized publish() method

[Daniel Fuchs]

On Wed, 12 Feb 2025 19:41:44 GMT, Jason Mehrens <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).
>
> src/java.logging/share/classes/java/util/logging/FileHandler.java line 755:
> 
>> 753:         synchronized(this) {
>> 754:             flush();
>> 755:             if (limit > 0 && (meter.written >= limit || meter.written < 0)) {
> 
> I don't think we want to write to meter.written and the re-aquire the monitor to then read meter.written.  Bytes written no longer corresponds to the formatted size of the current record. It could now include sum other records pending a rotate.
> 
> Any thought on creating a package private `StreamHandler::postPublish(LogRecord)` that is called from publish while holding monitor? Then FileHandler could just override that method to invoke flush, check, and rotate.

hmmm... Thanks for chiming in Jason. Good point. So what can happen here is that if multiple threads log concurrently to the same FileHandler then log records might continue to get written to the file after the limit has been reached but before the check that would rotate is performed. In pathological cases where new records continue to arrive and manage to enter the monitor before the post-publish action that rotate can do so, this could become problematic.

I think what you are proposing would work, since we control the implementation of super.publish().

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23491#discussion_r1953326840