A bit more CallFrame cleanup.

We don't have a constructor for CallFrame so it's not always clear which fields are initialized. In this case, VM::call_frame_ is NULL when the VM instance is created, so calling VM::push_call_frame sets previous to NULL when pushing the new frame. This is fine, but brittle. Ultimately, CallFrame needs to be split into separate frame types for managed code, FFI, JIT, and native methods (ie C-API), and constructors should be defined for every type.
rubinius · Apr 1, 2016 · 46933dd · 46933dd
1 parent f05b27d
commit 46933dd
Showing 6 changed files with 22 additions and 26 deletions.
diff --git a/machine/builtin/block_environment.cpp b/machine/builtin/block_environment.cpp
@@ -435,6 +435,7 @@ namespace rubinius {
 
     call_frame->constant_scope_ = invocation.constant_scope;
 
+    call_frame->previous = NULL;
     call_frame->arguments = &args;
     call_frame->dispatch_data = env;
     call_frame->compiled_code = env->compiled_code();

diff --git a/machine/builtin/native_method.cpp b/machine/builtin/native_method.cpp
@@ -661,6 +661,7 @@ namespace rubinius {
     NativeMethodFrame nmf(env, env->current_native_frame(), nm);
     CallFrame* previous_frame = 0;
     CallFrame* call_frame = ALLOCA_CALL_FRAME(0);
+    call_frame->previous = NULL;
     call_frame->constant_scope_ = 0;
     call_frame->dispatch_data = (void*)&nmf;
     call_frame->compiled_code = 0;

diff --git a/machine/capi/thread.cpp b/machine/capi/thread.cpp
@@ -249,23 +249,21 @@ extern "C" {
     thread->locals_remove(state, state->symbol("argument"));
 
     NativeMethodFrame nmf(env, 0, nm);
-    CallFrame* previous_frame = 0;
-    CallFrame cf;
-    cf.constant_scope_ = 0;
-    cf.dispatch_data = (void*)&nmf;
-    cf.compiled_code = 0;
-    cf.flags = CallFrame::cNativeMethod;
-    cf.optional_jit_data = 0;
-    cf.top_scope_ = 0;
-    cf.scope = 0;
-    cf.arguments = 0;
-
-    CallFrame* saved_frame = env->current_call_frame();
-    env->set_current_call_frame(&cf);
+    CallFrame call_frame;
+    call_frame.previous = NULL;
+    call_frame.constant_scope_ = 0;
+    call_frame.dispatch_data = (void*)&nmf;
+    call_frame.compiled_code = 0;
+    call_frame.flags = CallFrame::cNativeMethod;
+    call_frame.optional_jit_data = 0;
+    call_frame.top_scope_ = 0;
+    call_frame.scope = 0;
+    call_frame.arguments = 0;
+
+    env->set_current_call_frame(&call_frame);
     env->set_current_native_frame(&nmf);
 
-    // Register the CallFrame, because we might GC below this.
-    state->vm()->push_call_frame(&cf, previous_frame);
+    state->vm()->set_call_frame(&call_frame);
 
     nmf.setup(
         env->get_handle(thread),
@@ -293,14 +291,10 @@ extern "C" {
 
     LEAVE_CAPI(state);
 
-    state->vm()->pop_call_frame(previous_frame);
-
-    env->set_current_call_frame(saved_frame);
-    env->set_current_native_frame(nmf.previous());
+    env->set_current_call_frame(NULL);
+    env->set_current_native_frame(NULL);
     ep.pop(env);
 
-    state->vm()->thread.get()->alive(state, cFalse);
-
     return value;
   }
 
@@ -324,11 +318,6 @@ extern "C" {
 
     VALUE thr_handle = env->get_handle(thr);
     thr->fork(state);
-    // We do a lock and unlock here so we wait until the started
-    // thread is actually ready. This is to prevent we GC stuff on
-    // the stack in the C-API caller that might be stuffed in the void* argument
-    thr->hard_lock(state);
-    thr->hard_unlock(state);
 
     return thr_handle;
   }

diff --git a/machine/jit/llvm/util.cpp b/machine/jit/llvm/util.cpp
@@ -128,6 +128,7 @@ extern "C" {
 
     call_frame->prepare(mcode->stack_size);
 
+    call_frame->previous = NULL;
     call_frame->constant_scope_ = code->scope();
     call_frame->dispatch_data = NULL;
     call_frame->compiled_code = code;
@@ -166,6 +167,7 @@ extern "C" {
 
     call_frame->constant_scope_ = invocation->constant_scope;
 
+    call_frame->previous = NULL;
     call_frame->arguments = args;
     call_frame->dispatch_data = env;
     call_frame->compiled_code = env->compiled_code();

diff --git a/machine/machine_code.cpp b/machine/machine_code.cpp
@@ -748,6 +748,7 @@ namespace rubinius {
 
       call_frame->prepare(mcode->stack_size);
 
+      call_frame->previous = NULL;
       call_frame->constant_scope_ = code->scope();
       call_frame->dispatch_data = NULL;
       call_frame->compiled_code = code;
@@ -831,6 +832,7 @@ namespace rubinius {
 
     Arguments args(state->symbol("__script__"), G(main), cNil, 0, 0);
 
+    call_frame->previous = NULL;
     call_frame->constant_scope_ = code->scope();
     call_frame->dispatch_data = 0;
     call_frame->compiled_code = code;

diff --git a/machine/memory/finalizer.cpp b/machine/memory/finalizer.cpp
@@ -199,6 +199,7 @@ namespace memory {
           CallFrame* previous_frame = 0;
           CallFrame* call_frame = ALLOCA_CALL_FRAME(0);
 
+          call_frame->previous = NULL;
           call_frame->constant_scope_ = 0;
           call_frame->dispatch_data = (void*)&nmf;
           call_frame->compiled_code = 0;