RFR: JDK-8214097: Rework thread initialization and teardown logic
david.holmes at oracle.com
Fri Dec 21 02:48:09 UTC 2018
On 21/12/2018 1:07 am, Markus Gronlund wrote:
> Thanks David for doing this work.
> Looks good.
Thanks for looking at this.
> - Moved the JavaThread call to this->exit(false); from the end of thread_main_inner to the start of post_run - as Markus suggested. This also necessitated a change to the gtest logic compared to the original as it has to override post_run() now to avoid the exit call."
> Just so that I understand the reason for the updates to the gtest part:
> The reason you need to add a post_run() specialization in threadHelper.inline.hpp, is because the JavaThread::post_run() now invokes JavaThread::exit() (thanks for accommodating that btw), and JavaThread::exit() expects the thread to have a valid thread oop, something that the gtests do not setup/create?
> JavaThread::exit() contains assertions like:
> assert(threadObj.not_null(), "Java thread object should be created");
That's correct. And without a Thread oop, and being a real Java thread,
little else in JavaThread::exit makes any sense.
> Maybe the comment in threadHelper.inline.hpp:
> // override as JavaThread::post_run() calls Thread::exit
> Could become
> // override as JavaThread::post_run() calls JavaThread::exit() which expect a valid thread object oop
Sure. The Thread::exit rather than JavaThread::exit was a typo.
> -----Original Message-----
> From: David Holmes
> Sent: den 19 december 2018 22:22
> To: hotspot-dev developers <hotspot-dev at openjdk.java.net>; KIM.BARRETT <kim.barrett at oracle.com>; Markus Grönlund <markus.gronlund at oracle.com>
> Subject: RFR: JDK-8214097: Rework thread initialization and teardown logic
> Following on from the preliminary RFR:
> I've merged with Kim's changes from 8215097.
> Bug: https://bugs.openjdk.java.net/browse/JDK-8214097
> Webrev: http://cr.openjdk.java.net/~dholmes/8214097/webrev.v2/
> Changes since previous discussion:
> - Replaced Kim's initial_thread_created assertion with a check of Thread::current_or_null, with an explanation
> - Removed NJT list addition at construction time as it's not needed after 8215097.
> - Moved the JavaThread call to this->exit(false); from the end of thread_main_inner to the start of post_run - as Markus suggested. This also necessitated a change to the gtest logic compared to the original as it has to override post_run() now to avoid the exit call.
> - Made get_thread_name_string virtual so that the JavaThread subclasses in the gtest can override it and allow their names to be seen in hs_err files. (Makes debugging so much easier when you know which thread crashed!).
> - Mach 5 tiers 1 - 3
> - All runtime tests with:
> - default GC
> - ZGC
> - Shenandoah GC
> - gc/g1
> - gc/serial
> - gc/epsilon
> - gc/shenandoah
More information about the hotspot-dev