diff --git a/src/libextra/rl.rs b/src/libextra/rl.rs index 09ef7f22be591..74b7aea997877 100644 --- a/src/libextra/rl.rs +++ b/src/libextra/rl.rs @@ -8,13 +8,10 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// FIXME #3921. This is unsafe because linenoise uses global mutable -// state without mutexes. - use std::c_str::ToCStr; use std::libc::{c_char, c_int}; -use std::local_data; -use std::str; +use std::{local_data, str, rt}; +use std::unstable::finally::Finally; #[cfg(stage0)] pub mod rustrt { @@ -28,6 +25,9 @@ pub mod rustrt { fn linenoiseHistoryLoad(file: *c_char) -> c_int; fn linenoiseSetCompletionCallback(callback: *u8); fn linenoiseAddCompletion(completions: *(), line: *c_char); + + fn rust_take_linenoise_lock(); + fn rust_drop_linenoise_lock(); } } @@ -42,65 +42,107 @@ pub mod rustrt { externfn!(fn linenoiseHistoryLoad(file: *c_char) -> c_int) externfn!(fn linenoiseSetCompletionCallback(callback: extern "C" fn(*i8, *()))) externfn!(fn linenoiseAddCompletion(completions: *(), line: *c_char)) + + externfn!(fn rust_take_linenoise_lock()) + externfn!(fn rust_drop_linenoise_lock()) +} + +macro_rules! locked { + ($expr:expr) => { + unsafe { + // FIXME #9105: can't use a static mutex in pure Rust yet. + rustrt::rust_take_linenoise_lock(); + let x = $expr; + rustrt::rust_drop_linenoise_lock(); + x + } + } } /// Add a line to history -pub unsafe fn add_history(line: &str) -> bool { +pub fn add_history(line: &str) -> bool { do line.with_c_str |buf| { - rustrt::linenoiseHistoryAdd(buf) == 1 as c_int + (locked!(rustrt::linenoiseHistoryAdd(buf))) == 1 as c_int } } /// Set the maximum amount of lines stored -pub unsafe fn set_history_max_len(len: int) -> bool { - rustrt::linenoiseHistorySetMaxLen(len as c_int) == 1 as c_int +pub fn set_history_max_len(len: int) -> bool { + (locked!(rustrt::linenoiseHistorySetMaxLen(len as c_int))) == 1 as c_int } /// Save line history to a file -pub unsafe fn save_history(file: &str) -> bool { +pub fn save_history(file: &str) -> bool { do file.with_c_str |buf| { - rustrt::linenoiseHistorySave(buf) == 1 as c_int + // 0 on success, -1 on failure + (locked!(rustrt::linenoiseHistorySave(buf))) == 0 as c_int } } /// Load line history from a file -pub unsafe fn load_history(file: &str) -> bool { +pub fn load_history(file: &str) -> bool { do file.with_c_str |buf| { - rustrt::linenoiseHistoryLoad(buf) == 1 as c_int + // 0 on success, -1 on failure + (locked!(rustrt::linenoiseHistoryLoad(buf))) == 0 as c_int } } /// Print out a prompt and then wait for input and return it -pub unsafe fn read(prompt: &str) -> Option<~str> { +pub fn read(prompt: &str) -> Option<~str> { do prompt.with_c_str |buf| { - let line = rustrt::linenoise(buf); + let line = locked!(rustrt::linenoise(buf)); if line.is_null() { None } - else { Some(str::raw::from_c_str(line)) } + else { + unsafe { + do (|| { + Some(str::raw::from_c_str(line)) + }).finally { + // linenoise's return value is from strdup, so we + // better not leak it. + rt::global_heap::exchange_free(line); + } + } + } } } pub type CompletionCb = @fn(~str, @fn(~str)); -static complete_key: local_data::Key<@CompletionCb> = &local_data::Key; - -/// Bind to the main completion callback -pub unsafe fn complete(cb: CompletionCb) { - local_data::set(complete_key, @cb); - - extern fn callback(line: *c_char, completions: *()) { - do local_data::get(complete_key) |cb| { - let cb = **cb.unwrap(); - - unsafe { - do cb(str::raw::from_c_str(line)) |suggestion| { - do suggestion.with_c_str |buf| { - rustrt::linenoiseAddCompletion(completions, buf); +static complete_key: local_data::Key = &local_data::Key; + +/// Bind to the main completion callback in the current task. +/// +/// The completion callback should not call any `extra::rl` functions +/// other than the closure that it receives as its second +/// argument. Calling such a function will deadlock on the mutex used +/// to ensure that the calls are thread-safe. +pub fn complete(cb: CompletionCb) { + local_data::set(complete_key, cb); + + extern fn callback(c_line: *c_char, completions: *()) { + do local_data::get(complete_key) |opt_cb| { + // only fetch completions if a completion handler has been + // registered in the current task. + match opt_cb { + None => {}, + Some(cb) => { + let line = unsafe { str::raw::from_c_str(c_line) }; + do (*cb)(line) |suggestion| { + do suggestion.with_c_str |buf| { + // This isn't locked, because `callback` gets + // called inside `rustrt::linenoise`, which + // *is* already inside the mutex, so + // re-locking would be a deadlock. + unsafe { + rustrt::linenoiseAddCompletion(completions, buf); + } + } } } } } } - rustrt::linenoiseSetCompletionCallback(callback); + locked!(rustrt::linenoiseSetCompletionCallback(callback)); } diff --git a/src/librusti/rusti.rs b/src/librusti/rusti.rs index 5bd941759f4d7..8d61a971157fc 100644 --- a/src/librusti/rusti.rs +++ b/src/librusti/rusti.rs @@ -355,12 +355,12 @@ fn compile_crate(src_filename: ~str, binary: ~str) -> Option { /// None if no input was read (e.g. EOF was reached). fn get_line(use_rl: bool, prompt: &str) -> Option<~str> { if use_rl { - let result = unsafe { rl::read(prompt) }; + let result = rl::read(prompt); match result { None => None, Some(line) => { - unsafe { rl::add_history(line) }; + rl::add_history(line); Some(line) } } @@ -525,14 +525,12 @@ pub fn main_args(args: &[~str]) { println("unstable. If you encounter problems, please use the"); println("compiler instead. Type :help for help."); - unsafe { - do rl::complete |line, suggest| { - if line.starts_with(":") { - suggest(~":clear"); - suggest(~":exit"); - suggest(~":help"); - suggest(~":load"); - } + do rl::complete |line, suggest| { + if line.starts_with(":") { + suggest(~":clear"); + suggest(~":exit"); + suggest(~":help"); + suggest(~":load"); } } } diff --git a/src/rt/rust_builtin.cpp b/src/rt/rust_builtin.cpp index 03a17d2c2ef97..1871e7f36b363 100644 --- a/src/rt/rust_builtin.cpp +++ b/src/rt/rust_builtin.cpp @@ -633,6 +633,18 @@ rust_drop_env_lock() { env_lock.unlock(); } +static lock_and_signal linenoise_lock; + +extern "C" CDECL void +rust_take_linenoise_lock() { + linenoise_lock.lock(); +} + +extern "C" CDECL void +rust_drop_linenoise_lock() { + linenoise_lock.unlock(); +} + extern "C" CDECL unsigned int rust_valgrind_stack_register(void *start, void *end) { return VALGRIND_STACK_REGISTER(start, end); diff --git a/src/rt/rustrt.def.in b/src/rt/rustrt.def.in index bf3500e4c724e..56405224c8a9f 100644 --- a/src/rt/rustrt.def.in +++ b/src/rt/rustrt.def.in @@ -189,6 +189,8 @@ rust_take_global_args_lock rust_drop_global_args_lock rust_take_change_dir_lock rust_drop_change_dir_lock +rust_take_linenoise_lock +rust_drop_linenoise_lock rust_get_test_int rust_get_task rust_uv_get_loop_from_getaddrinfo_req diff --git a/src/test/run-pass/rl-human-test.rs b/src/test/run-pass/rl-human-test.rs new file mode 100644 index 0000000000000..558e0b6820dcb --- /dev/null +++ b/src/test/run-pass/rl-human-test.rs @@ -0,0 +1,75 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// xfail-fast no compile flags for check-fast + +// we want this to be compiled to avoid bitrot, but the actual test +//has to be conducted by a human, i.e. someone (you?) compiling this +//file with a plain rustc invocation and running it and checking it +//works. + +// compile-flags: --cfg robot_mode + +extern mod extra; +use extra::rl; + +static HISTORY_FILE: &'static str = "rl-human-test-history.txt"; + +fn main() { + // don't run this in robot mode, but still typecheck it. + if !cfg!(robot_mode) { + println("~~ Welcome to the rl test \"suite\". ~~"); + println!("Operations: + - restrict the history to 2 lines, + - set the tab-completion to suggest three copies of each of the last 3 letters (or 'empty'), + - add 'one' and 'two' to the history, + - save it to `{0}`, + - add 'three', + - prompt & save input (check the history & completion work and contains only 'two', 'three'), + - load from `{0}` + - prompt & save input (history should be 'one', 'two' again), + - prompt once more. + +The bool return values of each step are printed.", + HISTORY_FILE); + + println!("restricting history length: {}", rl::set_history_max_len(3)); + + do rl::complete |line, suggest| { + if line.is_empty() { + suggest(~"empty") + } else { + for c in line.rev_iter().take(3) { + suggest(format!("{0}{1}{1}{1}", line, c)) + } + } + } + + println!("adding 'one': {}", rl::add_history("one")); + println!("adding 'two': {}", rl::add_history("two")); + + println!("saving history: {}", rl::save_history(HISTORY_FILE)); + + println!("adding 'three': {}", rl::add_history("three")); + + match rl::read("> ") { + Some(s) => println!("saving input: {}", rl::add_history(s)), + None => return + } + println!("loading history: {}", rl::load_history(HISTORY_FILE)); + + match rl::read("> ") { + Some(s) => println!("saving input: {}", rl::add_history(s)), + None => return + } + + rl::read("> "); + } +}