On Mon, 8 Nov 2021 15:54:58 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>>> The recursive initialisation issue will require discussion to see if we can >>> avoid StackWalker.getInstance return null (which I assume is masking the >>> issue). >> >> For a better context, here's the stacktrace of such a call to >> `Thread.dumpStack` during the class initialization of `StackWalker`: >> >> Caused by: java.lang.NullPointerException: Cannot invoke >> "java.lang.StackWalker.forEach(java.util.function.Consumer)" because the >> return value of "java.lang.StackWalker.getInstance()" is null >> at java.base/java.lang.Thread.dumpStack(Thread.java:1383) >> at >> java.base/java.security.AccessController.checkPermission(AccessController.java:1054) >> at >> java.base/java.lang.SecurityManager.checkPermission(SecurityManager.java:411) >> at >> java.base/java.lang.reflect.AccessibleObject.checkPermission(AccessibleObject.java:91) >> at java.base/java.lang.reflect.Method.setAccessible(Method.java:193) >> at java.base/java.lang.Class$3.run(Class.java:3864) >> at java.base/java.lang.Class$3.run(Class.java:3862) >> at >> java.base/java.security.AccessController.doPrivileged(AccessController.java:318) >> at java.base/java.lang.Class.getEnumConstantsShared(Class.java:3861) >> at java.base/java.lang.System$2.getEnumConstantsShared(System.java:2295) >> at java.base/java.util.EnumSet.getUniverse(EnumSet.java:408) >> at java.base/java.util.EnumSet.noneOf(EnumSet.java:111) >> at java.base/java.lang.StackWalker.<clinit>(StackWalker.java:291) >> >> As you will notice, this call comes from the security (permission check) >> layer when `StackWalker` class is being `clinit`ed. The check for >> `StackWalker.getInstance()` being `null`, in the `Thread.dumpStack()` >> implementation is indeed almost a hackish way to identify this case where >> `StackWalker`'s `clinit` is in progress (in the current thread). I can't >> think of a different way to handle this use case, so looking forward to any >> suggestions. > >> The updated PR thus uses System.err as the lock to match that semantic. > > The test case has also been updated to add a new test which invokes > Thread.dumpStack() from multiple threads and verifies that the stacktrace > written out isn't garbled. Running the test _without_ the addition of the > lock on System.err in the Thread.dumpStack implementation does indeed show > the garbled output and causes the test failure (which is a good thing). I'm uncomfortable with this change, does the change have any benefit? ------------- PR: https://git.openjdk.java.net/jdk/pull/6292
