RFR: 8344235: Revisit SecurityManager usage in java.logging after JEP 486 and JEP 491 integration [v2]

Sean Mullan mullan at openjdk.org
Mon Nov 25 16:39:22 UTC 2024


On Thu, 21 Nov 2024 10:24:28 GMT, Daniel Fuchs <dfuchs at 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


More information about the core-libs-dev mailing list