Permalink
Browse files

Fix locking issues for contention cases

Previous fix wasn't completely correct. We need to make sure the old
header we want to replace is actually not inflated and set the original
up for that case accordingly.
  • Loading branch information...
1 parent 2336188 commit 15acea68d0a069214dde2a153b738e1872a54a21 @dbussink dbussink committed Nov 2, 2012
Showing with 22 additions and 17 deletions.
  1. +18 −15 vm/objectmemory.cpp
  2. +4 −2 vm/oop.cpp
View
@@ -179,11 +179,10 @@ namespace rubinius {
// contention condvar until the object is unlocked.
HeaderWord orig = obj->header;
- HeaderWord new_val = orig;
-
- orig.f.meaning = eAuxWordLock;
+ orig.f.inflated = 0;
- new_val.f.inflated = 0;
+ HeaderWord new_val = orig;
+ orig.f.meaning = eAuxWordLock;
new_val.f.LockContended = 1;
if(!obj->header.atomic_set(orig, new_val)) {
@@ -298,14 +297,14 @@ namespace rubinius {
int initial_count = 0;
HeaderWord orig = obj->header;
- HeaderWord tmp = orig;
+ HeaderWord new_val = orig;
- if(tmp.f.inflated) {
+ if(orig.f.inflated) {
// Already inflated. ERROR, let the caller sort it out.
return false;
}
- switch(tmp.f.meaning) {
+ switch(new_val.f.meaning) {
case eAuxWordEmpty:
// ERROR, we can not be here because it's empty. This is only to
// be called when the header is already in use.
@@ -314,11 +313,11 @@ namespace rubinius {
// We could be have made a header before trying again, so
// keep using the original one.
ih = inflated_headers_->allocate(obj);
- ih->set_object_id(tmp.f.aux_word);
+ ih->set_object_id(new_val.f.aux_word);
break;
case eAuxWordLock:
// We have to locking the object to inflate it, thats the law.
- if(tmp.f.aux_word >> cAuxLockTIDShift != state->vm()->thread_id()) {
+ if(new_val.f.aux_word >> cAuxLockTIDShift != state->vm()->thread_id()) {
return false;
}
@@ -334,10 +333,10 @@ namespace rubinius {
ih->initialize_mutex(state->vm()->thread_id(), initial_count);
- tmp.all_flags = ih;
- tmp.f.inflated = 1;
+ new_val.all_flags = ih;
+ new_val.f.inflated = 1;
- while(!obj->header.atomic_set(orig, tmp)) {
+ while(!obj->header.atomic_set(orig, new_val)) {
// The header can't have been inflated by another thread, the
// inflation process holds the OM lock.
//
@@ -347,15 +346,19 @@ namespace rubinius {
// Sanity check that the meaning is still the same, if not, then
// something is really wrong.
orig = obj->header;
- if(orig.f.meaning != tmp.f.meaning) {
+ if(orig.f.inflated) {
+ return false;
+ }
+ if(orig.f.meaning != new_val.f.meaning) {
if(cDebugThreading) {
std::cerr << "[LOCK object header consistence error detected.]" << std::endl;
}
return false;
}
- tmp.all_flags = ih;
- tmp.f.inflated = 1;
+ new_val = orig;
+ new_val.all_flags = ih;
+ new_val.f.inflated = 1;
}
return true;
View
@@ -27,11 +27,13 @@ namespace rubinius {
bool ObjectHeader::set_inflated_header(STATE, InflatedHeader* ih) {
HeaderWord orig = header;
+ if(orig.f.inflated) return false;
+
ih->reset_object(this);
if(!ih->update(state, orig)) return false;
HeaderWord new_val = orig;
- new_val.all_flags = ih;
+ new_val.all_flags = ih;
new_val.f.inflated = 1;
// Make sure to include a barrier to the header is all properly initialized
@@ -41,6 +43,7 @@ namespace rubinius {
// we catch that and keep trying until we get our version in.
while(!header.atomic_set(orig, new_val)) {
orig = header;
+ if(orig.f.inflated) return false;
if(!ih->update(state, orig)) return false;
}
@@ -437,7 +440,6 @@ namespace rubinius {
orig.f.meaning = eAuxWordHandle;
HeaderWord new_val = orig;
- new_val.f.inflated = 0;
new_val.f.meaning = eAuxWordEmpty;
new_val.f.aux_word = 0;

0 comments on commit 15acea6

Please sign in to comment.