RFR: 8307535: java.util.logging.Handlers should be more VirtualThread friendly [v3]

[Daniel Fuchs]

On Thu, 11 May 2023 10:13:04 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>>> It's the same reason here: in these classes (and before that change) the lock is `this` which is always exposed to subclasses or external classes. If a handler uses `InternalLock`, and an external class `synchronize(handler)` that could cause surprising effects. My first take at this was simply using `new ReantrantLock()` but I thought it made sense to reuse `InternalLock` instead. After all, there would be no point in not using `synchronized` in StreamHandler if the underlying output stream is a PrintStream for which use of InternalLock has been disabled?
>> 
>> The reason for InternalLock is because the Reader/Write "lock" field is exposed to subclasses and there is a possibility that a subclass could set the lock field to an instance of ReentrantLock and confusing all the locking.  You don't have this issue in j.u.logging. I am not objecting to using InternalLock, just surprised to see it being used here as I had assumed you'd just create your own explicit lock when not subclassed.
>
> It's the same usage than in `PrintStream`: the lock in `PrintStream` is an `InternalLock` even though it's never exposed to subclasses (it's a private field). My rationale was that if the underlying `PrintStream` uses `synchronized` and doesn't use `InternalLock`, because `-Djdk.io.useMonitors=true`, then there's no point in the `Handler` trying to avoid using `synchronized`. Though I admit that not all `Handlers` wrap a `PrintStream`, the `FileHandler` and `ConsoleHandler` (which are the more important ready-to-use concrete implementations) will eventually delegate to some underlying IO class that will be impacted by `-Djdk.io.useMonitors=true`. So I was thinking that we could/should use the same logic there.

Ah - I see that `PrintStream` lock can be accessed through SharedSecrets... Hmmm. OK - then maybe I should leave InternalLock alone and just use ReentrantLock. Let me prototype that.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13832#discussion_r1190953432