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

[Coleen Phillimore]

On 8/11/20 6:21 PM, Roman Kennke wrote:
> On Wed, 2020-08-12 at 08:01 +1000, David Holmes wrote:
>> On 12/08/2020 12:42 am, Roman Kennke wrote:
>>> On Tue, 2020-08-11 at 23:35 +1000, 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.
>>>>
>>>> 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.
>>> In general, non-GC threads should not mess with GC roots (e.g.
>>> OopStorage) during safepoints. I don't think that this plays well
>>> with
>>> any GC. Not sure if it can be automatically detected, it sure would
>>> be
>>> nice if that would fire an assert or so.
>> I will file a RFE for that.
> In-fact, this current bug is kinda what we want, except that the assert
> that we've seen is buried in GC specific code while it should be
> generic, and was checking another specific invariant (which is violated
> more rarely: only when observing the race), while we should explicitely
> check that threads do not mess with GC roots. This can probably be
> achieved in the heap access layer somehow.

Yes, I would have appreciated an assert earlier.  Thanks for your 
comments and to @David for the RFE.

Coleen
>
> Roman
>
>