On Mon, 8 Nov 2021 15:46:22 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

>> src/java.base/share/classes/java/lang/Thread.java line 1396:
>> 
>>> 1394:         // at this point in time. So we fallback to creating a 
>>> Exception instance
>>> 1395:         // and printing its stacktrace
>>> 1396:         new Exception(Thread.currentThread().name + " Stack 
>>> trace").printStackTrace();
>> 
>> The recursive initialisation issue will require discussion to see if we can 
>> avoid StackWalker.getInstance return null (which I assume is masking the 
>> issue).
>> 
>> printStackTrace interacts with locking of the streams to avoid garbled 
>> output when many threads are printing to standard output output/error at the 
>> same time. If we change dumpStack to use StackWalker then it will need to do 
>> the same.
>
>> printStackTrace interacts with locking of the streams to avoid garbled 
>> output when many threads are printing to standard output output/error at the 
>> same time. If we change dumpStack to use StackWalker then it will need to do 
>> the same.
> 
> Indeed. I have updated the PR to use a lock while writing out to the 
> `System.err`.
> I had a look at the `printStackTrace()` implementation and it ends up locking 
> the `PrintStream` (`System.err`) or `PrintWriter` for the duration of the 
> entire stacktrace printing of each stacktrace element. The updated PR thus 
> uses `System.err` as the lock to match that semantic.

> 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.

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

PR: https://git.openjdk.java.net/jdk/pull/6292

Reply via email to