On Thu, 21 Nov 2024 10:24:28 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:

>> This PR remove usage of SecurityManager, doPrivileges, etc... from 
>> `java.logging` and `java.base/jdk.internal.logger`
>> 
>> Only notable hack - Logger.checkPermission() no longer checks permissions, 
>> but has been renamed into `ensureLogManagerInitialized()` in order to avoid 
>> disturbing the initialization sequence of Logger/LogManager.
>> 
>> I am not 100% sure this is still needed - but I remember we had some 
>> entanglement issues in the past that had been hard to solve, so it seemed 
>> prudent to keep the call:
>> 
>> 
>>     if (manager == null) {
>>         manager = LogManager.getLogManager();
>>     }
>> 
>> 
>> where `manager` is a private volatile field in Logger.
>> 
>> I also took the opportunity to remove the locking workaround that had been 
>> introduced to support Virtual Threads and revert to using synchronized in 
>> the Handler classes now that JEP 491 has been integrated.
>
> Daniel Fuchs has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Review feedback

I noticed a comment that can be removed. Maybe next time you are in this file 
you can remember to remove it or you could fix it as part of another cleanup.

src/java.base/share/classes/jdk/internal/logger/LazyLoggers.java line 346:

> 344:             // the result.
> 345:             // This is just an optimization to avoid the cost of calling
> 346:             // doPrivileged every time.

These 2 lines can be removed.

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

PR Review: https://git.openjdk.org/jdk/pull/22281#pullrequestreview-2458923165
PR Review Comment: https://git.openjdk.org/jdk/pull/22281#discussion_r1856905654

Reply via email to