[14] RFR: 8235982: AArch64: gc/stress/jfr/TestStressAllocationGCEventsWithParallel.java assertion failure

[Stefan Johansson]

Hi Nick,

Thanks for fixing this issue.

On 2019-12-17 07:22, Nick Gasson wrote:
> Hi,
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8235982
> Webrev: http://cr.openjdk.java.net/~ngasson/8235982/webrev.01/
> 
> The above test has been intermittently failing with an assertion error 
> on AArch64 since the changes in "8220465: Use shadow regions for faster 
> ParallelGC full GCs".
> 
> PSParallelCompact::decrement_destination_counts assumes that 
> shadow_region() is valid if mark_copied() succeeded (i.e. _shadow_state 
> was FilledShadow before). MoveAndUpdateShadowClosure::complete_region 
> sets these with the following code:
> 
>    // Record the shadow region index
>    region_ptr->set_shadow_region(_shadow);
>    // Mark the shadow region as filled to indicate the data is ready to be
>    // copied back
>    region_ptr->mark_filled();
> 
> set_shadow_region() does an ordinary store to a volatile variable and 
> mark_filled() uses a CAS to set _shadow_state to FilledShadow with 
> relaxed memory ordering. On a non-TSO architecture like AArch64 these 
> two stores may be reordered when observed from another thread.
> 
> Fix by making the CAS to set _shadow_state use the default conservative 
> memory order which inserts a full barrier (see the discussion on the JBS 
> entry).
I agree with the conclusions in the JBS entry and this fix looks good,
StefanJ

> 
> 
> Thanks,
> Nick