Request for reviews (XS): 7011386: race in objArrayKlass::array_klass_impl (original) (raw)

Tom Rodriguez tom.rodriguez at oracle.com
Tue Jan 11 12:35:57 PST 2011

On Jan 11, 2011, at 11:15 AM, Vladimir Kozlov wrote:

Tom,

Using oopstore() instead of oopstorewithoutcheck() will generate barriers but it is overkill.

Those are gc barriers. I was really referring to fences. The code is a classic instance of the double check locking idiom for lazy init:

objArrayKlassHandle ak (THREAD, this_oop->higher_dimension()); if (ak.is_null()) { if (or_null) return NULL;

ResourceMark rm;
JavaThread *jt = (JavaThread *)THREAD;
{
  MutexLocker mc(Compile_lock, THREAD);   // for vtables
  // Ensure atomic creation of higher dimensions
  MutexLocker mu(MultiArray_lock, THREAD);

  // Check if another thread beat us
  ak = objArrayKlassHandle(THREAD, this_oop->higher_dimension());
  if( ak.is_null() ) {

So doesn't that mean that we need a fence of some kind?

tom

I think marking receiver as volatile should be enough:

void sethigherdimension(klassOop k) volatile { oopstorewithoutcheck((oop*) &higherdimension, (oop) k); } void setlowerdimension(klassOop k) volatile { oopstorewithoutcheck((oop*) &lowerdimension, (oop) k); } Thanks, Vladimir Tom Rodriguez wrote: That look good, though I keep wondering if we need a barrier in between or if those fields really should be volatile. It seems like we're playing a little loose with the locking for these lazy values. tom On Jan 11, 2011, at 9:54 AM, Vladimir Kozlov wrote:

http://cr.openjdk.java.net/~kvn/7011386/webrev

Fixed 7011386: race in objArrayKlass::arrayklassimpl Other threads may access lowerdimension field before it is initialized by thread which holds the lock in objArrayKlass::arrayklassimpl(). Move lowerdimension field initialization before higherdimension.

More information about the hotspot-runtime-dev mailing list