RFR: 8244551: Shenandoah: Fix racy update of update_watermark

Roman Kennke rkennke at redhat.com
Thu May 7 11:37:14 UTC 2020


>>> *) What's the point of this cast?
>>>
>>>  133   *const_cast<HeapWord**>(&_update_watermark) =  w;
>>
>> Make access non-volatile. You think it's too much?
> 
> I don't think that is relevant. As long as it is not the atomic store (with fences), we are fine.

Ok. I will push the fix with this additional change then:

diff -r 7910475d5001
src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.inline.hpp
--- a/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.inline.hpp
Thu May 07 07:03:25 2020 -0400
+++ b/src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.inline.hpp
Thu May 07 07:36:26 2020 -0400
@@ -128,9 +128,8 @@
 // Fast version that avoids synchronization, only to be used at safepoints.
 inline void
ShenandoahHeapRegion::set_update_watermark_at_safepoint(HeapWord* w) {
   assert(bottom() <= w && w <= top(), "within bounds");
-  assert(SafepointSynchronize::is_at_safepoint(),
-         "only call set_update_watermark_at_safepoint at safepoint");
-  *const_cast<HeapWord**>(&_update_watermark) =  w;
+  assert(SafepointSynchronize::is_at_safepoint(), "Should be at
Shenandoah safepoint");
+  _update_watermark = w;
 }

 #endif // SHARE_GC_SHENANDOAH_SHENANDOAHHEAPREGION_INLINE_HPP


Thanks for reviewing!
Roman



More information about the shenandoah-dev mailing list