Round 3: RFR: 8013651 NMT: reserve/release sequence id's in incorrect order due to race
Zhengyu Gu
zhengyu.gu at oracle.com
Tue Jun 11 08:00:45 PDT 2013
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