RFR: 8154750: Add missing OrderAccess operations to ClassLoaderData lock-free data structures

Kim Barrett kim.barrett at oracle.com
Thu May 26 19:56:59 UTC 2016


> On May 24, 2016, at 4:03 AM, David Holmes <david.holmes at oracle.com> wrote:
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8154750
> 
> webrev: http://cr.openjdk.java.net/~dholmes/8154750/webrev/

Thanks for fixing these uses of DCLP.

------------------------------------------------------------------------------ 
src/share/vm/classfile/classLoaderData.cpp 
  80 // helper function to avoid in-line casts
  81 template <typename T> static T load_ptr_acquire(volatile T* p) {
  82   return static_cast<T>(OrderAccess::load_ptr_acquire(p));
  83 }

For better type safety, maybe this should instead be

template<typename T> static T* load_ptr_acquire(T* volatile * p) {
  return static_cast<T*>(OrderAccess::load_ptr_acquire(p));
}

This also seems like it might be generally useful.  Would it make
sense to add it to OrderAccess?

------------------------------------------------------------------------------ 
src/share/vm/classfile/classLoaderData.cpp 
 197 void ClassLoaderData::modules_do(void f(ModuleEntry*)) {
 209 void ClassLoaderData::packages_do(void f(PackageEntry*)) {

Based on the RFR, I'm guessing these are only called at a safepoint or
while holding Module_lock?  Maybe add assert_locked_or_safepoint to
these too?

(Maybe these are only called from the corresponding
ClassLoaderDataGraph functions, which already have such an assertion.
But more local assertions in the CLD functions would make things
easier to understand.)

------------------------------------------------------------------------------ 
src/share/vm/classfile/classLoaderData.cpp
 352 // This is called by InstanceKlass::deallocate_contents() to remove the
 353 // scratch_class for redefine classes.  We need a lock because there it may not
 354 // be called at a safepoint if there's an error.
 355 void ClassLoaderData::remove_class(Klass* scratch_class) {
     ... and modifies _klasses list ...

How are the unlocked iterators over the _klasses list protected from
the modifications performed here?  The description is suggestive that
they might not be (e.g. need lock because might not be at a
safepoint). 

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



More information about the hotspot-runtime-dev mailing list