RFR: 8252406: Introduce Thread::as_Java_thread() convenience function

Kim Barrett kim.barrett at oracle.com
Mon Sep 7 23:23:51 UTC 2020


> On Sep 7, 2020, at 5:47 PM, David Holmes <dholmes at openjdk.java.net> wrote:
> 
> […]
> Commit messages:
> - Initial changes for 8252406
> 
> Changes: https://git.openjdk.java.net/jdk/pull/37/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=37&range=00
>  Issue: https://bugs.openjdk.java.net/browse/JDK-8252406
>  Stats: 463 lines in 112 files changed: 15 ins; 124 del; 324 mod
>  Patch: https://git.openjdk.java.net/jdk/pull/37.diff
>  Fetch: git fetch https://git.openjdk.java.net/jdk pull/37/head:pull/37
> 
> PR: https://git.openjdk.java.net/jdk/pull/37

------------------------------------------------------------------------------
src/hotspot/share/runtime/thread.hpp
 506   JavaThread* as_Java_thread() {
 507     assert(is_Java_thread(), "incorrect cast to JavaThread");
 508     return (JavaThread*)this;
 509   }

This should be using a static_cast.  However, making that change will
uncover a problem.

This definition cannot correctly be located here. It must follow the
definition of the JavaThread class. At this point (in the body of the
Thread class definition), JavaThread is incomplete, so the C-style
cast is a reinterpret_cast. It's only by implementation accident (and
that we're not using multiple or virtual inheritance anywhere in the
vicinity), that a reinterpret_cast happens to work.

(The definition can remain in this file if that's more
convenient #include-wise than putting it in thread.inline.hpp, though
the latter would be the more usual placement.)

(One of the very real dangers with C-style casts is that you might not
be doing the cast you think you are doing.)

------------------------------------------------------------------------------
src/hotspot/share/runtime/thread.hpp
 510   JavaThread * as_const_Java_thread() const {
 511     assert(is_Java_thread(), "incorrect cast to JavaThread");
 512     return (JavaThread*)this;
 513   }

This should be

const JavaThread* as_Java_Thread() const {
  assert(is_Java_thread(), "incorrect cast to JavaThread");
  return static_cast<const JavaThread*>(this);
}

And of course, moved after the definition of JavaThread, per the above
discussion of the non-const function.

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

I reviewed all the (non-Shenandoah) gc changes, and they look good.



More information about the serviceability-dev mailing list