RFR (S) 8251336: Shenandoah: assert "only get here when SATB active" after JDK-8244997

[Coleen Phillimore]

On 8/11/20 9:35 AM, David Holmes wrote:
> Hi Coleen,
>
> On 11/08/2020 9:40 pm, Coleen Phillimore wrote:
>> Summary: Release OopStorage oops outside a safepoint.
>>
>> Tested with tier1-6.
>>
>> open webrev at 
>> http://cr.openjdk.java.net/~coleenp/2020/8251336.01/webrev
>> bug link https://bugs.openjdk.java.net/browse/JDK-8251336
>>
>> Thank you to Zhengyu for alerting me about this issue.
>
> Yeah ... I was focusing on the locking aspect and completely 
> overlooked the fact that section of code is within the scope of the 
> TBIVM. Not that it would have immediately rung any alarm bells.

Yes, it escaped me that TBIVM marks the thread as _thread_blocked so 
safe in a safepoint.  I thought if release were not ok there, I would 
have seen some sort of crash.
>
> Do we have usage rules about manipulating OopStorage at a safepoint? 
> If some GC's can't cope with it then there should be some detection of 
> that in the OopStorage APIs.

I don't know this.
>
>   71 static void release_oop_handles(OopHandleList* handles) {
>   72   MutexLocker ml(Service_lock, Mutex::_no_safepoint_check_flag);
>
> release_oop_handles  doesn't need to grab the Service_lock as it 
> doesn't touch any shared state.

Oh, yes, you are right.  I can remove the lock.

Thanks,
Coleen
>
> Otherwise fix looks fine.
>
> Thanks,
> David
>
>> Coleen