Round 3: RFR: 8013651 NMT: reserve/release sequence id's in incorrect order due to race

Coleen Phillimore coleen.phillimore at oracle.com
Wed Jun 12 05:12:30 PDT 2013


On 6/12/2013 5:54 AM, David Holmes wrote:
> On 12/06/2013 4:56 AM, Calvin Cheung wrote:
>> I've reviewed it with Zhengyu offline.
>
> Still requires a full review from a Reviewer.

I believe I reviewed it on the open list.   If not, the latest version 
looks good to me.

Coleen

>
> David
>
>> There are a few typos in the comment block (lines 638 - 678) in
>> memTracker.cpp.
>> He'll fix those typos before push.
>>
>> thanks,
>> Calvin
>>
>> On 6/11/2013 8:00 AM, Zhengyu Gu wrote:
>>> Hi,
>>>
>>> I still need one full reviewer.
>>>
>>> Thanks,
>>>
>>> -Zhengyu
>>>
>>> On Jun 11, 2013, at 8:33 AM, Zhengyu Gu wrote:
>>>
>>>> I think it is in ClassPathZipEntry::open_stream()
>>>> (src/share/vm/classfile/classLoader.cpp #243)
>>>>
>>>> -Zhengyu
>>>>
>>>>
>>>> On Jun 11, 2013, at 3:42 AM, David Holmes wrote:
>>>>
>>>>> On 8/06/2013 12:55 AM, Zhengyu Gu wrote:
>>>>>> On 6/6/2013 10:31 PM, David Holmes wrote:
>>>>>>> I'm confused about the expectations of the constructor:
>>>>>>>
>>>>>>> MemTracker::Tracker::Tracker(MemoryOperation op, Thread* thr)
>>>>>>>
>>>>>>> Is thr, if not NULL, always the current thread? If so, then I don't
>>>>>>> think it would be allocating from a SafepointSafe state; and if not
>>>>>>> then it could change state immediately after you have checked it!
>>>>>>>
>>>>>> Yes, thr is the current thread, and we actually do classloading in
>>>>>> _thread_in_native state, following is the stack.
>>>>>>
>>>>>> jvm.dll!MemTracker::Tracker::Tracker(MemTracker::Tracker::MemoryOperation 
>>>>>>
>>>>>> op,
>>>>>> Thread * thr)  Line 729    C++
>>>>>>      jvm.dll!MemTracker::record_arena_size(unsigned char * addr,
>>>>>> unsigned int size)  Line 344    C++
>>>>>>      jvm.dll!Arena::set_size_in_bytes(unsigned int size)  Line 532 +
>>>>>> 0xd bytes    C++
>>>>>>      jvm.dll!Arena::grow(unsigned int x,
>>>>>> AllocFailStrategy::AllocFailEnum alloc_failmode)  Line 570    C++
>>>>>>      jvm.dll!Arena::Amalloc(unsigned int x,
>>>>>> AllocFailStrategy::AllocFailEnum alloc_failmode)  Line 388 + 0x10
>>>>>> bytes    C++
>>>>>>      jvm.dll!ResourceArea::allocate_bytes(unsigned int size,
>>>>>> AllocFailStrategy::AllocFailEnum alloc_failmode)  Line 71    C++
>>>>>>      jvm.dll!resource_allocate_bytes(unsigned int size,
>>>>>> AllocFailStrategy::AllocFailEnum alloc_failmode)  Line 39    C++
>>>>>>      jvm.dll!ClassPathZipEntry::open_stream(const char * name)  Line
>>>>>> 265 + 0xe bytes    C++
>>>>>>      jvm.dll!LazyClassPathEntry::open_stream(const char * name) Line
>>>>>> 323    C++
>>>>>>      jvm.dll!ClassLoader::load_classfile(Symbol * h_name, Thread *
>>>>>> __the_thread__)  Line 900 + 0x17 bytes    C++
>>>>>> jvm.dll!SystemDictionary::load_instance_class(Symbol *
>>>>>> class_name,
>>>>>> Handle class_loader, Thread * __the_thread__)  Line 1284 + 0x14 
>>>>>> bytes
>>>>>> C++
>>>>>> jvm.dll!SystemDictionary::resolve_instance_class_or_null(Symbol *
>>>>>> name, Handle class_loader, Handle protection_domain, Thread *
>>>>>> __the_thread__)  Line 769 + 0x18 bytes    C++
>>>>>>      jvm.dll!SystemDictionary::resolve_or_null(Symbol * class_name,
>>>>>> Handle class_loader, Handle protection_domain, Thread *
>>>>>> __the_thread__)
>>>>>> Line 227 + 0x15 bytes    C++
>>>>>>      jvm.dll!SystemDictionary::resolve_or_fail(Symbol * class_name,
>>>>>> Handle class_loader, Handle protection_domain, bool throw_error,
>>>>>> Thread
>>>>>> * __the_thread__)  Line 166 + 0x15 bytes    C++
>>>>>> jvm.dll!ConstantPool::klass_at_impl(constantPoolHandle this_oop,
>>>>>> int which, Thread * __the_thread__)  Line 252 + 0x17 bytes    C++
>>>>>>      jvm.dll!ConstantPool::klass_at(int which, Thread *
>>>>>> __the_thread__)  Line 352 + 0x1b bytes    C++
>>>>>>      jvm.dll!InterpreterRuntime::anewarray(JavaThread * thread,
>>>>>> ConstantPool * pool, int index, int size)  Line 187 + 0x10 bytes C++
>>>>>>      02ee6241()
>>>>> So this is an IRT_ENTRY that puts us _thread_in_vm. Where does the
>>>>> transition to _thread_in_native occur in the subsequent frames?
>>>>>
>>>>> David
>>>>> -----
>>>>>
>>>>>> jvm.dll!JavaCalls::call_helper(JavaValue * result, methodHandle *
>>>>>> m, JavaCallArguments * args, Thread * __the_thread__) Line 402 + 
>>>>>> 0x36
>>>>>> bytes    C++
>>>>>>      jvm.dll!os::os_exception_wrapper(void (JavaValue *, 
>>>>>> methodHandle
>>>>>> *, JavaCallArguments *, Thread *)* f, JavaValue * value,
>>>>>> methodHandle *
>>>>>> method, JavaCallArguments * args, Thread * thread)  Line 113 + 0x13
>>>>>> bytes    C++
>>>>>>      jvm.dll!JavaCalls::call(JavaValue * result, methodHandle 
>>>>>> method,
>>>>>> JavaCallArguments * args, Thread * __the_thread__)  Line 307 + 0x1a
>>>>>> bytes    C++
>>>>>> jvm.dll!InstanceKlass::call_class_initializer_impl(instanceKlassHandle 
>>>>>>
>>>>>> this_oop, Thread * __the_thread__)  Line 1125 + 0x1f bytes    C++
>>>>>> jvm.dll!InstanceKlass::call_class_initializer(Thread *
>>>>>> __the_thread__)  Line 1093 + 0xd bytes    C++
>>>>>> jvm.dll!InstanceKlass::initialize_impl(instanceKlassHandle
>>>>>> this_oop, Thread * __the_thread__)  Line 843    C++
>>>>>>      jvm.dll!InstanceKlass::initialize(Thread * __the_thread__) Line
>>>>>> 502 + 0xd bytes    C++
>>>>>>      jvm.dll!initialize_class(Symbol * class_name, Thread *
>>>>>> __the_thread__)  Line 973 + 0x20 bytes    C++
>>>>>>      jvm.dll!Threads::create_vm(JavaVMInitArgs * args, bool *
>>>>>> canTryAgain)  Line 3479 + 0xf bytes    C++
>>>>>>      jvm.dll!JNI_CreateJavaVM(JavaVM_ * * vm, void * * penv, void *
>>>>>> args)  Line 5113 + 0xd bytes    C++
>>>>>>      java.exe!013713c1()
>>>>>>      [Frames below may be incorrect and/or missing, no symbols 
>>>>>> loaded
>>>>>> for java.exe]
>>>>>>      java.exe!01371e78()
>>>>>>      java.exe!0137cf67()
>>>>>>      java.exe!0137ab83()
>>>>>>      java.exe!0137cfa5()
>>>>>>      java.exe!0137ac0d()
>>>>>>      kernel32.dll!768b3677()
>>>>>>      ntdll.dll!76fe9f42()
>>>>>>      ntdll.dll!76fe9f15()
>>>>>>
>>>>>>
>>>>>>> In memTracker.hpp for the !INCLUDE_NMT case class Tracker doesn't
>>>>>>> have
>>>>>>> an allocation-type as a supertype. I'm also unclear whether the
>>>>>>> methods that have:
>>>>>>>
>>>>>>> return Tracker();
>>>>>>>
>>>>>>> are going to be returning a stack-allocated object, or whether this
>>>>>>> will actually force use of a copy-constructor at the call site. We
>>>>>>> really want this to be a no-op (or as close as possible) when 
>>>>>>> NMT is
>>>>>>> not compiled in. Perhaps a static singleton instance of Tracker 
>>>>>>> could
>>>>>>> be used instead?
>>>>>>>
>>>>>> Yes, static on can help solaris. Windows and Linux all optimize the
>>>>>> copy
>>>>>> constructor away.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> -Zhengyu
>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>> On 5/06/2013 12:26 AM, Zhengyu Gu wrote:
>>>>>>>> Round 3:
>>>>>>>>
>>>>>>>> Based on Coleen and Stefan's comment, reverted most of NMT 
>>>>>>>> tracking
>>>>>>>> calls to original to reduce code changes.
>>>>>>>>
>>>>>>>> http://cr.openjdk.java.net/~zgu/8013651/webrev.03/
>>>>>>>> <http://cr.openjdk.java.net/%7Ezgu/8013651/webrev.03/>
>>>>>>>>
>>>>>>>>
>>>>>>>> Tests:
>>>>>>>>   JPRT
>>>>>>>>   vm.quick.testlist on Linux 32
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>> -Zhengyu
>>>>>>>>
>>>>>>>>
>>>>>>>> On 5/22/2013 10:28 AM, Zhengyu Gu wrote:
>>>>>>>>> Based on the discussion with Karen, Coleen and Harold, following
>>>>>>>>> changes are made:
>>>>>>>>>
>>>>>>>>> 1) Renamed NMTTrackOp to NMTTracker, avoid the confusion with VM
>>>>>>>>> operations.
>>>>>>>>> 2) Used NMTTracker's dtor to discard the tracking operation if no
>>>>>>>>> recording is done.
>>>>>>>>>
>>>>>>>>> Tests:
>>>>>>>>> - JPRT
>>>>>>>>> - vm.quick.testlist on Linux 32, Solaris sparcv9 and Windows 32.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> -Zhengyu
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 5/14/2013 10:01 AM, Zhengyu Gu wrote:
>>>>>>>>>> There can be race conditions between the memory operations 
>>>>>>>>>> and the
>>>>>>>>>> book keeping records are written. For example, thread 1 
>>>>>>>>>> releases a
>>>>>>>>>> virtual memory block, before it can write the release record,
>>>>>>>>>> thread
>>>>>>>>>> 2 reserves the same virtual memory block and writes reservation
>>>>>>>>>> first, as result, NMT indicates the block is "released".
>>>>>>>>>>
>>>>>>>>>> The solution is that, for those operations that can cause the 
>>>>>>>>>> race
>>>>>>>>>> conditions, NMT should pre-reserve sequence number for it, if 
>>>>>>>>>> the
>>>>>>>>>> operation succeeds, NMT uses pre-reserved sequence number to 
>>>>>>>>>> write
>>>>>>>>>> the record.
>>>>>>>>>>
>>>>>>>>>> The tricky part is that, a sequence number is only good for the
>>>>>>>>>> generation it is acquired, when there are reserved sequence
>>>>>>>>>> number,
>>>>>>>>>> NMT has to prevent itself from entering so called "sync-point"
>>>>>>>>>> where
>>>>>>>>>> the generation can be advanced.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Bug: http://bugs.sun.com/view_bug.do?bug_id=8013651
>>>>>>>>>> Webrev: http://cr.openjdk.java.net/~zgu/8013651/webrev/
>>>>>>>>>> <http://cr.openjdk.java.net/%7Ezgu/8013651/webrev/>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Tests:
>>>>>>>>>>   1) JPRT
>>>>>>>>>>   2) vm.quick.testlist on Linux 32, Linux x64 and Solaris 
>>>>>>>>>> Sparcv9
>>>>>>>>>>   3) Kitchensink on Linux 32, Linux x64, Solaris Sparcv9 and
>>>>>>>>>> Windows
>>>>>>>>>> x64
>>>>>>>>>>   4) NMT jtreg tests on Linux 32, Linux x64 and Solaris Sparcv9
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>> -Zhengyu
>>



More information about the hotspot-runtime-dev mailing list