Skip to content
Permalink
Browse files
8244551: Shenandoah: Fix racy update of update_watermark
Reviewed-by: shade
  • Loading branch information
rkennke committed May 7, 2020
1 parent b231ad7 commit 62bf2d07e7864cff2c8759acfb17580725cdfd0a
@@ -1484,7 +1484,7 @@ class ShenandoahFinalMarkUpdateRegionStateClosure : public ShenandoahHeapRegionC

// Remember limit for updating refs. It's guaranteed that we get no
// from-space-refs written from here on.
r->set_update_watermark(r->top());
r->set_update_watermark_at_safepoint(r->top());
} else {
assert(!r->has_live(), "Region " SIZE_FORMAT " should have no live data", r->index());
assert(_ctx->top_at_mark_start(r) == r->top(),
@@ -243,7 +243,7 @@ class ShenandoahHeapRegion {
volatile size_t _live_data;
volatile size_t _critical_pins;

HeapWord* _update_watermark;
HeapWord* volatile _update_watermark;

public:
ShenandoahHeapRegion(HeapWord* start, size_t index, bool committed);
@@ -382,19 +382,9 @@ class ShenandoahHeapRegion {
size_t get_tlab_allocs() const;
size_t get_gclab_allocs() const;

HeapWord* get_update_watermark() const {
// Updates to the update-watermark only happen at safepoints.
// Since those updates are only monotonically increasing, possibly reading
// a stale value is only conservative - we would not miss to update any fields.
HeapWord* watermark = _update_watermark;
assert(bottom() <= watermark && watermark <= top(), "within bounds");
return watermark;
}

void set_update_watermark(HeapWord* w) {
assert(bottom() <= w && w <= top(), "within bounds");
_update_watermark = w;
}
inline HeapWord* get_update_watermark() const;
inline void set_update_watermark(HeapWord* w);
inline void set_update_watermark_at_safepoint(HeapWord* w);

private:
void do_commit();
@@ -114,4 +114,22 @@ inline size_t ShenandoahHeapRegion::garbage() const {
return result;
}

inline HeapWord* ShenandoahHeapRegion::get_update_watermark() const {
HeapWord* watermark = Atomic::load_acquire(&_update_watermark);
assert(bottom() <= watermark && watermark <= top(), "within bounds");
return watermark;
}

inline void ShenandoahHeapRegion::set_update_watermark(HeapWord* w) {
assert(bottom() <= w && w <= top(), "within bounds");
Atomic::release_store(&_update_watermark, w);
}

// 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(), "Should be at Shenandoah safepoint");
_update_watermark = w;
}

#endif // SHARE_GC_SHENANDOAH_SHENANDOAHHEAPREGION_INLINE_HPP

0 comments on commit 62bf2d0

Please sign in to comment.