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.

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

Commit messages:
 - 8153133: Thread.dumpStack() can use StackWalker

Changes: https://git.openjdk.java.net/jdk/pull/6292/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6292&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8153133
  Stats: 171 lines in 3 files changed: 170 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6292.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6292/head:pull/6292

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

Reply via email to