RFR: Remove racy assert in ShenandoahResetNextMarkBitmapTraversalTask

[Roman Kennke]

> On 09/11/2018 03:27 PM, Zhengyu Gu wrote:
>> On 09/11/2018 08:06 AM, Roman Kennke wrote:
>>> The assert in ShenandoahResetNextMarkBitmapTraversalTask that is racily
>>> checking TAMS against top. Racy because other manipulations (e.g.
>>> recycle-assist) happen under HeapLock, but not this check. This
>>> sometimes blows up. I don't know how to fix that except removing it. We
>>> could acquire the HeapLock under #ifdef ASSERT but that seems like overkill.
>>>
>>>
>> Hmmm... I am not sure this ShenandoahResetNextBitmapTraversalTask is safe at all.
>>
>> The region status races against mutator's allocation, e.g. region->is_trash() can race against
>> FreeSet's recycle_trash(), now you have memory ordering issues to worry about. Am I wrong?
> 
> I am with Zhengyu here: this does not look safe at all.
> 
> Maybe we should instead swap the cleanup order in ShenandoahHeap::op_cleanup_traversal(): first
> recycle all the trash, and *then* reset the bitmaps. Then we don't have to test for
> region->is_trash() in ShResetNextBitmapTraversal at all (we should still assert it), resolving the
> original race.
> 

Right. The following makes both the order in cleanup and the
reset-next-markbitmap loop consistent with adaptive:

Incremental:
http://cr.openjdk.java.net/~rkennke/fixtamsassert/webrev.01.diff/
Full:
http://cr.openjdk.java.net/~rkennke/fixtamsassert/webrev.01/

Testing: passes tier3_gc_shenandoah

Roman