fix(vm): Restore the vm to a valid state after error has occured

Probably more instances where this is wrong but I need to do a larger
refactor to catch them all

repl/tests/rexpect.rs

@@ -164,3 +164,11 @@ fn import() {
 
     repl.test("let { assert } = import! std.test", None);
 }
+
+#[test]
+fn assert() {
+    let mut repl = REPL::new();
+
+    repl.test("let { assert } = import! std.test", None);
+    repl.test("assert False", None);
+}

src/compiler_pipeline.rs

@@ -30,14 +30,14 @@ use vm::compiler::CompiledModule;
 use vm::core;
 use vm::future::{BoxFutureValue, FutureValue};
 use vm::macros::MacroExpander;
-use vm::thread::{Execute, RootedValue, Thread, ThreadInternal, VmRoot};
+use vm::thread::{Execute, ExecuteTop, RootedValue, Thread, ThreadInternal, VmRoot};
 
 use {Compiler, Error, Result};
 
-fn execute<T, F>(vm: T, f: F) -> FutureValue<Execute<T>>
+fn execute<T, F>(vm: T, f: F) -> FutureValue<ExecuteTop<T>>
 where
-    T: Deref<Target = Thread>,
-    F: for<'vm> FnOnce(&'vm Thread) -> FutureValue<Execute<&'vm Thread>>,
+    T: Deref<Target = Thread> + Clone,
+    F: for<'vm> FnOnce(&'vm Thread) -> FutureValue<ExecuteTop<&'vm Thread>>,
 {
     let opt = match f(&vm) {
         FutureValue::Value(result) => Some(result.map(|v| v.1)),
@@ -46,7 +46,7 @@ where
     };
     match opt {
         Some(result) => FutureValue::Value(result.map(|v| (vm, v))),
-        None => FutureValue::Future(Execute::new(vm)),
+        None => FutureValue::Future(ExecuteTop(Execute::new(vm))),
     }
 }
 
@@ -730,7 +730,7 @@ where
         let closure = try_future!(vm.global_env().new_global_thunk(module));
 
         let vm1 = vm.clone();
-        execute(vm1, |vm| vm.call_thunk(closure))
+        execute(vm1, |vm| vm.call_thunk_top(closure))
             .map(|(vm, value)| ExecuteValue {
                 id: module_id,
                 expr: expr,
@@ -851,7 +851,7 @@ where
         let metadata = module.metadata;
         let vm1 = vm.clone();
         let closure = try_future!(vm.global_env().new_global_thunk(module.module));
-        execute(vm1, |vm| vm.call_thunk(closure))
+        execute(vm1, |vm| vm.call_thunk_top(closure))
             .map(|(vm, value)| ExecuteValue {
                 id: module_id,
                 expr: (),
@@ -951,7 +951,7 @@ where
         } = v;
 
         let vm1 = vm.clone();
-        execute(vm1, |vm| vm.execute_io(value.get_value()))
+        execute(vm1, |vm| vm.execute_io_top(value.get_value()))
             .map(move |(_, value)| {
                 // The type of the new value will be `a` instead of `IO a`
                 let actual = resolve::remove_aliases_cow(&*vm.get_env(), &typ);

src/io.rs

@@ -13,7 +13,7 @@ use vm::api::{
 use vm::future::FutureValue;
 use vm::gc::{Gc, Traverseable};
 use vm::internal::ValuePrinter;
-use vm::stack::{StackFrame, State};
+use vm::stack::StackFrame;
 use vm::thread::{Thread, ThreadInternal};
 use vm::types::*;
 use vm::{self, ExternModule, Result};
@@ -145,27 +145,14 @@ fn catch<'vm>(
     FutureResult(future)
 }
 
-fn clear_frames<T>(err: Error, stack: StackFrame) -> IO<T> {
-    fn clear_frames_(err: Error, mut stack: StackFrame) -> String {
-        let frame_level = stack
-            .stack
-            .get_frames()
-            .iter()
-            .rposition(|frame| frame.state == State::Lock)
-            .unwrap_or(0);
-
-        let fmt = match err {
-            // Ignore the stacktrace as we take a more specific range of the stack here
-            Error::VM(vm::Error::Panic(ref err, _)) => {
-                let trace = stack.stack.stacktrace(frame_level);
-                format!("{}\n{}", err, trace)
-            }
-            _ => format!("{}", err),
-        };
-        while let Ok(_) = stack.exit_scope() {}
-        fmt
+fn clear_frames<T>(mut err: Error, stack: StackFrame) -> IO<T> {
+    let new_trace = ::vm::thread::reset_stack(stack);
+    match err {
+        // Ignore the stacktrace as we take a more specific range of the stack here
+        Error::VM(vm::Error::Panic(_, ref mut trace)) => *trace = Some(new_trace),
+        _ => (),
     }
-    IO::Exception(clear_frames_(err, stack))
+    IO::Exception(err.to_string())
 }
 
 field_decl! { value, typ }

vm/src/thread.rs

@@ -75,6 +75,39 @@ where
     }
 }
 
+pub struct ExecuteTop<T>(pub Execute<T>);
+
+impl<T> Future for ExecuteTop<T>
+where
+    T: Deref<Target = Thread> + Clone,
+{
+    type Item = (T, Value);
+    type Error = Error;
+
+    // Returns `T` so that it can be reused by the caller
+    fn poll(&mut self) -> Poll<(T, Value), Error> {
+        let thread = self
+            .0
+            .thread
+            .as_ref()
+            .expect("cannot poll Execute future after it has succeded")
+            .clone();
+        match self.0.poll() {
+            Ok(Async::Ready(x)) => Ok(Async::Ready(x)),
+            Ok(Async::NotReady) => Ok(Async::NotReady),
+            Err(mut err) => {
+                let mut context = thread.context();
+                let stack = StackFrame::current(&mut context.stack);
+                let new_trace = reset_stack(stack);
+                if let Error::Panic(_, ref mut trace) = err {
+                    *trace = Some(new_trace);
+                }
+                Err(err)
+            }
+        }
+    }
+}
+
 /// Enum signaling a successful or unsuccess ful call to an extern function.
 /// If an error occured the error message is expected to be on the top of the stack.
 #[derive(Eq, PartialEq)]
@@ -814,9 +847,41 @@ where
     /// Evaluates a zero argument function (a thunk)
     fn call_thunk(&self, closure: GcPtr<ClosureData>) -> FutureValue<Execute<&Self>>;
 
+    fn call_thunk_top(&self, closure: GcPtr<ClosureData>) -> FutureValue<ExecuteTop<&Self>> {
+        match self.call_thunk(closure) {
+            FutureValue::Value(v) => FutureValue::Value(v.map_err(|mut err| {
+                let mut context = self.context();
+                let stack = StackFrame::current(&mut context.stack);
+                let new_trace = reset_stack(stack);
+                if let Error::Panic(_, ref mut trace) = err {
+                    *trace = Some(new_trace);
+                }
+                err
+            })),
+            FutureValue::Future(f) => FutureValue::Future(ExecuteTop(f)),
+            FutureValue::Polled => FutureValue::Polled,
+        }
+    }
+
     /// Executes an `IO` action
     fn execute_io(&self, value: Value) -> FutureValue<Execute<&Self>>;
 
+    fn execute_io_top(&self, value: Value) -> FutureValue<ExecuteTop<&Self>> {
+        match self.execute_io(value) {
+            FutureValue::Value(v) => FutureValue::Value(v.map_err(|mut err| {
+                let mut context = self.context();
+                let stack = StackFrame::current(&mut context.stack);
+                let new_trace = reset_stack(stack);
+                if let Error::Panic(_, ref mut trace) = err {
+                    *trace = Some(new_trace);
+                }
+                err
+            })),
+            FutureValue::Future(f) => FutureValue::Future(ExecuteTop(f)),
+            FutureValue::Polled => FutureValue::Polled,
+        }
+    }
+
     /// Calls a function on the stack.
     /// When this function is called it is expected that the function exists at
     /// `stack.len() - args - 1` and that the arguments are of the correct type
@@ -1475,22 +1540,29 @@ impl<'b> OwnedContext<'b> {
             let thread = self.thread;
             drop(self);
             // Poll the future that was returned from the initial call to this extern function
-            match poll_fn(thread)? {
-                Async::Ready(context) => {
+            match poll_fn(thread) {
+                Ok(Async::Ready(context)) => {
                     self = context;
                     if let Some(lock) = lock {
                         self.stack.release_lock(lock);
                     }
                     self.borrow_mut().stack.frame.instruction_index = POLL_CALL;
                     return Ok(Async::Ready(self));
                 }
-                Async::NotReady => {
+                Ok(Async::NotReady) => {
                     self = thread.owned_context();
                     self.stack.get_frames_mut()[frame_offset].instruction_index = POLL_CALL;
                     // Restore `poll_fn` so it can be polled again
                     self.poll_fns.push((lock, poll_fn));
                     return Ok(Async::NotReady);
                 }
+                Err(err) => {
+                    self = thread.owned_context();
+                    if let Some(lock) = lock {
+                        self.stack.release_lock(lock);
+                    }
+                    return Err(err);
+                }
             }
         }
 
@@ -1503,12 +1575,6 @@ impl<'b> OwnedContext<'b> {
                 debug!("{} {:?}", stack.len(), &*stack);
                 stack.pop();
             }
-            if !(match stack.frame.state {
-                State::Extern(ref e) => e.id == function.id,
-                _ => false,
-            }) {
-                "asd".to_string();
-            }
             debug_assert!(
                 match stack.frame.state {
                     State::Extern(ref e) => e.id == function.id,
@@ -2165,6 +2231,23 @@ impl<'vm> ActiveThread<'vm> {
         &mut self.context.as_mut().unwrap().gc
     }
 }
+#[doc(hidden)]
+pub fn reset_stack(mut stack: StackFrame) -> ::stack::Stacktrace {
+    let frame_level = stack
+        .stack
+        .get_frames()
+        .iter()
+        .rposition(|frame| frame.state == State::Lock)
+        .unwrap_or(0);
+
+    let trace = stack.stack.stacktrace(frame_level);
+    while stack.stack.get_frames().len() > 1 {
+        if let Err(_) = stack.exit_scope() {
+            break;
+        }
+    }
+    trace
+}
 
 #[cfg(test)]
 mod tests {