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

Jaikiran Pai has updated the pull request incrementally with one additional 
commit since the last revision:

  fix jcheck failure - convert tab to space

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

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6292/files
  - new: https://git.openjdk.java.net/jdk/pull/6292/files/aa5f6288..95739ab4

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6292&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6292&range=01-02

  Stats: 6 lines in 1 file changed: 0 ins; 0 del; 6 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