Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Rework SpinLock to use test_and_set and test_and_clear primitives

This uses the GCC atomic built-ins which are also described here
http://gcc.gnu.org/onlinedocs/gcc-4.1.2/gcc/Atomic-Builtins.html for
atomic test and set and test and clear. It has the right memory
barrier semantics for the memory barrier we need for it.

It also reworks some places where we should use a SpinLock instead
of the primitive operations.

This fixes hangs such as the following:

https://gist.github.com/511305b6e435ef7d7d1c

Here it keeps hanging in the SpinLock forever because it doesn't
properly see the SpinLock being unlocked somewhere else.
  • Loading branch information...
commit 00c6f5904b489a0449b9bc2264dfe70da543cb73 1 parent 0c5df78
@dbussink dbussink authored
View
14 vm/builtin/regexp.cpp
@@ -126,7 +126,7 @@ namespace rubinius {
o_reg->onig_data = NULL;
o_reg->forced_encoding_ = false;
- o_reg->lock_ = 0;
+ o_reg->lock_.init();
return o_reg;
}
@@ -398,7 +398,7 @@ namespace rubinius {
return Qnil;
}
- while(!atomic::compare_and_swap(&lock_, 0, 1));
+ lock_.lock();
maybe_recompile(state);
@@ -444,7 +444,7 @@ namespace rubinius {
write_barrier(state, ba);
}
- lock_ = 0;
+ lock_.unlock();
if(beg == ONIG_MISMATCH) {
onig_region_free(&region, 0);
@@ -478,7 +478,7 @@ namespace rubinius {
if(pos > max) return Qnil;
str += pos;
- while(!atomic::compare_and_swap(&lock_, 0, 1));
+ lock_.lock();
maybe_recompile(state);
@@ -515,7 +515,7 @@ namespace rubinius {
write_barrier(state, ba);
}
- lock_ = 0;
+ lock_.unlock();
if(beg != ONIG_MISMATCH) {
md = get_match_data(state, &region, string, this, pos);
@@ -535,7 +535,7 @@ namespace rubinius {
Exception::argument_error(state, "Not properly initialized Regexp");
}
- while(!atomic::compare_and_swap(&lock_, 0, 1));
+ lock_.lock();
maybe_recompile(state);
@@ -580,7 +580,7 @@ namespace rubinius {
write_barrier(state, ba);
}
- lock_ = 0;
+ lock_.unlock();
if(beg != ONIG_MISMATCH) {
md = get_match_data(state, &region, string, this, pos);
View
2  vm/builtin/regexp.hpp
@@ -27,7 +27,7 @@ namespace rubinius {
LookupTable* names_; // slot
regex_t* onig_data;
bool forced_encoding_;
- uint32_t lock_;
+ thread::SpinLock lock_;
public:
/* accessors */
View
2  vm/builtin/system.cpp
@@ -48,6 +48,7 @@
#include "builtin/float.hpp"
#include "builtin/methodtable.hpp"
#include "builtin/io.hpp"
+#include "builtin/thread.hpp"
#include "builtin/staticscope.hpp"
#include "builtin/block_environment.hpp"
@@ -454,6 +455,7 @@ namespace rubinius {
if(result == 0) {
/* @todo any other re-initialisation needed? */
+ state->vm()->thread->init_lock();
state->shared().reinit(state);
state->shared().om->on_fork(state);
SignalHandler::on_fork(state, false);
View
7 vm/builtin/thread.cpp
@@ -69,11 +69,10 @@ namespace rubinius {
thr->vm_ = target;
thr->klass(state, as<Class>(self));
thr->runner_ = runner;
+ thr->init_lock_.init();
target->thread.set(thr);
- new(&thr->init_lock_) thread::SpinLock();
-
if(!main_thread) thr->init_lock_.lock();
return thr;
@@ -263,6 +262,10 @@ namespace rubinius {
vm_ = NULL;
}
+ void Thread::init_lock() {
+ init_lock_.init();
+ }
+
Object* Thread::join(STATE, CallFrame* calling_environment) {
state->set_call_frame(calling_environment);
View
1  vm/builtin/thread.hpp
@@ -176,6 +176,7 @@ namespace rubinius {
// Rubinius.primitive :thread_unlock_locks
Object* unlock_locks(STATE, GCToken gct);
+ void init_lock();
void cleanup();
/**
View
7 vm/inline_cache.cpp
@@ -465,7 +465,6 @@ namespace rubinius {
// github#157
if(!cache->fill_method_missing(state, recv_class, mce)) {
Exception::internal_error(state, call_frame, "no method_missing");
- cache->private_lock_ = 0;
return 0;
}
@@ -604,13 +603,13 @@ namespace rubinius {
// potentially room for registering another class,
// so we should lock it here before we're allowed to write it.
- while(!atomic::compare_and_swap(&private_lock_, 0, 1));
+ private_lock_.lock();
for(int i = 0; i < cTrackedICHits; i++) {
if(!seen_classes_[i].klass()) {
// An empty space, record it.
seen_classes_[i].assign(mce->receiver_class());
- private_lock_ = 0;
+ private_lock_.unlock();
return;
}
}
@@ -619,7 +618,7 @@ namespace rubinius {
// Hmmm, what do we do when this is full? Just ignore them?
// For now, just keep track of how many times we overflow.
seen_classes_overflow_++;
- private_lock_ = 0;
+ private_lock_.unlock();
}
void InlineCache::print_location(STATE, std::ostream& stream) {
View
3  vm/inline_cache.hpp
@@ -69,7 +69,7 @@ namespace rubinius {
int seen_classes_overflow_;
InlineCacheHit seen_classes_[cTrackedICHits];
- int private_lock_;
+ thread::SpinLock private_lock_;
public:
@@ -131,7 +131,6 @@ namespace rubinius {
, initial_backend_(empty_cache)
, execute_backend_(empty_cache)
, seen_classes_overflow_(0)
- , private_lock_(0)
{}
#ifdef TRACK_IC_LOCATION
View
30 vm/util/atomic.hpp
@@ -48,6 +48,8 @@
namespace atomic {
+ typedef volatile int atomic_int_t;
+
inline void memory_barrier() {
#if defined(GCC_BARRIER)
__sync_synchronize();
@@ -171,6 +173,34 @@ namespace atomic {
return val;
#endif
}
+
+ template <typename intish>
+ inline intish test_and_set(intish *ptr) {
+#if defined(GCC_SYNC)
+ return __sync_lock_test_and_set(ptr, 1);
+#elif defined(APPLE_SYNC)
+ return OSAtomicTestAndSetBarrier(0, (volatile void*)ptr);
+#elif defined(X86_SYNC)
+ return !compare_and_swap((uint32_t*)ptr, 0, 1);
+#else
+#error "no sync primitive found"
+#endif
+ }
+
+ template <typename intish>
+ inline void test_and_clear(intish *ptr) {
+#if defined(GCC_SYNC)
+ __sync_lock_release(ptr);
+#elif defined(APPLE_SYNC)
+ OSAtomicTestAndClearBarrier(0, (volatile void*)ptr);
+#elif defined(X86_SYNC)
+ memory_barrier();
+ *ptr = 0;
+#else
+#error "no sync primitive found"
+#endif
+ }
+
}
#include "util/atomic_types.hpp"
View
18 vm/util/thread.hpp
@@ -509,6 +509,10 @@ namespace thread {
: native_(0)
{}
+ void init() {
+ native_ = 0;
+ }
+
void lock() {
OSSpinLockLock(&native_);
}
@@ -543,23 +547,27 @@ namespace thread {
typedef StackUnlockGuard<SpinLock> UnlockGuard;
private:
- int lock_;
+ atomic::atomic_int_t lock_;
public:
SpinLock()
- : lock_(1)
+ : lock_(0)
{}
+ void init() {
+ lock_ = 0;
+ }
+
void lock() {
- while(!atomic::compare_and_swap(&lock_, 1, 0));
+ while(atomic::test_and_set(&lock_));
}
void unlock() {
- lock_ = 1;
+ atomic::test_and_clear(&lock_);
}
Code try_lock() {
- if(atomic::compare_and_swap(&lock_, 1, 0)) {
+ if(!atomic::test_and_set(&lock_)) {
return cLocked;
}
Please sign in to comment.
Something went wrong with that request. Please try again.