On Tue, 15 Oct 2024 17:01:59 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>>> While making `LogManager.checkAccess` be a no-op might be more convenient, >>> it could unconditionally permit operations that formerly required a >>> permission check: clearly a bad idea. Always throwing a `SecurityException` >>> is the safest option. >> >> It's not about convenience _or_ safety; this part of the change has a >> provably flawed logical basis. >> >> These methods would no longer called from within the JDK after this change. >> All three of these methods were already previously defined to be a no-op >> when no security manager was installed (specifically when >> `System.getSecurityManager() == null`). Since no security manager may be >> installed after this change, this method will always return `null`. Thus, a >> no-op is still the most correct behavior and does not permit any operation >> that previously required a permission check (since it was already a no-op >> any time no security manager was installed, which will now be the only >> possible scenario). Therefore it is provably no safer to throw >> `SecurityException` here, since this will only prompt library developers to >> introduce the workaround I posted above when their tests fail, yielding the >> exact same result (except with a minor inconvenience to library developers). >> >> Either way is fine (as I said, the workaround is trivial), but IMO it's best >> to be conscious of the correct reasoning lest flawed assumptions _do_ end up >> enabling the introduction of unsafe changes elsewhere in the code. We don't >> have to make any assumptions about safety or previous behavior because it's >> all statically provable. > > I see your point now. We have strived to preserve compatibility with > libraries that follow well known code patterns as described in the JEP, so I > will discuss this with others who are more familiar with the compatibility > risk of making this change. I have changed `LogManager.checkAccess`, `Thread.checkAccess`, `ThreadGroup.checkAccess` to always do nothing. See the fixes in https://github.com/openjdk/jdk/pull/21498/commits/2ebb6de50194770658462507cee28b785fb30dbd and https://github.com/openjdk/jdk/pull/21498/commits/16e17b89b3dbce4f49706c032c315977dd54c315. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21498#discussion_r1806950866
