From 7cac9fe76349120ea2373f3ce47a561271b5e8b6 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Tue, 3 Dec 2013 19:18:58 -0800 Subject: [PATCH] librustuv: RAII-ify `Local::borrow`, and remove some 12 Cells. --- src/librustuv/lib.rs | 17 +++--- src/librustuv/macros.rs | 5 +- src/librustuv/net.rs | 9 ++-- src/librustuv/uvio.rs | 17 +++--- src/libstd/at_vec.rs | 6 +-- src/libstd/rt/borrowck.rs | 9 ++-- src/libstd/rt/comm.rs | 13 ++--- src/libstd/rt/local.rs | 88 +++++++++++++++--------------- src/libstd/rt/local_heap.rs | 3 +- src/libstd/rt/local_ptr.rs | 47 ++++++++++++---- src/libstd/rt/sched.rs | 19 +++---- src/libstd/rt/task.rs | 104 ++++++++++++++++++------------------ src/libstd/rt/tube.rs | 27 +++++----- src/libstd/task/mod.rs | 20 +++---- 14 files changed, 208 insertions(+), 176 deletions(-) diff --git a/src/librustuv/lib.rs b/src/librustuv/lib.rs index ad1c53e9739..09a13bdddaa 100644 --- a/src/librustuv/lib.rs +++ b/src/librustuv/lib.rs @@ -162,16 +162,20 @@ pub struct ForbidSwitch { impl ForbidSwitch { fn new(s: &'static str) -> ForbidSwitch { + let mut sched = Local::borrow(None::); ForbidSwitch { - msg: s, sched: Local::borrow(|s: &mut Scheduler| s.sched_id()) + msg: s, + sched: sched.get().sched_id(), } } } impl Drop for ForbidSwitch { fn drop(&mut self) { - assert!(self.sched == Local::borrow(|s: &mut Scheduler| s.sched_id()), - "didnt want a scheduler switch: {}", self.msg); + let mut sched = Local::borrow(None::); + assert!(self.sched == sched.get().sched_id(), + "didnt want a scheduler switch: {}", + self.msg); } } @@ -389,15 +393,16 @@ pub fn slice_to_uv_buf(v: &[u8]) -> Buf { #[cfg(test)] fn local_loop() -> &'static mut Loop { unsafe { - cast::transmute(Local::borrow(|sched: &mut Scheduler| { + cast::transmute({ + let mut sched = Local::borrow(None::); let mut io = None; - sched.event_loop.io(|i| { + sched.get().event_loop.io(|i| { let (_vtable, uvio): (uint, &'static mut uvio::UvIoFactory) = cast::transmute(i); io = Some(uvio); }); io.unwrap() - }).uv_loop()) + }.uv_loop()) } } diff --git a/src/librustuv/macros.rs b/src/librustuv/macros.rs index eb679553e76..a63dcc6de31 100644 --- a/src/librustuv/macros.rs +++ b/src/librustuv/macros.rs @@ -29,7 +29,10 @@ macro_rules! uvdebug ( // get a handle for the current scheduler macro_rules! get_handle_to_current_scheduler( - () => (Local::borrow(|sched: &mut Scheduler| sched.make_handle())) + () => ({ + let mut sched = Local::borrow(None::); + sched.get().make_handle() + }) ) pub fn dumb_println(args: &fmt::Arguments) { diff --git a/src/librustuv/net.rs b/src/librustuv/net.rs index 1af0266f16a..1e9c4044345 100644 --- a/src/librustuv/net.rs +++ b/src/librustuv/net.rs @@ -1080,11 +1080,10 @@ fn test_simple_homed_udp_io_bind_then_move_task_then_home_and_close() { }; unsafe fn local_io() -> &'static mut IoFactory { - Local::borrow(|sched: &mut Scheduler| { - let mut io = None; - sched.event_loop.io(|i| io = Some(i)); - cast::transmute(io.unwrap()) - }) + let mut sched = Local::borrow(None::); + let mut io = None; + sched.get().event_loop.io(|i| io = Some(i)); + cast::transmute(io.unwrap()) } let test_function: proc() = proc() { diff --git a/src/librustuv/uvio.rs b/src/librustuv/uvio.rs index 8d80f5ff610..c0d7a35cef8 100644 --- a/src/librustuv/uvio.rs +++ b/src/librustuv/uvio.rs @@ -45,9 +45,10 @@ fn go_to_IO_home(&mut self) -> uint { let _f = ForbidUnwind::new("going home"); - let current_sched_id = Local::borrow(|sched: &mut Scheduler| { - sched.sched_id() - }); + let current_sched_id = { + let mut sched = Local::borrow(None::); + sched.get().sched_id() + }; // Only need to invoke a context switch if we're not on the right // scheduler. @@ -59,9 +60,10 @@ fn go_to_IO_home(&mut self) -> uint { }); }) } - let current_sched_id = Local::borrow(|sched: &mut Scheduler| { - sched.sched_id() - }); + let current_sched_id = { + let mut sched = Local::borrow(None::); + sched.get().sched_id() + }; assert!(current_sched_id == self.home().sched_id); self.home().sched_id @@ -96,7 +98,8 @@ struct HomingMissile { impl HomingMissile { pub fn check(&self, msg: &'static str) { - let local_id = Local::borrow(|sched: &mut Scheduler| sched.sched_id()); + let mut sched = Local::borrow(None::); + let local_id = sched.get().sched_id(); assert!(local_id == self.io_home, "{}", msg); } } diff --git a/src/libstd/at_vec.rs b/src/libstd/at_vec.rs index 2b105a3fa7d..2b64c5c83fb 100644 --- a/src/libstd/at_vec.rs +++ b/src/libstd/at_vec.rs @@ -169,6 +169,7 @@ pub mod raw { use at_vec::capacity; use cast; use cast::{transmute, transmute_copy}; + use option::None; use ptr; use mem; use uint; @@ -259,9 +260,8 @@ fn local_realloc(ptr: *(), size: uint) -> *() { use rt::local::Local; use rt::task::Task; - Local::borrow(|task: &mut Task| { - task.heap.realloc(ptr as *mut Box<()>, size) as *() - }) + let mut task = Local::borrow(None::); + task.get().heap.realloc(ptr as *mut Box<()>, size) as *() } } diff --git a/src/libstd/rt/borrowck.rs b/src/libstd/rt/borrowck.rs index 30c2264bd86..82f92bdb803 100644 --- a/src/libstd/rt/borrowck.rs +++ b/src/libstd/rt/borrowck.rs @@ -8,7 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use cell::Cell; use c_str::{ToCStr, CString}; use libc::{c_char, size_t}; use option::{Option, None, Some}; @@ -35,7 +34,8 @@ pub struct BorrowRecord { } fn try_take_task_borrow_list() -> Option<~[BorrowRecord]> { - Local::borrow(|task: &mut Task| task.borrow_list.take()) + let mut task = Local::borrow(None::); + task.get().borrow_list.take() } fn swap_task_borrow_list(f: |~[BorrowRecord]| -> ~[BorrowRecord]) { @@ -44,8 +44,9 @@ fn swap_task_borrow_list(f: |~[BorrowRecord]| -> ~[BorrowRecord]) { None => ~[] }; let borrows = f(borrows); - let borrows = Cell::new(borrows); - Local::borrow(|task: &mut Task| task.borrow_list = Some(borrows.take())) + + let mut task = Local::borrow(None::); + task.get().borrow_list = Some(borrows) } pub fn clear_task_borrow_list() { diff --git a/src/libstd/rt/comm.rs b/src/libstd/rt/comm.rs index 1b5303e76c9..d6024b7abea 100644 --- a/src/libstd/rt/comm.rs +++ b/src/libstd/rt/comm.rs @@ -25,7 +25,7 @@ use util; use util::Void; use comm::{GenericChan, GenericSmartChan, GenericPort, Peekable, SendDeferred}; -use cell::{Cell, RefCell}; +use cell::RefCell; use clone::Clone; use tuple::ImmutableTuple; @@ -169,10 +169,8 @@ fn try_send_inner(mut self, val: T, do_resched: bool) -> bool { Scheduler::run_task(woken_task); }); } else { - let recvr = Cell::new(recvr); - Local::borrow(|sched: &mut Scheduler| { - sched.enqueue_blocked_task(recvr.take()); - }) + let mut sched = Local::borrow(None::); + sched.get().enqueue_blocked_task(recvr); } } } @@ -230,9 +228,8 @@ fn optimistic_check(&mut self) -> bool { // The optimistic check is never necessary for correctness. For testing // purposes, making it randomly return false simulates a racing sender. use rand::{Rand}; - let actually_check = Local::borrow(|sched: &mut Scheduler| { - Rand::rand(&mut sched.rng) - }); + let mut sched = Local::borrow(None::); + let actually_check = Rand::rand(&mut sched.get().rng); if actually_check { unsafe { (*self.packet()).state.load(Acquire) == STATE_ONE } } else { diff --git a/src/libstd/rt/local.rs b/src/libstd/rt/local.rs index 2375ce55766..d73ad98a25b 100644 --- a/src/libstd/rt/local.rs +++ b/src/libstd/rt/local.rs @@ -12,36 +12,28 @@ use rt::sched::Scheduler; use rt::task::Task; use rt::local_ptr; -use cell::Cell; -pub trait Local { +/// Encapsulates some task-local data. +pub trait Local { fn put(value: ~Self); fn take() -> ~Self; fn exists(unused_value: Option) -> bool; - fn borrow(f: |&mut Self| -> T) -> T; + fn borrow(unused_value: Option) -> Borrowed; unsafe fn unsafe_take() -> ~Self; unsafe fn unsafe_borrow() -> *mut Self; unsafe fn try_unsafe_borrow() -> Option<*mut Self>; } -impl Local for Task { +impl Local> for Task { #[inline] fn put(value: ~Task) { unsafe { local_ptr::put(value) } } #[inline] fn take() -> ~Task { unsafe { local_ptr::take() } } fn exists(_: Option) -> bool { local_ptr::exists() } - fn borrow(f: |&mut Task| -> T) -> T { - let mut res: Option = None; - let res_ptr: *mut Option = &mut res; + #[inline] + fn borrow(_: Option) -> local_ptr::Borrowed { unsafe { - local_ptr::borrow(|task| { - let result = f(task); - *res_ptr = Some(result); - }) - } - match res { - Some(r) => { r } - None => { rtabort!("function failed in local_borrow") } + local_ptr::borrow::() } } #[inline] @@ -54,13 +46,35 @@ unsafe fn try_unsafe_borrow() -> Option<*mut Task> { } } -impl Local for Scheduler { +/// Encapsulates a temporarily-borrowed scheduler. +pub struct BorrowedScheduler { + priv task: local_ptr::Borrowed, +} + +impl BorrowedScheduler { + fn new(mut task: local_ptr::Borrowed) -> BorrowedScheduler { + if task.get().sched.is_none() { + rtabort!("no scheduler") + } else { + BorrowedScheduler { + task: task, + } + } + } + + #[inline] + pub fn get<'a>(&'a mut self) -> &'a mut ~Scheduler { + match self.task.get().sched { + None => rtabort!("no scheduler"), + Some(ref mut sched) => sched, + } + } +} + +impl Local for Scheduler { fn put(value: ~Scheduler) { - let value = Cell::new(value); - Local::borrow(|task: &mut Task| { - let task = task; - task.sched = Some(value.take()); - }); + let mut task = Local::borrow(None::); + task.get().sched = Some(value); } #[inline] fn take() -> ~Scheduler { @@ -71,24 +85,12 @@ fn take() -> ~Scheduler { } } fn exists(_: Option) -> bool { - Local::borrow(|task: &mut Task| { - match task.sched { - Some(ref _task) => true, - None => false - } - }) + let mut task = Local::borrow(None::); + task.get().sched.is_some() } - fn borrow(f: |&mut Scheduler| -> T) -> T { - Local::borrow(|task: &mut Task| { - match task.sched { - Some(~ref mut task) => { - f(task) - } - None => { - rtabort!("no scheduler") - } - } - }) + #[inline] + fn borrow(_: Option) -> BorrowedScheduler { + BorrowedScheduler::new(Local::borrow(None::)) } unsafe fn unsafe_take() -> ~Scheduler { rtabort!("unimpl") } unsafe fn unsafe_borrow() -> *mut Scheduler { @@ -182,11 +184,11 @@ fn borrow_with_return() { let task = ~Task::new_root(&mut sched.stack_pool, None, proc(){}); Local::put(task); - let res = Local::borrow(|_task: &mut Task| { - true - }); - assert!(res) - let task: ~Task = Local::take(); + { + let _ = Local::borrow(None::); + } + + let task: ~Task = Local::take(); cleanup_task(task); } } diff --git a/src/libstd/rt/local_heap.rs b/src/libstd/rt/local_heap.rs index 2386a261bdf..e364137de45 100644 --- a/src/libstd/rt/local_heap.rs +++ b/src/libstd/rt/local_heap.rs @@ -304,7 +304,8 @@ pub unsafe fn local_free(ptr: *libc::c_char) { } pub fn live_allocs() -> *mut Box { - Local::borrow(|task: &mut Task| task.heap.live_allocs) + let mut task = Local::borrow(None::); + task.get().heap.live_allocs } #[cfg(test)] diff --git a/src/libstd/rt/local_ptr.rs b/src/libstd/rt/local_ptr.rs index be3b5f951eb..66fe9742121 100644 --- a/src/libstd/rt/local_ptr.rs +++ b/src/libstd/rt/local_ptr.rs @@ -18,8 +18,7 @@ #[allow(dead_code)]; use cast; -use cell::Cell; -use unstable::finally::Finally; +use ops::Drop; #[cfg(windows)] // mingw-w32 doesn't like thread_local things #[cfg(target_os = "android")] // see #10686 @@ -28,20 +27,48 @@ #[cfg(not(windows), not(target_os = "android"))] pub use self::compiled::*; +/// Encapsulates a borrowed value. When this value goes out of scope, the +/// pointer is returned. +pub struct Borrowed { + priv val: *(), +} + +#[unsafe_destructor] +impl Drop for Borrowed { + fn drop(&mut self) { + unsafe { + if self.val.is_null() { + rtabort!("Aiee, returning null borrowed object!"); + } + let val: ~T = cast::transmute(self.val); + put::(val); + assert!(exists()); + } + } +} + +impl Borrowed { + pub fn get<'a>(&'a mut self) -> &'a mut T { + unsafe { + let val_ptr: &mut ~T = cast::transmute(&mut self.val); + let val_ptr: &'a mut T = *val_ptr; + val_ptr + } + } +} + /// Borrow the thread-local value from thread-local storage. /// While the value is borrowed it is not available in TLS. /// /// # Safety note /// /// Does not validate the pointer type. -pub unsafe fn borrow(f: |&mut T|) { - let mut value = take(); - - // XXX: Need a different abstraction from 'finally' here to avoid unsafety - let unsafe_ptr = cast::transmute_mut_region(&mut *value); - let value_cell = Cell::new(value); - - (|| f(unsafe_ptr)).finally(|| put(value_cell.take())); +#[inline] +pub unsafe fn borrow() -> Borrowed { + let val: *() = cast::transmute(take::()); + Borrowed { + val: val, + } } /// Compiled implementation of accessing the runtime local pointer. This is diff --git a/src/libstd/rt/sched.rs b/src/libstd/rt/sched.rs index af8de06adb7..f4410f7ee27 100644 --- a/src/libstd/rt/sched.rs +++ b/src/libstd/rt/sched.rs @@ -24,7 +24,6 @@ use rt::local::Local; use rt::rtio::{RemoteCallback, PausibleIdleCallback, Callback}; use borrow::{to_uint}; -use cell::Cell; use rand::{XorShiftRng, Rng, Rand}; use iter::range; use unstable::mutex::Mutex; @@ -235,12 +234,12 @@ pub fn run(mut ~self) { unsafe { let event_loop: *mut ~EventLoop = &mut self.event_loop; - // Our scheduler must be in the task before the event loop - // is started. - let self_sched = Cell::new(self); - Local::borrow(|stask: &mut Task| { - stask.sched = Some(self_sched.take()); - }); + { + // Our scheduler must be in the task before the event loop + // is started. + let mut stask = Local::borrow(None::); + stask.get().sched = Some(self); + } (*event_loop).run(); } @@ -751,10 +750,8 @@ pub fn run_task(task: ~Task) { } pub fn run_task_later(next_task: ~Task) { - let next_task = Cell::new(next_task); - Local::borrow(|sched: &mut Scheduler| { - sched.enqueue_task(next_task.take()); - }); + let mut sched = Local::borrow(None::); + sched.get().enqueue_task(next_task); } /// Yield control to the scheduler, executing another task. This is guaranteed diff --git a/src/libstd/rt/task.rs b/src/libstd/rt/task.rs index 63cc397e8d7..0cb60c58a6d 100644 --- a/src/libstd/rt/task.rs +++ b/src/libstd/rt/task.rs @@ -144,17 +144,15 @@ pub fn build_homed_child(stack_size: Option, f: proc(), home: SchedHome) -> ~Task { - let f = Cell::new(f); - let home = Cell::new(home); - Local::borrow(|running_task: &mut Task| { - let mut sched = running_task.sched.take_unwrap(); - let new_task = ~running_task.new_child_homed(&mut sched.stack_pool, - stack_size, - home.take(), - f.take()); - running_task.sched = Some(sched); - new_task - }) + let mut running_task = Local::borrow(None::); + let mut sched = running_task.get().sched.take_unwrap(); + let new_task = ~running_task.get() + .new_child_homed(&mut sched.stack_pool, + stack_size, + home, + f); + running_task.get().sched = Some(sched); + new_task } pub fn build_child(stack_size: Option, f: proc()) -> ~Task { @@ -165,17 +163,14 @@ pub fn build_homed_root(stack_size: Option, f: proc(), home: SchedHome) -> ~Task { - let f = Cell::new(f); - let home = Cell::new(home); - Local::borrow(|running_task: &mut Task| { - let mut sched = running_task.sched.take_unwrap(); - let new_task = ~Task::new_root_homed(&mut sched.stack_pool, - stack_size, - home.take(), - f.take()); - running_task.sched = Some(sched); - new_task - }) + let mut running_task = Local::borrow(None::); + let mut sched = running_task.get().sched.take_unwrap(); + let new_task = ~Task::new_root_homed(&mut sched.stack_pool, + stack_size, + home, + f); + running_task.get().sched = Some(sched); + new_task } pub fn build_root(stack_size: Option, f: proc()) -> ~Task { @@ -371,26 +366,25 @@ pub fn homed(&self) -> bool { // Grab both the scheduler and the task from TLS and check if the // task is executing on an appropriate scheduler. pub fn on_appropriate_sched() -> bool { - Local::borrow(|task: &mut Task| { - let sched_id = task.sched.get_ref().sched_id(); - let sched_run_anything = task.sched.get_ref().run_anything; - match task.task_type { - GreenTask(Some(AnySched)) => { - rtdebug!("anysched task in sched check ****"); - sched_run_anything - } - GreenTask(Some(Sched(SchedHandle { sched_id: ref id, ..}))) => { - rtdebug!("homed task in sched check ****"); - *id == sched_id - } - GreenTask(None) => { - rtabort!("task without home"); - } - SchedTask => { - rtabort!("type error: expected: GreenTask, found: SchedTask"); - } + let mut task = Local::borrow(None::); + let sched_id = task.get().sched.get_ref().sched_id(); + let sched_run_anything = task.get().sched.get_ref().run_anything; + match task.get().task_type { + GreenTask(Some(AnySched)) => { + rtdebug!("anysched task in sched check ****"); + sched_run_anything } - }) + GreenTask(Some(Sched(SchedHandle { sched_id: ref id, ..}))) => { + rtdebug!("homed task in sched check ****"); + *id == sched_id + } + GreenTask(None) => { + rtabort!("task without home"); + } + SchedTask => { + rtabort!("type error: expected: GreenTask, found: SchedTask"); + } + } } } @@ -440,9 +434,10 @@ fn build_start_wrapper(start: proc()) -> proc() { unsafe { // Again - might work while safe, or it might not. - Local::borrow(|sched: &mut Scheduler| { - sched.run_cleanup_job(); - }); + { + let mut sched = Local::borrow(None::); + sched.get().run_cleanup_job(); + } // To call the run method on a task we need a direct // reference to it. The task is in TLS, so we can @@ -594,16 +589,19 @@ pub extern "C" fn rust_stack_exhausted() { // #2361 - possible implementation of not using landing pads if in_green_task_context() { - Local::borrow(|task: &mut Task| { - let n = task.name.as_ref().map(|n| n.as_slice()).unwrap_or(""); + let mut task = Local::borrow(None::); + let n = task.get() + .name + .as_ref() + .map(|n| n.as_slice()) + .unwrap_or(""); - // See the message below for why this is not emitted to the - // task's logger. This has the additional conundrum of the - // logger may not be initialized just yet, meaning that an FFI - // call would happen to initialized it (calling out to libuv), - // and the FFI call needs 2MB of stack when we just ran out. - rterrln!("task '{}' has overflowed its stack", n); - }) + // See the message below for why this is not emitted to the + // task's logger. This has the additional conundrum of the + // logger may not be initialized just yet, meaning that an FFI + // call would happen to initialized it (calling out to libuv), + // and the FFI call needs 2MB of stack when we just ran out. + rterrln!("task '{}' has overflowed its stack", n); } else { rterrln!("stack overflow in non-task context"); } diff --git a/src/libstd/rt/tube.rs b/src/libstd/rt/tube.rs index 0d4171d5a64..15d8c7f9aac 100644 --- a/src/libstd/rt/tube.rs +++ b/src/libstd/rt/tube.rs @@ -121,9 +121,9 @@ fn blocking_test() { let tube_clone = Cell::new(tube_clone); let sched: ~Scheduler = Local::take(); sched.deschedule_running_task_and_then(|sched, task| { - let tube_clone = Cell::new(tube_clone.take()); + let tube_clone = tube_clone.take(); do sched.event_loop.callback { - let mut tube_clone = tube_clone.take(); + let mut tube_clone = tube_clone; // The task should be blocked on this now and // sending will wake it up. tube_clone.send(1); @@ -148,19 +148,18 @@ fn many_blocking_test() { callback_send(tube_clone.take(), 0); fn callback_send(tube: Tube, i: int) { - if i == 100 { return; } + if i == 100 { + return + } - let tube = Cell::new(Cell::new(tube)); - Local::borrow(|sched: &mut Scheduler| { - let tube = tube.take(); - do sched.event_loop.callback { - let mut tube = tube.take(); - // The task should be blocked on this now and - // sending will wake it up. - tube.send(i); - callback_send(tube, i + 1); - } - }) + let mut sched = Local::borrow(None::); + do sched.get().event_loop.callback { + let mut tube = tube; + // The task should be blocked on this now and + // sending will wake it up. + tube.send(i); + callback_send(tube, i + 1); + } } sched.enqueue_blocked_task(task); diff --git a/src/libstd/task/mod.rs b/src/libstd/task/mod.rs index 1777a523073..24a24f24818 100644 --- a/src/libstd/task/mod.rs +++ b/src/libstd/task/mod.rs @@ -429,12 +429,11 @@ pub fn with_task_name(blk: |Option<&str>| -> U) -> U { use rt::task::Task; if in_green_task_context() { - Local::borrow(|task: &mut Task| { - match task.name { - Some(ref name) => blk(Some(name.as_slice())), - None => blk(None) - } - }) + let mut task = Local::borrow(None::); + match task.get().name { + Some(ref name) => blk(Some(name.as_slice())), + None => blk(None) + } } else { fail!("no task name exists in non-green task context") } @@ -456,7 +455,8 @@ pub fn failing() -> bool { use rt::task::Task; - Local::borrow(|local: &mut Task| local.unwinder.unwinding) + let mut local = Local::borrow(None::); + local.get().unwinder.unwinding } // The following 8 tests test the following 2^3 combinations: @@ -601,9 +601,9 @@ fn test_try_fail() { #[cfg(test)] fn get_sched_id() -> int { - Local::borrow(|sched: &mut ::rt::sched::Scheduler| { - sched.sched_id() as int - }) + use rt::sched::Scheduler; + let mut sched = Local::borrow(None::); + sched.get().sched_id() as int } #[test]