RFR: 8153133: Thread.dumpStack() can use StackWalker

[Alan Bateman]

On Mon, 8 Nov 2021 10:45:02 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

> Can I please get a review of this change which seeks to implement the enhancement noted in https://bugs.openjdk.java.net/browse/JDK-8153133?
> 
> The commit in this PR uses the `StackWalker` API to dump the stacktrace of the thread. A few things to note about this change:
> 
> - Previously, the dumped stacktrace would be preceded by a line (from the `Exception` instance) which would state `java.lang.Exception: Stack trace` and the rest of the lines would be the stacktrace. The change in this PR does a small (unrelated) enhacement  which now includes the thread name in the message. So something like `MainThread Stack trace` where `MainThread` is the name of the thread whose stacktrace is being dumped. The linked JBS doesn't mention this change as a necessity but I decided to include it in this PR since I've always missed the thread name in that dumped stacktrace and since we are now using the StackWalker API, the mention of `java.lang.Exception: ...` line in the thread dump will no longer make sense (unless we wanted backward compatibility)
> - The change doesn't use lambda to allow `Thread.dumpStack()` to be called (for debugging purposes) from places where lambda usage might be too early. 
> - The change also includes a fallback to use the `Exception.printStackTrace()` to allow for calls to `Thread.dumpStack()` when `StackWalker` class itself is being initialized.
> 
> The PR also includes a new jtreg test which verifies this change. The test has 3 test actions - one without security manager, one with security manager and one with `java.security.debug=access,stack` to exercise the case where Thread.dumpStack() gets called during StackWalker class initialization.
> 
> The linked JBS issue notes that it would be good to use the StackWalker API even in the `Thread.getStackTrace` implementation. This PR however doesn't include that change because I wanted to tackle that separately since I think that would need to run some performance benchmarks to evaluate any performance changes. The current `Thread.dumpStack` method clearly states that it is meant for debugging purposes so I haven't run any kind of performance benchmarks on this change.

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.

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

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