auto merge of #7769 : alexcrichton/rust/issue-7732-fix-rusti-again, r=cmr

Turns out this was a more subtle bug than I originally thought. My analysis can be found in #7732, but I also tried to put descriptive info into the comments.

Closes #7732

Author: bors

src/librustc/back/link.rs

@@ -101,15 +101,30 @@ pub mod jit {
     use back::link::llvm_err;
     use driver::session::Session;
     use lib::llvm::llvm;
-    use lib::llvm::{ModuleRef, ContextRef};
+    use lib::llvm::{ModuleRef, ContextRef, ExecutionEngineRef};
     use metadata::cstore;
 
     use std::cast;
-    use std::ptr;
-    use std::str;
-    use std::sys;
+    use std::local_data;
     use std::unstable::intrinsics;
 
+    struct LLVMJITData {
+        ee: ExecutionEngineRef,
+        llcx: ContextRef
+    }
+
+    pub trait Engine {}
+    impl Engine for LLVMJITData {}
+
+    impl Drop for LLVMJITData {
+        fn drop(&self) {
+            unsafe {
+                llvm::LLVMDisposeExecutionEngine(self.ee);
+                llvm::LLVMContextDispose(self.llcx);
+            }
+        }
+    }
+
     pub fn exec(sess: Session,
                 c: ContextRef,
                 m: ModuleRef,
@@ -130,7 +145,7 @@ pub mod jit {
 
                 debug!("linking: %s", path);
 
-                do str::as_c_str(path) |buf_t| {
+                do path.as_c_str |buf_t| {
                     if !llvm::LLVMRustLoadCrate(manager, buf_t) {
                         llvm_err(sess, ~"Could not link");
                     }
@@ -149,7 +164,7 @@ pub mod jit {
             // Next, we need to get a handle on the _rust_main function by
             // looking up it's corresponding ValueRef and then requesting that
             // the execution engine compiles the function.
-            let fun = do str::as_c_str("_rust_main") |entry| {
+            let fun = do "_rust_main".as_c_str |entry| {
                 llvm::LLVMGetNamedFunction(m, entry)
             };
             if fun.is_null() {
@@ -163,20 +178,45 @@ pub mod jit {
             // closure
             let code = llvm::LLVMGetPointerToGlobal(ee, fun);
             assert!(!code.is_null());
-            let closure = sys::Closure {
-                code: code,
-                env: ptr::null()
-            };
-            let func: &fn() = cast::transmute(closure);
+            let func: extern "Rust" fn() = cast::transmute(code);
             func();
 
-            // Sadly, there currently is no interface to re-use this execution
-            // engine, so it's disposed of here along with the context to
-            // prevent leaks.
-            llvm::LLVMDisposeExecutionEngine(ee);
-            llvm::LLVMContextDispose(c);
+            // Currently there is no method of re-using the executing engine
+            // from LLVM in another call to the JIT. While this kinda defeats
+            // the purpose of having a JIT in the first place, there isn't
+            // actually much code currently which would re-use data between
+            // different invocations of this. Additionally, the compilation
+            // model currently isn't designed to support this scenario.
+            //
+            // We can't destroy the engine/context immediately here, however,
+            // because of annihilation. The JIT code contains drop glue for any
+            // types defined in the crate we just ran, and if any of those boxes
+            // are going to be dropped during annihilation, the drop glue must
+            // be run. Hence, we need to transfer ownership of this jit engine
+            // to the caller of this function. To be convenient for now, we
+            // shove it into TLS and have someone else remove it later on.
+            let data = ~LLVMJITData { ee: ee, llcx: c };
+            set_engine(data as ~Engine);
         }
     }
+
+    // The stage1 compiler won't work, but that doesn't really matter. TLS
+    // changed only very recently to allow storage of owned values.
+    fn engine_key(_: ~Engine) {}
+
+    #[cfg(not(stage0))]
+    fn set_engine(engine: ~Engine) {
+        unsafe { local_data::set(engine_key, engine) }
+    }
+    #[cfg(stage0)]
+    fn set_engine(_: ~Engine) {}
+
+    #[cfg(not(stage0))]
+    pub fn consume_engine() -> Option<~Engine> {
+        unsafe { local_data::pop(engine_key) }
+    }
+    #[cfg(stage0)]
+    pub fn consume_engine() -> Option<~Engine> { None }
 }
 
 pub mod write {

src/librustc/rustc.rs

@@ -332,7 +332,11 @@ pub fn monitor(f: ~fn(diagnostic::Emitter)) {
 
         let _finally = finally { ch: ch };
 
-        f(demitter)
+        f(demitter);
+
+        // Due reasons explain in #7732, if there was a jit execution context it
+        // must be consumed and passed along to our parent task.
+        back::link::jit::consume_engine()
     } {
         result::Ok(_) => { /* fallthrough */ }
         result::Err(_) => {

src/librusti/rusti.rs

@@ -36,8 +36,23 @@
  * - Pass #3
  *   Finally, a program is generated to deserialize the local variable state,
  *   run the code input, and then reserialize all bindings back into a local
- *   hash map. Once this code runs, the input has fully been run and the REPL
- *   waits for new input.
+ *   hash map. This code is then run in the JIT engine provided by the rust
+ *   compiler.
+ *
+ * - Pass #4
+ *   Once this code runs, the input has fully been run and the hash map of local
+ *   variables from TLS is read back into the local store of variables. This is
+ *   then used later to pass back along to the parent rusti task and then begin
+ *   waiting for input again.
+ *
+ * - Pass #5
+ *   When running rusti code, it's important to consume ownership of the LLVM
+ *   jit contextual information to prevent code from being deallocated too soon
+ *   (before drop glue runs, see #7732). For this reason, the jit context is
+ *   consumed and also passed along to the parent task. The parent task then
+ *   keeps around all contexts while rusti is running. This must be done because
+ *   tasks could in theory be spawned off and running in the background (still
+ *   using the code).
  *
  * Encoding/decoding is done with EBML, and there is simply a map of ~str ->
  * ~[u8] maintaining the values of each local binding (by name).
@@ -60,6 +75,7 @@ use std::cell::Cell;
 use extra::rl;
 
 use rustc::driver::{driver, session};
+use rustc::back::link::jit;
 use syntax::{ast, diagnostic};
 use syntax::ast_util::*;
 use syntax::parse::token;
@@ -80,8 +96,9 @@ pub struct Repl {
     binary: ~str,
     running: bool,
     lib_search_paths: ~[~str],
+    engines: ~[~jit::Engine],
 
-    program: Program,
+    program: ~Program,
 }
 
 // Action to do after reading a :command
@@ -91,13 +108,15 @@ enum CmdAction {
 }
 
 /// Run an input string in a Repl, returning the new Repl.
-fn run(mut repl: Repl, input: ~str) -> Repl {
+fn run(mut program: ~Program, binary: ~str, lib_search_paths: ~[~str],
+       input: ~str) -> (~Program, Option<~jit::Engine>)
+{
     // Build some necessary rustc boilerplate for compiling things
-    let binary = repl.binary.to_managed();
+    let binary = binary.to_managed();
     let options = @session::options {
         crate_type: session::unknown_crate,
         binary: binary,
-        addl_lib_search_paths: @mut repl.lib_search_paths.map(|p| Path(*p)),
+        addl_lib_search_paths: @mut lib_search_paths.map(|p| Path(*p)),
         jit: true,
         .. copy *session::basic_options()
     };
@@ -136,9 +155,9 @@ fn run(mut repl: Repl, input: ~str) -> Repl {
             };
             match vi.node {
                 ast::view_item_extern_mod(*) => {
-                    repl.program.record_extern(s);
+                    program.record_extern(s);
                 }
-                ast::view_item_use(*) => { repl.program.record_view_item(s); }
+                ast::view_item_use(*) => { program.record_view_item(s); }
             }
         }
 
@@ -156,10 +175,10 @@ fn run(mut repl: Repl, input: ~str) -> Repl {
                                 // them at all usable they need to be decorated
                                 // with #[deriving(Encoable, Decodable)]
                                 ast::item_struct(*) => {
-                                    repl.program.record_struct(name, s);
+                                    program.record_struct(name, s);
                                 }
                                 // Item declarations are hoisted out of main()
-                                _ => { repl.program.record_item(name, s); }
+                                _ => { program.record_item(name, s); }
                             }
                         }
 
@@ -190,17 +209,17 @@ fn run(mut repl: Repl, input: ~str) -> Repl {
     }
     // return fast for empty inputs
     if to_run.len() == 0 && result.is_none() {
-        return repl;
+        return (program, None);
     }
 
     //
     // Stage 2: run everything up to typeck to learn the types of the new
     //          variables introduced into the program
     //
     info!("Learning about the new types in the program");
-    repl.program.set_cache(); // before register_new_vars (which changes them)
+    program.set_cache(); // before register_new_vars (which changes them)
     let input = to_run.connect("\n");
-    let test = repl.program.test_code(input, &result, *new_locals);
+    let test = program.test_code(input, &result, *new_locals);
     debug!("testing with ^^^^^^ %?", (||{ println(test) })());
     let dinput = driver::str_input(test.to_managed());
     let cfg = driver::build_configuration(sess, binary, &dinput);
@@ -210,14 +229,14 @@ fn run(mut repl: Repl, input: ~str) -> Repl {
     // Once we're typechecked, record the types of all local variables defined
     // in this input
     do find_main(crate.expect("crate after cu_typeck"), sess) |blk| {
-        repl.program.register_new_vars(blk, tcx.expect("tcx after cu_typeck"));
+        program.register_new_vars(blk, tcx.expect("tcx after cu_typeck"));
     }
 
     //
     // Stage 3: Actually run the code in the JIT
     //
     info!("actually running code");
-    let code = repl.program.code(input, &result);
+    let code = program.code(input, &result);
     debug!("actually running ^^^^^^ %?", (||{ println(code) })());
     let input = driver::str_input(code.to_managed());
     let cfg = driver::build_configuration(sess, binary, &input);
@@ -231,9 +250,15 @@ fn run(mut repl: Repl, input: ~str) -> Repl {
     //          local variable bindings.
     //
     info!("cleaning up after code");
-    repl.program.consume_cache();
+    program.consume_cache();
 
-    return repl;
+    //
+    // Stage 5: Extract the LLVM execution engine to take ownership of the
+    //          generated JIT code. This means that rusti can spawn parallel
+    //          tasks and we won't deallocate the code emitted until rusti
+    //          itself is destroyed.
+    //
+    return (program, jit::consume_engine());
 
     fn parse_input(sess: session::Session, binary: @str,
                    input: &str) -> @ast::crate {
@@ -418,8 +443,8 @@ fn run_cmd(repl: &mut Repl, _in: @io::Reader, _out: @io::Writer,
 /// Executes a line of input, which may either be rust code or a
 /// :command. Returns a new Repl if it has changed.
 pub fn run_line(repl: &mut Repl, in: @io::Reader, out: @io::Writer, line: ~str,
-                use_rl: bool)
-    -> Option<Repl> {
+                use_rl: bool) -> bool
+{
     if line.starts_with(":") {
         // drop the : and the \n (one byte each)
         let full = line.slice(1, line.len());
@@ -442,21 +467,30 @@ pub fn run_line(repl: &mut Repl, in: @io::Reader, out: @io::Writer, line: ~str,
                         }
                     }
                 }
-                return None;
+                return true;
             }
         }
     }
 
     let line = Cell::new(line);
-    let r = Cell::new(copy *repl);
+    let program = Cell::new(copy repl.program);
+    let lib_search_paths = Cell::new(copy repl.lib_search_paths);
+    let binary = Cell::new(copy repl.binary);
     let result = do task::try {
-        run(r.take(), line.take())
+        run(program.take(), binary.take(), lib_search_paths.take(), line.take())
     };
 
-    if result.is_ok() {
-        return Some(result.get());
+    match result {
+        Ok((program, engine)) => {
+            repl.program = program;
+            match engine {
+                Some(e) => { repl.engines.push(e); }
+                None => {}
+            }
+            return true;
+        }
+        Err(*) => { return false; }
     }
-    return None;
 }
 
 pub fn main() {
@@ -468,8 +502,9 @@ pub fn main() {
         binary: copy args[0],
         running: true,
         lib_search_paths: ~[],
+        engines: ~[],
 
-        program: Program::new(),
+        program: ~Program::new(),
     };
 
     let istty = unsafe { libc::isatty(libc::STDIN_FILENO as i32) } != 0;
@@ -502,10 +537,7 @@ pub fn main() {
                     }
                     loop;
                 }
-                match run_line(&mut repl, in, out, line, istty) {
-                    Some(new_repl) => repl = new_repl,
-                    None => { }
-                }
+                run_line(&mut repl, in, out, line, istty);
             }
         }
     }
@@ -524,7 +556,8 @@ mod tests {
             binary: ~"rusti",
             running: true,
             lib_search_paths: ~[],
-            program: Program::new(),
+            engines: ~[],
+            program: ~Program::new(),
         }
     }
 
@@ -535,9 +568,9 @@ mod tests {
     fn run_program(prog: &str) {
         let mut r = repl();
         for prog.split_iter('\n').advance |cmd| {
-            let result = run_line(&mut r, io::stdin(), io::stdout(),
-                                  cmd.to_owned(), false);
-            r = result.expect(fmt!("the command '%s' failed", cmd));
+            assert!(run_line(&mut r, io::stdin(), io::stdout(),
+                             cmd.to_owned(), false),
+                    "the command '%s' failed", cmd);
         }
     }
     fn run_program(_: &str) {}
@@ -682,7 +715,7 @@ mod tests {
         assert!(r.running);
         let result = run_line(&mut r, io::stdin(), io::stdout(),
                               ~":exit", false);
-        assert!(result.is_none());
+        assert!(result);
         assert!(!r.running);
     }
 }