RFR 8029178: Parallel class loading test anonymous-simple gets SIGSEGV in Metaspace:,:contains

[Coleen Phillmore]

Jon,  Thanks for reviewing this.

On 1/3/2014 1:09 PM, Jon Masamitsu wrote:
> Coleen,
>
> http://cr.openjdk.java.net/~coleenp/8029178/src/share/vm/memory/metachunk.hpp.frames.html 
>
>
> 146   bool contains(const void* ptr) { return bottom() < ptr && ptr <= 
> _top; }
>
> I think bottom() points to the first chunk.  Also _top points to the 
> start
> of the unallocated space (next place where a chunk will be allocated).
> So I would think this should be
>
> bool contains(const void* ptr) { return bottom() <=ptr && ptr < _top; }
>
>

Yes, I agree.
>
> http://cr.openjdk.java.net/~coleenp/8029178/src/share/vm/memory/metaspace.cpp.frames.html 
>
>
> Don't you need some locking to protect the "chunks_in_use()" lists?
>
> 2382 bool SpaceManager::contains(const void *ptr) {
> 2383   for (ChunkIndex i = ZeroIndex; i < NumberOfInUseLists; i = 
> next_chunk_index(i))
> 2384   {
> 2385     Metachunk* curr = chunks_in_use(i);
> 2386     while (curr != NULL) {
> 2387       if (curr->contains(ptr)) return true;
> 2388       curr = curr->next();
> 2389     }
> 2390   }
> 2391   return false;
> 2392 }
> 2393

If new chunks are added while this is iterating over them, they are 
added to the front of the list.  This is ok because we would only call 
this for metadata that is already allocated in one of the chunks.   I 
don't want to add a lock but I do need some memory ordering 
instructions.   I'll move the one that I had on virtual space lists to 
where we add the chunks.

The Metaspace/SpaceManager destructors can only be called for the 
_unloading list, which is created at a safepoint and we don't walk those 
lists in CLDG::contains.

Thank you for pointing these out.   I'll retest and send out another webrev.

Coleen
>
> Jon
>
>
> On 1/2/2014 9:25 AM, Coleen Phillmore wrote:
>> Summary: Metaspace::contains cannot look at purged metaspaces while 
>> CMS concurrently deallocates them.
>>
>> Removed 2 calls to is_metaspace_object where the object may be in a 
>> deallocated metaspace.  Removed walking virtual space lists for 
>> determining contains because the virtual space list can change 
>> concurrently with the walk.   CLDG::contains is slower but no 
>> slowdowns with testing were observed.
>>
>> Tested by SQE testbase tests, jtreg tests.   Functional testing by 
>> parallel class loading tests and nsk/coverage/arguments/arguments008 
>> (ie. calls Method::is_valid_method)
>>
>> open webrev at http://cr.openjdk.java.net/~coleenp/8029178/
>> bug link https://bugs.openjdk.java.net/browse/JDK-8029178
>>
>> Thanks,
>> Coleen
>