Skip to content

Commit

Permalink
Don't panic if forwarding word is not FORWARDED and add debug asserti…
Browse files Browse the repository at this point in the history
…ons (#580)

* Don't panic if forwarding word is not FORWARDED and add debug assertions

Some policies (such as Immix) can leave objects inplace and can reset
the forwarding word while tracing (such as when Immix is out of space in
its copy allocators). In such a case, we simply want to return the
current object instead of attempting to read the forwarding pointer.
This commit removes our faulty assumption and assertion and adds further
debug assertions for a sanity check.

Closes #579
  • Loading branch information
k-sareen authored May 5, 2022
1 parent ce82a7d commit 3dbdd7a
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 6 deletions.
35 changes: 34 additions & 1 deletion src/policy/immix/immixspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,11 +391,44 @@ impl<VM: VMBinding> ImmixSpace<VM> {
debug_assert!(!super::BLOCK_ONLY);
let forwarding_status = ForwardingWord::attempt_to_forward::<VM>(object);
if ForwardingWord::state_is_forwarded_or_being_forwarded(forwarding_status) {
ForwardingWord::spin_and_get_forwarded_object::<VM>(object, forwarding_status)
// We lost the forwarding race as some other thread has set the forwarding word; wait
// until the object has been forwarded by the winner. Note that the object may not
// necessarily get forwarded since Immix opportunistically moves objects.
#[allow(clippy::let_and_return)]
let new_object =
ForwardingWord::spin_and_get_forwarded_object::<VM>(object, forwarding_status);
#[cfg(debug_assertions)]
{
if new_object == object {
debug_assert!(
self.is_marked(object, self.mark_state) || self.defrag.space_exhausted() || Self::is_pinned(object),
"Forwarded object is the same as original object {} even though it should have been copied",
object,
);
} else {
// new_object != object
debug_assert!(
!Block::containing::<VM>(new_object).is_defrag_source(),
"Block {:?} containing forwarded object {} should not be a defragmentation source",
Block::containing::<VM>(new_object),
new_object,
);
}
}
new_object
} else if self.is_marked(object, self.mark_state) {
// We won the forwarding race but the object is already marked so we clear the
// forwarding status and return the unmoved object
debug_assert!(
self.defrag.space_exhausted() || Self::is_pinned(object),
"Forwarded object is the same as original object {} even though it should have been copied",
object,
);
ForwardingWord::clear_forwarding_bits::<VM>(object);
object
} else {
// We won the forwarding race; actually forward and copy the object if it is not pinned
// and we have sufficient space in our copy allocator
let new_object = if Self::is_pinned(object) || self.defrag.space_exhausted() {
self.attempt_mark(object, self.mark_state);
ForwardingWord::clear_forwarding_bits::<VM>(object);
Expand Down
14 changes: 9 additions & 5 deletions src/util/object_forwarding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,16 @@ pub fn spin_and_get_forwarded_object<VM: VMBinding>(
if forwarding_bits == FORWARDED {
read_forwarding_pointer::<VM>(object)
} else {
panic!(
"Invalid forwarding state 0x{:x} 0x{:x} for object {}",
// For some policies (such as Immix), we can have interleaving such that one thread clears
// the forwarding word while another thread was stuck spinning in the above loop.
// See: https://github.com/mmtk/mmtk-core/issues/579
debug_assert!(
forwarding_bits == FORWARDING_NOT_TRIGGERED_YET,
"Invalid/Corrupted forwarding word {:x} for object {}",
forwarding_bits,
read_forwarding_pointer::<VM>(object),
object
)
object,
);
object
}
}

Expand Down

0 comments on commit 3dbdd7a

Please sign in to comment.