Browse files

Properly mark things as on stack in variable scope

Because of the reordering (and because there were lingering bugs here in
the first place), we need to guard places properly with on stack guards
when allocation might happen.

This is also in the spin loop for getting and setting a local, because a
heap tuple might be allocated that could have caused a gc. We put the
guard inside the loop though, so we don't have to setup the on stack
guard for the common case of not having to spin loop here.
  • Loading branch information...
1 parent dcf6d75 commit e98194962aadcf14cd28c4dfe2193f2d063c7450 @dbussink dbussink committed Mar 25, 2013
Showing with 40 additions and 15 deletions.
  1. +29 −8 vm/builtin/variable_scope.cpp
  2. +1 −3 vm/builtin/variable_scope.hpp
  3. +10 −4 vm/stack_variables.cpp
View
37 vm/builtin/variable_scope.cpp
@@ -13,6 +13,7 @@
#include "fiber_data.hpp"
#include "ontology.hpp"
+#include "on_stack.hpp"
namespace rubinius {
void VariableScope::init(STATE) {
@@ -72,8 +73,11 @@ namespace rubinius {
Tuple* VariableScope::locals(STATE) {
Tuple* tup = Tuple::create(state, number_of_locals_);
+ VariableScope* self = this;
+ OnStack<1> os(state, self);
+
for(int i = 0; i < number_of_locals_; i++) {
- tup->put(state, i, get_local(state, i));
+ tup->put(state, i, self->get_local(state, i));
}
return tup;
@@ -99,8 +103,12 @@ namespace rubinius {
void VariableScope::set_local(STATE, int pos, Object* val) {
if(isolated_) {
- while(heap_locals_->nil_p()) atomic::pause();
- heap_locals_->put(state, pos, val);
+ VariableScope* self = this;
+ while(heap_locals_->nil_p()) {
+ OnStack<2> os(state, self, val);
+ atomic::pause();
+ }
+ self->heap_locals_->put(state, pos, val);
} else {
set_local(pos, val);
}
@@ -124,11 +132,21 @@ namespace rubinius {
ary[pos] = val;
}
- Object* VariableScope::get_local(int pos) {
+ Object* VariableScope::get_local(STATE, int pos) {
if(isolated_) {
- while(heap_locals_->nil_p()) atomic::pause();
- return heap_locals_->at(pos);
+ VariableScope* self = this;
+ while(heap_locals_->nil_p()) {
+ OnStack<1> os(state, self);
+ atomic::pause();
+ }
+ return self->heap_locals_->at(pos);
+ } else {
+ return get_local(pos);
}
+ }
+
+ Object* VariableScope::get_local(int pos) {
+ assert(!isolated_);
Object** ary = locals_;
if(Fiber* fib = try_as<Fiber>(fiber_)) {
@@ -148,12 +166,15 @@ namespace rubinius {
if(isolated_) return;
if(!atomic::compare_and_swap(&isolated_, 0, 1)) return;
+ VariableScope* self = this;
+
+ OnStack<1> os(state, self);
Tuple* new_locals = Tuple::create(state, number_of_locals_);
for(int i = 0; i < number_of_locals_; i++) {
- new_locals->put(state, i, locals_[i]);
+ new_locals->put(state, i, self->locals_[i]);
}
atomic::memory_barrier();
- heap_locals(state, new_locals);
+ self->heap_locals(state, new_locals);
}
void VariableScope::Info::mark(Object* obj, ObjectMark& mark) {
View
4 vm/builtin/variable_scope.hpp
@@ -74,9 +74,7 @@ namespace rubinius {
void set_local(STATE, int pos, Object* val);
Object* get_local(int pos);
- Object* get_local(STATE, int pos) {
- return get_local(pos);
- }
+ Object* get_local(STATE, int pos);
int number_of_locals() {
return number_of_locals_;
View
14 vm/stack_variables.cpp
@@ -3,6 +3,7 @@
#include "builtin/lookuptable.hpp"
#include "machine_code.hpp"
#include "call_frame.hpp"
+#include "on_stack.hpp"
namespace rubinius {
@@ -14,6 +15,7 @@ namespace rubinius {
MachineCode* mcode = call_frame->compiled_code->machine_code();
VariableScope* scope = state->new_object_dirty<VariableScope>(G(variable_scope));
+ OnStack<1> os(state, scope);
if(parent_) {
scope->parent(state, parent_);
} else {
@@ -31,15 +33,19 @@ namespace rubinius {
scope->number_of_locals_ = mcode->number_of_locals;
scope->isolated_ = 0;
+ if(!full) {
+ scope->isolated_ = 1;
+ scope->heap_locals(state, Tuple::create(state, mcode->number_of_locals));
+ for(int i = 0; i < scope->number_of_locals_; i++) {
+ scope->set_local(state, i, locals_[i]);
+ }
+ }
+
scope->locals_ = locals_;
scope->dynamic_locals(state, nil<LookupTable>());
scope->set_block_as_method(call_frame->block_as_method_p());
- if(!full) {
- flush_to_heap(state);
- }
-
on_heap_ = scope;
return scope;

0 comments on commit e981949

Please sign in to comment.