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

[Alan Bateman]

On Thu, 11 May 2023 07:52:51 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?
>
> I can revert to using plain `ReentrantLock` if you think it's preferable.

> 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.

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

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