RFR: 8215889: assert(!_unloading) failed: This oop is not available to unloading class loader data with ZGC

[Erik Osterlund]

Hi Coleen,

Thanks for the review!

/Erik

> On 9 Jan 2019, at 17:47, coleen.phillimore at oracle.com wrote:
> 
> 
> http://cr.openjdk.java.net/~eosterlund/8215889/webrev.00/src/hotspot/share/ci/ciMethodData.cpp.udiff.html
> 
> -
> - { // During translation a safepoint can happen or VM lock can be taken (e.g., Compile_lock).
> - MutexUnlocker ml(mdo->extra_data_lock());
> - data_dst.translate_from(&data_src);
> - }
> + data_dst.translate_from(&data_src);
> 
> You don't need to unlock extra_data_lock because prepare_metadata() has already added the Method* to ci_metadata (which can take out the Compile_lock).
> 
> Adding Method* to ci_metadata will cause the Method* to not be unloaded.  This is my commentary. I think your comments about this are sufficient.
> 
> I wish PrepareExtraDataClosure was in oops/methodData.cpp but it can't be because it has to add Method* to ci_metadata handles.
> 
> This looks good.
> 
> Thanks,
> Coleen
> 
>> On 1/7/19 4:51 AM, Erik Österlund wrote:
>> Hi,
>> 
>> There are SpeculativeTrapData entries in the extra data space of MDOs that are currently not being checked for stale Method* entries due to concurrent class unloading.
>> 
>> The fix involves lazily cleaning SpeculativeTrapData entries during ciMethodData::load_extra_data(), which unpacks the extra data from the source MDO to the ci copy of the MDO, that the compiler subsequently uses as reference during the ongoing compilation, and needs to have live metadata only.
>> 
>> A new ciMethodData::prepare_metadata() method is added to ci MDO mirrors that lazily cleans the extra data space and pre-caches the ciEnv with all the metadata it encounters. When creating ciMethod handles, the Compile_lock might be taken, which strictly requires safepoint checking. Therefore, prepare_metadata() loops until it can pre-cache all live metadata without any cache misses, because that implies the subsequent code copying the MDO can not safepoint while extracting the extra data from the MDO, which is a requirement as 1) a safepoint may invalidate the metadata again, 2) both the cleaning (from the concurrent GC thread) and extraction (from the compiler thread) must be done under the mdo->extra_data_lock().
>> 
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8215889
>> 
>> Webrev:
>> http://cr.openjdk.java.net/~eosterlund/8215889/webrev.00/
>> 
>> Testing: hs-tier1-6, and a bunch of local testing, including 24 hours kitchensink in fastdebug.
>> 
>> Thanks,
>> /Erik
>