mirror of
https://github.com/openjdk/jdk.git
synced 2025-08-26 22:34:27 +02:00
6722113: CMS: Incorrect overflow handling during precleaning of Reference lists
When we encounter marking stack overflow during precleaning of Reference lists, we were using the overflow list mechanism, which can cause problems on account of mutating the mark word of the header because of conflicts with mutator accesses and updates of that field. Instead we should use the usual mechanism for overflow handling in concurrent phases, namely dirtying of the card on which the overflowed object lies. Since precleaning effectively does a form of discovered list processing, albeit with discovery enabled, we needed to adjust some code to be correct in the face of interleaved processing and discovery. Reviewed-by: apetrusenko, jcoomes
This commit is contained in:
parent
28b2c4aeaf
commit
db6bef2c70
5 changed files with 95 additions and 48 deletions
|
@ -47,7 +47,9 @@ public:
|
|||
}
|
||||
bool empty() const { return head() == ReferenceProcessor::sentinel_ref(); }
|
||||
size_t length() { return _len; }
|
||||
void set_length(size_t len) { _len = len; }
|
||||
void set_length(size_t len) { _len = len; }
|
||||
void inc_length(size_t inc) { _len += inc; assert(_len > 0, "Error"); }
|
||||
void dec_length(size_t dec) { _len -= dec; }
|
||||
private:
|
||||
// Set value depending on UseCompressedOops. This could be a template class
|
||||
// but then we have to fix all the instantiations and declarations that use this class.
|
||||
|
@ -436,13 +438,13 @@ public:
|
|||
// The "allow_null_referent" argument tells us to allow for the possibility
|
||||
// of a NULL referent in the discovered Reference object. This typically
|
||||
// happens in the case of concurrent collectors that may have done the
|
||||
// discovery concurrently or interleaved with mutator execution.
|
||||
// discovery concurrently, or interleaved, with mutator execution.
|
||||
inline void load_ptrs(DEBUG_ONLY(bool allow_null_referent));
|
||||
|
||||
// Move to the next discovered reference.
|
||||
inline void next();
|
||||
|
||||
// Remove the current reference from the list and move to the next.
|
||||
// Remove the current reference from the list
|
||||
inline void remove();
|
||||
|
||||
// Make the Reference object active again.
|
||||
|
@ -476,7 +478,6 @@ public:
|
|||
inline size_t removed() const { return _removed; }
|
||||
)
|
||||
|
||||
private:
|
||||
inline void move_to_next();
|
||||
|
||||
private:
|
||||
|
@ -553,7 +554,7 @@ inline void DiscoveredListIterator::remove() {
|
|||
oopDesc::store_heap_oop((oop*)_prev_next, _next);
|
||||
}
|
||||
NOT_PRODUCT(_removed++);
|
||||
move_to_next();
|
||||
_refs_list.dec_length(1);
|
||||
}
|
||||
|
||||
inline void DiscoveredListIterator::move_to_next() {
|
||||
|
@ -591,12 +592,13 @@ ReferenceProcessor::process_phase1(DiscoveredList& refs_list,
|
|||
gclog_or_tty->print_cr("Dropping reference (" INTPTR_FORMAT ": %s" ") by policy",
|
||||
iter.obj(), iter.obj()->blueprint()->internal_name());
|
||||
}
|
||||
// Remove Reference object from list
|
||||
iter.remove();
|
||||
// Make the Reference object active again
|
||||
iter.make_active();
|
||||
// keep the referent around
|
||||
iter.make_referent_alive();
|
||||
// Remove Reference object from list
|
||||
iter.remove();
|
||||
iter.move_to_next();
|
||||
} else {
|
||||
iter.next();
|
||||
}
|
||||
|
@ -629,12 +631,13 @@ ReferenceProcessor::pp2_work(DiscoveredList& refs_list,
|
|||
iter.obj(), iter.obj()->blueprint()->internal_name());
|
||||
}
|
||||
// The referent is reachable after all.
|
||||
// Remove Reference object from list.
|
||||
iter.remove();
|
||||
// Update the referent pointer as necessary: Note that this
|
||||
// should not entail any recursive marking because the
|
||||
// referent must already have been traversed.
|
||||
iter.make_referent_alive();
|
||||
// Remove Reference object from list
|
||||
iter.remove();
|
||||
iter.move_to_next();
|
||||
} else {
|
||||
iter.next();
|
||||
}
|
||||
|
@ -670,6 +673,7 @@ ReferenceProcessor::pp2_work_concurrent_discovery(DiscoveredList& refs_list,
|
|||
} else {
|
||||
keep_alive->do_oop((oop*)next_addr);
|
||||
}
|
||||
iter.move_to_next();
|
||||
} else {
|
||||
iter.next();
|
||||
}
|
||||
|
@ -832,9 +836,9 @@ void ReferenceProcessor::balance_queues(DiscoveredList ref_lists[])
|
|||
}
|
||||
java_lang_ref_Reference::set_discovered(move_tail, ref_lists[to_idx].head());
|
||||
ref_lists[to_idx].set_head(move_head);
|
||||
ref_lists[to_idx].set_length(ref_lists[to_idx].length() + refs_to_move);
|
||||
ref_lists[to_idx].inc_length(refs_to_move);
|
||||
ref_lists[from_idx].set_head(new_head);
|
||||
ref_lists[from_idx].set_length(ref_lists[from_idx].length() - refs_to_move);
|
||||
ref_lists[from_idx].dec_length(refs_to_move);
|
||||
} else {
|
||||
++to_idx;
|
||||
}
|
||||
|
@ -923,7 +927,6 @@ void ReferenceProcessor::clean_up_discovered_references() {
|
|||
void ReferenceProcessor::clean_up_discovered_reflist(DiscoveredList& refs_list) {
|
||||
assert(!discovery_is_atomic(), "Else why call this method?");
|
||||
DiscoveredListIterator iter(refs_list, NULL, NULL);
|
||||
size_t length = refs_list.length();
|
||||
while (iter.has_next()) {
|
||||
iter.load_ptrs(DEBUG_ONLY(true /* allow_null_referent */));
|
||||
oop next = java_lang_ref_Reference::next(iter.obj());
|
||||
|
@ -941,12 +944,11 @@ void ReferenceProcessor::clean_up_discovered_reflist(DiscoveredList& refs_list)
|
|||
)
|
||||
// Remove Reference object from list
|
||||
iter.remove();
|
||||
--length;
|
||||
iter.move_to_next();
|
||||
} else {
|
||||
iter.next();
|
||||
}
|
||||
}
|
||||
refs_list.set_length(length);
|
||||
NOT_PRODUCT(
|
||||
if (PrintGCDetails && TraceReferenceGC) {
|
||||
gclog_or_tty->print(
|
||||
|
@ -1024,7 +1026,7 @@ ReferenceProcessor::add_to_discovered_list_mt(DiscoveredList& refs_list,
|
|||
// We have separate lists for enqueueing so no synchronization
|
||||
// is necessary.
|
||||
refs_list.set_head(obj);
|
||||
refs_list.set_length(refs_list.length() + 1);
|
||||
refs_list.inc_length(1);
|
||||
if (_discovered_list_needs_barrier) {
|
||||
_bs->write_ref_field((void*)discovered_addr, current_head); guarantee(false, "Needs to be fixed: YSR");
|
||||
}
|
||||
|
@ -1168,7 +1170,7 @@ bool ReferenceProcessor::discover_reference(oop obj, ReferenceType rt) {
|
|||
_bs->write_ref_field((oop*)discovered_addr, current_head);
|
||||
}
|
||||
list->set_head(obj);
|
||||
list->set_length(list->length() + 1);
|
||||
list->inc_length(1);
|
||||
}
|
||||
|
||||
// In the MT discovery case, it is currently possible to see
|
||||
|
@ -1209,45 +1211,48 @@ void ReferenceProcessor::preclean_discovered_references(
|
|||
TraceTime tt("Preclean SoftReferences", PrintGCDetails && PrintReferenceGC,
|
||||
false, gclog_or_tty);
|
||||
for (int i = 0; i < _num_q; i++) {
|
||||
if (yield->should_return()) {
|
||||
return;
|
||||
}
|
||||
preclean_discovered_reflist(_discoveredSoftRefs[i], is_alive,
|
||||
keep_alive, complete_gc, yield);
|
||||
}
|
||||
}
|
||||
if (yield->should_return()) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Weak references
|
||||
{
|
||||
TraceTime tt("Preclean WeakReferences", PrintGCDetails && PrintReferenceGC,
|
||||
false, gclog_or_tty);
|
||||
for (int i = 0; i < _num_q; i++) {
|
||||
if (yield->should_return()) {
|
||||
return;
|
||||
}
|
||||
preclean_discovered_reflist(_discoveredWeakRefs[i], is_alive,
|
||||
keep_alive, complete_gc, yield);
|
||||
}
|
||||
}
|
||||
if (yield->should_return()) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Final references
|
||||
{
|
||||
TraceTime tt("Preclean FinalReferences", PrintGCDetails && PrintReferenceGC,
|
||||
false, gclog_or_tty);
|
||||
for (int i = 0; i < _num_q; i++) {
|
||||
if (yield->should_return()) {
|
||||
return;
|
||||
}
|
||||
preclean_discovered_reflist(_discoveredFinalRefs[i], is_alive,
|
||||
keep_alive, complete_gc, yield);
|
||||
}
|
||||
}
|
||||
if (yield->should_return()) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Phantom references
|
||||
{
|
||||
TraceTime tt("Preclean PhantomReferences", PrintGCDetails && PrintReferenceGC,
|
||||
false, gclog_or_tty);
|
||||
for (int i = 0; i < _num_q; i++) {
|
||||
if (yield->should_return()) {
|
||||
return;
|
||||
}
|
||||
preclean_discovered_reflist(_discoveredPhantomRefs[i], is_alive,
|
||||
keep_alive, complete_gc, yield);
|
||||
}
|
||||
|
@ -1256,9 +1261,12 @@ void ReferenceProcessor::preclean_discovered_references(
|
|||
|
||||
// Walk the given discovered ref list, and remove all reference objects
|
||||
// whose referents are still alive, whose referents are NULL or which
|
||||
// are not active (have a non-NULL next field). NOTE: For this to work
|
||||
// correctly, refs discovery can not be happening concurrently with this
|
||||
// step.
|
||||
// are not active (have a non-NULL next field). NOTE: When we are
|
||||
// thus precleaning the ref lists (which happens single-threaded today),
|
||||
// we do not disable refs discovery to honour the correct semantics of
|
||||
// java.lang.Reference. As a result, we need to be careful below
|
||||
// that ref removal steps interleave safely with ref discovery steps
|
||||
// (in this thread).
|
||||
void
|
||||
ReferenceProcessor::preclean_discovered_reflist(DiscoveredList& refs_list,
|
||||
BoolObjectClosure* is_alive,
|
||||
|
@ -1266,7 +1274,6 @@ ReferenceProcessor::preclean_discovered_reflist(DiscoveredList& refs_list,
|
|||
VoidClosure* complete_gc,
|
||||
YieldClosure* yield) {
|
||||
DiscoveredListIterator iter(refs_list, keep_alive, is_alive);
|
||||
size_t length = refs_list.length();
|
||||
while (iter.has_next()) {
|
||||
iter.load_ptrs(DEBUG_ONLY(true /* allow_null_referent */));
|
||||
oop obj = iter.obj();
|
||||
|
@ -1281,7 +1288,6 @@ ReferenceProcessor::preclean_discovered_reflist(DiscoveredList& refs_list,
|
|||
}
|
||||
// Remove Reference object from list
|
||||
iter.remove();
|
||||
--length;
|
||||
// Keep alive its cohort.
|
||||
iter.make_referent_alive();
|
||||
if (UseCompressedOops) {
|
||||
|
@ -1291,12 +1297,11 @@ ReferenceProcessor::preclean_discovered_reflist(DiscoveredList& refs_list,
|
|||
oop* next_addr = (oop*)java_lang_ref_Reference::next_addr(obj);
|
||||
keep_alive->do_oop(next_addr);
|
||||
}
|
||||
iter.move_to_next();
|
||||
} else {
|
||||
iter.next();
|
||||
}
|
||||
}
|
||||
refs_list.set_length(length);
|
||||
|
||||
// Close the reachable set
|
||||
complete_gc->do_void();
|
||||
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue