RFR: 8203885: ConcurrentLocksDump::dump_at_safepoint() should not allocate array in resource area

Kim Barrett kim.barrett at oracle.com
Tue May 29 06:09:20 UTC 2018


> On May 28, 2018, at 8:16 AM, Per Liden <per.liden at oracle.com> wrote:
> 
> ConcurrentLocksDump::dump_at_safepoint() creates a GrowableArray, which gets allocated in a resource area. This array is than passed down a call chain, where it can't control that another ResourceMark isn't created. In the leaf of this call chain, a closure (FindInstanceClosure) is executed, which appends to the array, which means it might need to be resized. This doesn't work if a new ResourceMark has been created, since the array resize will happen in a nested ResourceArea context. As a result, the append operation fails in GenericGrowableArray::check_nesting().
> 
> This has so far gone unnoticed because CollectedHeap::object_iterate() in existing collectors typically don't create new ResourceMarks. This is not true for ZGC (and potentially other concurrent collectors), which needs to walk thread stacks, which in turn requires a ResourceMark.
> 
> The proposed fix is to make this array C Heap allocated.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8203885
> Webrev: http://cr.openjdk.java.net/~pliden/8203885/webrev.0
> 
> Testing: hs-tier{1,3}
> 
> /Per

Looks good. 

I was going to ask why the GrowableArray needs to be heap allocated?
Why not just

  GrowableArray<oop> aos_objects(INITIAL_ARRAY_SIZE, true);
  ... s/aos_objects/&aos_objects/ ...
  // delete aos_objects no longer needed

After some digging, probably because of this, from growableArray.hpp:

119    assert(!on_C_heap() || allocated_on_C_heap(), "growable array must be on C heap if elements are");

Conflating the allocation of the GrowableArray object with the
allocation of the underlying array.  That seems wrong, but not a
problem to be solved as part of this change.



More information about the hotspot-dev mailing list