On Thu, 11 May 2023 07:51:12 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> src/java.logging/share/classes/java/util/logging/Handler.java line 88:
>>
>>> 86: return null;
>>> 87: } else {
>>> 88: return InternalLock.newLockOrNull();
>>
>> I'm surprised to see InternalLock used here. That class was created for the
>> java.io area to avoid surprises when a subclass uses a RL as the lock
>> object. I assume it's just convenience to use it here, that is, I don't
>> think the internal lock is exposed to subclasses in the j.u.logging API.
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13832#discussion_r1190772512
