diff --git a/src/libextra/rl.rs b/src/libextra/rl.rs index 09ef7f22be5..f4ad07d464d 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,38 +42,53 @@ 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 + (locked!(rustrt::linenoiseHistorySave(buf))) == 1 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 + (locked!(rustrt::linenoiseHistoryLoad(buf))) == 1 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)) } @@ -84,8 +99,13 @@ 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) { +/// Bind to the main completion callback. +/// +/// 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(line: *c_char, completions: *()) { @@ -95,6 +115,10 @@ pub unsafe fn complete(cb: CompletionCb) { unsafe { do cb(str::raw::from_c_str(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. rustrt::linenoiseAddCompletion(completions, buf); } } @@ -102,5 +126,5 @@ pub unsafe fn complete(cb: CompletionCb) { } } - rustrt::linenoiseSetCompletionCallback(callback); + locked!(rustrt::linenoiseSetCompletionCallback(callback)); } diff --git a/src/librusti/rusti.rs b/src/librusti/rusti.rs index 5bd941759f4..8d61a971157 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 03a17d2c2ef..1871e7f36b3 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);