Skip to content
Browse files

Fix thread shutdown race condition

This ensures we can tear down the roots and thread data structures
outside of using them in the GC. There was a race possible where we
would be iterating threads in the GC while an entry was removed. This
was possible because the deconstructor for VM would run outside a GC
dependent block.
  • Loading branch information...
1 parent 8bea23f commit 18c257794aeb5ff5002c2508364be501e18ae567 @dbussink dbussink committed Nov 25, 2013
Showing with 16 additions and 1 deletion.
  1. +2 −1 vm/builtin/thread.cpp
  2. +4 −0 vm/shared_state.cpp
  3. +1 −0 vm/shared_state.hpp
  4. +9 −0 vm/world_state.hpp
View
3 vm/builtin/thread.cpp
@@ -279,8 +279,8 @@ namespace rubinius {
vm->thread->cleanup();
vm->thread->init_lock_.unlock();
- vm->shared.gc_independent(state, 0);
vm->shared.clear_critical(state);
+ SharedState& shared = vm->shared;
VM::discard(state, vm);
@@ -289,6 +289,7 @@ namespace rubinius {
}
RUBINIUS_THREAD_STOP(thread_name.c_str(), vm->thread_id(), 0);
+ shared.gc_independent();
return 0;
}
View
4 vm/shared_state.cpp
@@ -229,6 +229,10 @@ namespace rubinius {
world_->become_independent(state);
}
+ void SharedState::gc_independent() {
+ world_->become_independent();
+ }
+
const unsigned int* SharedState::object_memory_mark_address() const {
return om->mark_address();
}
View
1 vm/shared_state.hpp
@@ -300,6 +300,7 @@ namespace rubinius {
void gc_dependent(THREAD, utilities::thread::Condition* = NULL);
void gc_independent(THREAD);
+ void gc_independent();
void set_critical(STATE, CallFrame* call_frame);
void clear_critical(STATE);
View
9 vm/world_state.hpp
@@ -68,6 +68,15 @@ namespace rubinius {
}
}
+ /**
+ * Called when a thread is shutting down. This is done so it needs
+ * no access to a VM structure since that is already deallocated while
+ * still being gc dependent.
+ */
+ void become_independent() {
+ atomic::fetch_and_sub(&pending_threads_, 1);
+ }
+
void become_dependent(THREAD, utilities::thread::Condition* cond = NULL) {
switch(state->run_state()) {
case ManagedThread::eAlone:

0 comments on commit 18c2577

Please sign in to comment.
Something went wrong with that request. Please try again.