[jdk11u-dev] RFR: 8274338: com/sun/jdi/RedefineCrossEvent.java failed "assert(m != __null) failed: NULL mirror" [v2]

[Zhengyu Gu]

On Fri, 5 Nov 2021 15:27:36 GMT, Zhengyu Gu <zgu at openjdk.org> wrote:

>> Backport for parity with Oracle 11.0.15.
>> 
>> The original patch applies cleanly, but some of CDS tests fail due to following new assertion in `ClassLoaderData::loaded_classes_do()`
>> 
>> `  // To call this, one must have the MultiArray_lock held, but the _klasses list still has lock free reads.
>>   assert_locked_or_safepoint(MultiArray_lock);
>> `
>> The solution is to take `MultiArray_lock` in `MetaspaceShared::link_and_cleanup_shared_classes()` method
>> 
>> Test:
>> - [x]  tier1
>
> Zhengyu Gu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   @phohensee's comment

> _Mailing list message from [David Holmes](mailto:david.holmes at oracle.com) on [jdk-updates-dev](mailto:jdk-updates-dev at mail.openjdk.java.net):_
> 
> On 9/11/2021 10:50 pm, Lindenmaier, Goetz wrote:
> 
> > Hi Zhengyu,
> > Since this change has been pushed, we see four test crashing in our CI,
> > see below.  Do you have an idea what is wrong with these?
> 
> From JBS:
> 
> "The original patch applies cleanly, but needs to take additional MultiArray_lock on CDS path to get it work correctly. "
> 
> but that additional change is broken as you can't hold the lock on that path due to the linking that can occur which requires a call up to Java which can't be done when holding the mutex. I don't know why Zhengyu thought that was neccessary.
> 
> David -----

Hi David,

Sorry, I did not see your comment until now.

Adding MultiArray_lock on CDS path, as stated in comment, was to workaround the new assertion in ClassLoaderData::loaded_classes_do() in following code:

`void MetaspaceShared::link_and_cleanup_shared_classes(TRAPS) {`
`  // We need to iterate because verification may cause additional classes`
`  // to be loaded.`
` LinkSharedClassesClosure link_closure(THREAD);`
 `do {`
 `  link_closure.reset();`
 `   ClassLoaderDataGraph::loaded_classes_do(&link_closure);`
 `   guarantee(!HAS_PENDING_EXCEPTION, "exception in link_class");`
 ` } while (link_closure.made_progress());`

Should this backport drop the new assertion in `ClassLoaderData::loaded_classes_do()`?

Thanks,

-Zhengyu

-------------

PR: https://git.openjdk.java.net/jdk11u-dev/pull/584