auto merge of #8447 : alexcrichton/rust/local-data-merge, r=brson

This moves all local_data stuff into the `local_data` module and only that
module alone. It also removes a fair amount of "super-unsafe" code in favor of
just vanilla code generated by the compiler at the same time.

Closes #8113

5 files changed, 467 insertions, 549 deletions

diff --git a/src/libstd/local_data.rs b/src/libstd/local_data.rs
index 5d6610e6b55..88e7dd692fe 100644
--- a/src/libstd/local_data.rs
+++ b/src/libstd/local_data.rs
@@ -39,11 +39,11 @@ magic.
 
 */
 
+use cast;
+use libc;
 use prelude::*;
-
-use task::local_data_priv::*;
-
-#[cfg(test)] use task;
+use rt::task::{Task, LocalStorage};
+use util;
 
 /**
  * Indexes a task-local data slot. This pointer is used for comparison to
@@ -60,263 +60,509 @@ pub type Key<T> = &'static KeyValue<T>;
 
 pub enum KeyValue<T> { Key }
 
-/**
- * Remove a task-local data value from the table, returning the
- * reference that was originally created to insert it.
- */
-pub fn pop<T: 'static>(key: Key<T>) -> Option<T> {
-    unsafe { local_pop(Handle::new(), key) }
-}
-
-/**
- * Retrieve a task-local data value. It will also be kept alive in the
- * table until explicitly removed.
- */
-pub fn get<T: 'static, U>(key: Key<T>, f: &fn(Option<&T>) -> U) -> U {
-    unsafe { local_get(Handle::new(), key, f) }
-}
+trait LocalData {}
+impl<T: 'static> LocalData for T {}
 
-/**
- * Retrieve a mutable borrowed pointer to a task-local data value.
- */
-pub fn get_mut<T: 'static, U>(key: Key<T>, f: &fn(Option<&mut T>) -> U) -> U {
-    unsafe { local_get_mut(Handle::new(), key, f) }
+// The task-local-map stores all TLS information for the currently running task.
+// It is stored as an owned pointer into the runtime, and it's only allocated
+// when TLS is used for the first time. This map must be very carefully
+// constructed because it has many mutable loans unsoundly handed out on it to
+// the various invocations of TLS requests.
+//
+// One of the most important operations is loaning a value via `get` to a
+// caller. In doing so, the slot that the TLS entry is occupying cannot be
+// invalidated because upon returning it's loan state must be updated. Currently
+// the TLS map is a vector, but this is possibly dangerous because the vector
+// can be reallocated/moved when new values are pushed onto it.
+//
+// This problem currently isn't solved in a very elegant way. Inside the `get`
+// function, it internally "invalidates" all references after the loan is
+// finished and looks up into the vector again. In theory this will prevent
+// pointers from being moved under our feet so long as LLVM doesn't go too crazy
+// with the optimizations.
+//
+// n.b. If TLS is used heavily in future, this could be made more efficient with
+//      a proper map.
+#[doc(hidden)]
+pub type Map = ~[Option<(*libc::c_void, TLSValue, LoanState)>];
+type TLSValue = ~LocalData;
+
+// Gets the map from the runtime. Lazily initialises if not done so already.
+unsafe fn get_local_map() -> &mut Map {
+    use rt::local::Local;
+
+    let task: *mut Task = Local::unsafe_borrow();
+    match &mut (*task).storage {
+        // If the at_exit function is already set, then we just need to take
+        // a loan out on the TLS map stored inside
+        &LocalStorage(Some(ref mut map_ptr)) => {
+            return map_ptr;
+        }
+        // If this is the first time we've accessed TLS, perform similar
+        // actions to the oldsched way of doing things.
+        &LocalStorage(ref mut slot) => {
+            *slot = Some(~[]);
+            match *slot {
+                Some(ref mut map_ptr) => { return map_ptr }
+                None => abort()
+            }
+        }
+    }
 }
 
-/**
- * Store a value in task-local data. If this key already has a value,
- * that value is overwritten (and its destructor is run).
- */
-pub fn set<T: 'static>(key: Key<T>, data: T) {
-    unsafe { local_set(Handle::new(), key, data) }
+#[deriving(Eq)]
+enum LoanState {
+    NoLoan, ImmLoan, MutLoan
 }
 
-/**
- * Modify a task-local data value. If the function returns 'None', the
- * data is removed (and its reference dropped).
- */
-pub fn modify<T: 'static>(key: Key<T>, f: &fn(Option<T>) -> Option<T>) {
-    unsafe {
-        match f(pop(::cast::unsafe_copy(&key))) {
-            Some(next) => { set(key, next); }
-            None => {}
+impl LoanState {
+    fn describe(&self) -> &'static str {
+        match *self {
+            NoLoan => "no loan",
+            ImmLoan => "immutable",
+            MutLoan => "mutable"
         }
     }
 }
 
-#[test]
-fn test_tls_multitask() {
-    static my_key: Key<@~str> = &Key;
-    set(my_key, @~"parent data");
-    do task::spawn {
-        // TLS shouldn't carry over.
-        assert!(get(my_key, |k| k.map_move(|k| *k)).is_none());
-        set(my_key, @~"child data");
-        assert!(*(get(my_key, |k| k.map_move(|k| *k)).unwrap()) ==
-                ~"child data");
-        // should be cleaned up for us
+fn key_to_key_value<T: 'static>(key: Key<T>) -> *libc::c_void {
+    unsafe { cast::transmute(key) }
+}
+
+/// Removes a task-local value from task-local storage. This will return
+/// Some(value) if the key was present in TLS, otherwise it will return None.
+///
+/// A runtime assertion will be triggered it removal of TLS value is attempted
+/// while the value is still loaned out via `get` or `get_mut`.
+pub fn pop<T: 'static>(key: Key<T>) -> Option<T> {
+    let map = unsafe { get_local_map() };
+    let key_value = key_to_key_value(key);
+
+    for entry in map.mut_iter() {
+        match *entry {
+            Some((k, _, loan)) if k == key_value => {
+                if loan != NoLoan {
+                    fail!("TLS value cannot be removed because it is currently \
+                          borrowed as %s", loan.describe());
+                }
+                // Move the data out of the `entry` slot via util::replace.
+                // This is guaranteed to succeed because we already matched
+                // on `Some` above.
+                let data = match util::replace(entry, None) {
+                    Some((_, data, _)) => data,
+                    None => abort()
+                };
+
+                // Move `data` into transmute to get out the memory that it
+                // owns, we must free it manually later.
+                let (_vtable, box): (uint, ~~T) = unsafe {
+                    cast::transmute(data)
+                };
+
+                // Now that we own `box`, we can just move out of it as we would
+                // with any other data.
+                return Some(**box);
+            }
+            _ => {}
+        }
     }
-    // Must work multiple times
-    assert!(*(get(my_key, |k| k.map_move(|k| *k)).unwrap()) == ~"parent data");
-    assert!(*(get(my_key, |k| k.map_move(|k| *k)).unwrap()) == ~"parent data");
-    assert!(*(get(my_key, |k| k.map_move(|k| *k)).unwrap()) == ~"parent data");
+    return None;
 }
 
-#[test]
-fn test_tls_overwrite() {
-    static my_key: Key<@~str> = &Key;
-    set(my_key, @~"first data");
-    set(my_key, @~"next data"); // Shouldn't leak.
-    assert!(*(get(my_key, |k| k.map_move(|k| *k)).unwrap()) == ~"next data");
+/// Retrieves a value from TLS. The closure provided is yielded `Some` of a
+/// reference to the value located in TLS if one exists, or `None` if the key
+/// provided is not present in TLS currently.
+///
+/// It is considered a runtime error to attempt to get a value which is already
+/// on loan via the `get_mut` method provided.
+pub fn get<T: 'static, U>(key: Key<T>, f: &fn(Option<&T>) -> U) -> U {
+    get_with(key, ImmLoan, f)
 }
 
-#[test]
-fn test_tls_pop() {
-    static my_key: Key<@~str> = &Key;
-    set(my_key, @~"weasel");
-    assert!(*(pop(my_key).unwrap()) == ~"weasel");
-    // Pop must remove the data from the map.
-    assert!(pop(my_key).is_none());
+/// Retrieves a mutable value from TLS. The closure provided is yielded `Some`
+/// of a reference to the mutable value located in TLS if one exists, or `None`
+/// if the key provided is not present in TLS currently.
+///
+/// It is considered a runtime error to attempt to get a value which is already
+/// on loan via this or the `get` methods. This is similar to how it's a runtime
+/// error to take two mutable loans on an `@mut` box.
+pub fn get_mut<T: 'static, U>(key: Key<T>, f: &fn(Option<&mut T>) -> U) -> U {
+    do get_with(key, MutLoan) |x| {
+        match x {
+            None => f(None),
+            // We're violating a lot of compiler guarantees with this
+            // invocation of `transmute_mut`, but we're doing runtime checks to
+            // ensure that it's always valid (only one at a time).
+            //
+            // there is no need to be upset!
+            Some(x) => { f(Some(unsafe { cast::transmute_mut(x) })) }
+        }
+    }
 }
 
-#[test]
-fn test_tls_modify() {
-    static my_key: Key<@~str> = &Key;
-    modify(my_key, |data| {
-        match data {
-            Some(@ref val) => fail!("unwelcome value: %s", *val),
-            None           => Some(@~"first data")
+fn get_with<T: 'static, U>(key: Key<T>,
+                           state: LoanState,
+                           f: &fn(Option<&T>) -> U) -> U {
+    // This function must be extremely careful. Because TLS can store owned
+    // values, and we must have some form of `get` function other than `pop`,
+    // this function has to give a `&` reference back to the caller.
+    //
+    // One option is to return the reference, but this cannot be sound because
+    // the actual lifetime of the object is not known. The slot in TLS could not
+    // be modified until the object goes out of scope, but the TLS code cannot
+    // know when this happens.
+    //
+    // For this reason, the reference is yielded to a specified closure. This
+    // way the TLS code knows exactly what the lifetime of the yielded pointer
+    // is, allowing callers to acquire references to owned data. This is also
+    // sound so long as measures are taken to ensure that while a TLS slot is
+    // loaned out to a caller, it's not modified recursively.
+    let map = unsafe { get_local_map() };
+    let key_value = key_to_key_value(key);
+
+    let pos = map.iter().position(|entry| {
+        match *entry {
+            Some((k, _, _)) if k == key_value => true, _ => false
         }
     });
-    modify(my_key, |data| {
-        match data {
-            Some(@~"first data") => Some(@~"next data"),
-            Some(@ref val)       => fail!("wrong value: %s", *val),
-            None                 => fail!("missing value")
+    match pos {
+        None => { return f(None); }
+        Some(i) => {
+            let ret;
+            let mut return_loan = false;
+            match map[i] {
+                Some((_, ref data, ref mut loan)) => {
+                    match (state, *loan) {
+                        (_, NoLoan) => {
+                            *loan = state;
+                            return_loan = true;
+                        }
+                        (ImmLoan, ImmLoan) => {}
+                        (want, cur) => {
+                            fail!("TLS slot cannot be borrowed as %s because \
+                                   it is already borrowed as %s",
+                                  want.describe(), cur.describe());
+                        }
+                    }
+                    // data was created with `~~T as ~LocalData`, so we extract
+                    // pointer part of the trait, (as ~~T), and then use
+                    // compiler coercions to achieve a '&' pointer.
+                    unsafe {
+                        match *cast::transmute::<&TLSValue, &(uint, ~~T)>(data){
+                            (_vtable, ref box) => {
+                                let value: &T = **box;
+                                ret = f(Some(value));
+                            }
+                        }
+                    }
+                }
+                _ => abort()
+            }
+
+            // n.b. 'data' and 'loans' are both invalid pointers at the point
+            // 'f' returned because `f` could have appended more TLS items which
+            // in turn relocated the vector. Hence we do another lookup here to
+            // fixup the loans.
+            if return_loan {
+                match map[i] {
+                    Some((_, _, ref mut loan)) => { *loan = NoLoan; }
+                    None => abort()
+                }
+            }
+            return ret;
         }
-    });
-    assert!(*(pop(my_key).unwrap()) == ~"next data");
+    }
 }
 
-#[test]
-fn test_tls_crust_automorestack_memorial_bug() {
-    // This might result in a stack-canary clobber if the runtime fails to
-    // set sp_limit to 0 when calling the cleanup extern - it might
-    // automatically jump over to the rust stack, which causes next_c_sp
-    // to get recorded as something within a rust stack segment. Then a
-    // subsequent upcall (esp. for logging, think vsnprintf) would run on
-    // a stack smaller than 1 MB.
-    static my_key: Key<@~str> = &Key;
-    do task::spawn {
-        set(my_key, @~"hax");
-    }
+fn abort() -> ! {
+    #[fixed_stack_segment]; #[inline(never)];
+    unsafe { libc::abort() }
 }
 
-#[test]
-fn test_tls_multiple_types() {
-    static str_key: Key<@~str> = &Key;
-    static box_key: Key<@@()> = &Key;
-    static int_key: Key<@int> = &Key;
-    do task::spawn {
-        set(str_key, @~"string data");
-        set(box_key, @@());
-        set(int_key, @42);
+/// Inserts a value into task local storage. If the key is already present in
+/// TLS, then the previous value is removed and replaced with the provided data.
+///
+/// It is considered a runtime error to attempt to set a key which is currently
+/// on loan via the `get` or `get_mut` methods.
+pub fn set<T: 'static>(key: Key<T>, data: T) {
+    let map = unsafe { get_local_map() };
+    let keyval = key_to_key_value(key);
+
+    // When the task-local map is destroyed, all the data needs to be cleaned
+    // up. For this reason we can't do some clever tricks to store '~T' as a
+    // '*c_void' or something like that. To solve the problem, we cast
+    // everything to a trait (LocalData) which is then stored inside the map.
+    // Upon destruction of the map, all the objects will be destroyed and the
+    // traits have enough information about them to destroy themselves.
+    //
+    // FIXME(#7673): This should be "~data as ~LocalData" (only one sigil)
+    let data = ~~data as ~LocalData:;
+
+    fn insertion_position(map: &mut Map,
+                          key: *libc::c_void) -> Option<uint> {
+        // First see if the map contains this key already
+        let curspot = map.iter().position(|entry| {
+            match *entry {
+                Some((ekey, _, loan)) if key == ekey => {
+                    if loan != NoLoan {
+                        fail!("TLS value cannot be overwritten because it is
+                               already borrowed as %s", loan.describe())
+                    }
+                    true
+                }
+                _ => false,
+            }
+        });
+        // If it doesn't contain the key, just find a slot that's None
+        match curspot {
+            Some(i) => Some(i),
+            None => map.iter().position(|entry| entry.is_none())
+        }
+    }
+
+    // The type of the local data map must ascribe to Send, so we do the
+    // transmute here to add the Send bound back on. This doesn't actually
+    // matter because TLS will always own the data (until its moved out) and
+    // we're not actually sending it to other schedulers or anything.
+    let data: ~LocalData = unsafe { cast::transmute(data) };
+    match insertion_position(map, keyval) {
+        Some(i) => { map[i] = Some((keyval, data, NoLoan)); }
+        None => { map.push(Some((keyval, data, NoLoan))); }
     }
 }
 
-#[test]
-fn test_tls_overwrite_multiple_types() {
-    static str_key: Key<@~str> = &Key;
-    static box_key: Key<@@()> = &Key;
-    static int_key: Key<@int> = &Key;
-    do task::spawn {
-        set(str_key, @~"string data");
-        set(int_key, @42);
-        // This could cause a segfault if overwriting-destruction is done
-        // with the crazy polymorphic transmute rather than the provided
-        // finaliser.
-        set(int_key, @31337);
+/// Modifies a task-local value by temporarily removing it from task-local
+/// storage and then re-inserting if `Some` is returned from the closure.
+///
+/// This function will have the same runtime errors as generated from `pop` and
+/// `set` (the key must not currently be on loan
+pub fn modify<T: 'static>(key: Key<T>, f: &fn(Option<T>) -> Option<T>) {
+    match f(pop(key)) {
+        Some(next) => { set(key, next); }
+        None => {}
     }
 }
 
-#[test]
-#[should_fail]
-fn test_tls_cleanup_on_failure() {
-    static str_key: Key<@~str> = &Key;
-    static box_key: Key<@@()> = &Key;
-    static int_key: Key<@int> = &Key;
-    set(str_key, @~"parent data");
-    set(box_key, @@());
-    do task::spawn {
-        // spawn_linked
-        set(str_key, @~"string data");
+#[cfg(test)]
+mod tests {
+    use prelude::*;
+    use super::*;
+    use task;
+
+    #[test]
+    fn test_tls_multitask() {
+        static my_key: Key<@~str> = &Key;
+        set(my_key, @~"parent data");
+        do task::spawn {
+            // TLS shouldn't carry over.
+            assert!(get(my_key, |k| k.map_move(|k| *k)).is_none());
+            set(my_key, @~"child data");
+            assert!(*(get(my_key, |k| k.map_move(|k| *k)).unwrap()) ==
+                    ~"child data");
+            // should be cleaned up for us
+        }
+        // Must work multiple times
+        assert!(*(get(my_key, |k| k.map_move(|k| *k)).unwrap()) == ~"parent data");
+        assert!(*(get(my_key, |k| k.map_move(|k| *k)).unwrap()) == ~"parent data");
+        assert!(*(get(my_key, |k| k.map_move(|k| *k)).unwrap()) == ~"parent data");
+    }
+
+    #[test]
+    fn test_tls_overwrite() {
+        static my_key: Key<@~str> = &Key;
+        set(my_key, @~"first data");
+        set(my_key, @~"next data"); // Shouldn't leak.
+        assert!(*(get(my_key, |k| k.map_move(|k| *k)).unwrap()) == ~"next data");
+    }
+
+    #[test]
+    fn test_tls_pop() {
+        static my_key: Key<@~str> = &Key;
+        set(my_key, @~"weasel");
+        assert!(*(pop(my_key).unwrap()) == ~"weasel");
+        // Pop must remove the data from the map.
+        assert!(pop(my_key).is_none());
+    }
+
+    #[test]
+    fn test_tls_modify() {
+        static my_key: Key<@~str> = &Key;
+        modify(my_key, |data| {
+            match data {
+                Some(@ref val) => fail!("unwelcome value: %s", *val),
+                None           => Some(@~"first data")
+            }
+        });
+        modify(my_key, |data| {
+            match data {
+                Some(@~"first data") => Some(@~"next data"),
+                Some(@ref val)       => fail!("wrong value: %s", *val),
+                None                 => fail!("missing value")
+            }
+        });
+        assert!(*(pop(my_key).unwrap()) == ~"next data");
+    }
+
+    #[test]
+    fn test_tls_crust_automorestack_memorial_bug() {
+        // This might result in a stack-canary clobber if the runtime fails to
+        // set sp_limit to 0 when calling the cleanup extern - it might
+        // automatically jump over to the rust stack, which causes next_c_sp
+        // to get recorded as something within a rust stack segment. Then a
+        // subsequent upcall (esp. for logging, think vsnprintf) would run on
+        // a stack smaller than 1 MB.
+        static my_key: Key<@~str> = &Key;
+        do task::spawn {
+            set(my_key, @~"hax");
+        }
+    }
+
+    #[test]
+    fn test_tls_multiple_types() {
+        static str_key: Key<@~str> = &Key;
+        static box_key: Key<@@()> = &Key;
+        static int_key: Key<@int> = &Key;
+        do task::spawn {
+            set(str_key, @~"string data");
+            set(box_key, @@());
+            set(int_key, @42);
+        }
+    }
+
+    #[test]
+    fn test_tls_overwrite_multiple_types() {
+        static str_key: Key<@~str> = &Key;
+        static box_key: Key<@@()> = &Key;
+        static int_key: Key<@int> = &Key;
+        do task::spawn {
+            set(str_key, @~"string data");
+            set(int_key, @42);
+            // This could cause a segfault if overwriting-destruction is done
+            // with the crazy polymorphic transmute rather than the provided
+            // finaliser.
+            set(int_key, @31337);
+        }
+    }
+
+    #[test]
+    #[should_fail]
+    fn test_tls_cleanup_on_failure() {
+        static str_key: Key<@~str> = &Key;
+        static box_key: Key<@@()> = &Key;
+        static int_key: Key<@int> = &Key;
+        set(str_key, @~"parent data");
         set(box_key, @@());
-        set(int_key, @42);
+        do task::spawn {
+            // spawn_linked
+            set(str_key, @~"string data");
+            set(box_key, @@());
+            set(int_key, @42);
+            fail!();
+        }
+        // Not quite nondeterministic.
+        set(int_key, @31337);
         fail!();
     }
-    // Not quite nondeterministic.
-    set(int_key, @31337);
-    fail!();
-}
 
-#[test]
-fn test_static_pointer() {
-    static key: Key<@&'static int> = &Key;
-    static VALUE: int = 0;
-    let v: @&'static int = @&VALUE;
-    set(key, v);
-}
+    #[test]
+    fn test_static_pointer() {
+        static key: Key<@&'static int> = &Key;
+        static VALUE: int = 0;
+        let v: @&'static int = @&VALUE;
+        set(key, v);
+    }
 
-#[test]
-fn test_owned() {
-    static key: Key<~int> = &Key;
-    set(key, ~1);
+    #[test]
+    fn test_owned() {
+        static key: Key<~int> = &Key;
+        set(key, ~1);
 
-    do get(key) |v| {
         do get(key) |v| {
             do get(key) |v| {
+                do get(key) |v| {
+                    assert_eq!(**v.unwrap(), 1);
+                }
                 assert_eq!(**v.unwrap(), 1);
             }
             assert_eq!(**v.unwrap(), 1);
         }
-        assert_eq!(**v.unwrap(), 1);
-    }
-    set(key, ~2);
-    do get(key) |v| {
-        assert_eq!(**v.unwrap(), 2);
+        set(key, ~2);
+        do get(key) |v| {
+            assert_eq!(**v.unwrap(), 2);
+        }
     }
-}
 
-#[test]
-fn test_get_mut() {
-    static key: Key<int> = &Key;
-    set(key, 1);
+    #[test]
+    fn test_get_mut() {
+        static key: Key<int> = &Key;
+        set(key, 1);
 
-    do get_mut(key) |v| {
-        *v.unwrap() = 2;
-    }
+        do get_mut(key) |v| {
+            *v.unwrap() = 2;
+        }
 
-    do get(key) |v| {
-        assert_eq!(*v.unwrap(), 2);
+        do get(key) |v| {
+            assert_eq!(*v.unwrap(), 2);
+        }
     }
-}
 
-#[test]
-fn test_same_key_type() {
-    static key1: Key<int> = &Key;
-    static key2: Key<int> = &Key;
-    static key3: Key<int> = &Key;
-    static key4: Key<int> = &Key;
-    static key5: Key<int> = &Key;
-    set(key1, 1);
-    set(key2, 2);
-    set(key3, 3);
-    set(key4, 4);
-    set(key5, 5);
-
-    get(key1, |x| assert_eq!(*x.unwrap(), 1));
-    get(key2, |x| assert_eq!(*x.unwrap(), 2));
-    get(key3, |x| assert_eq!(*x.unwrap(), 3));
-    get(key4, |x| assert_eq!(*x.unwrap(), 4));
-    get(key5, |x| assert_eq!(*x.unwrap(), 5));
-}
+    #[test]
+    fn test_same_key_type() {
+        static key1: Key<int> = &Key;
+        static key2: Key<int> = &Key;
+        static key3: Key<int> = &Key;
+        static key4: Key<int> = &Key;
+        static key5: Key<int> = &Key;
+        set(key1, 1);
+        set(key2, 2);
+        set(key3, 3);
+        set(key4, 4);
+        set(key5, 5);
+
+        get(key1, |x| assert_eq!(*x.unwrap(), 1));
+        get(key2, |x| assert_eq!(*x.unwrap(), 2));
+        get(key3, |x| assert_eq!(*x.unwrap(), 3));
+        get(key4, |x| assert_eq!(*x.unwrap(), 4));
+        get(key5, |x| assert_eq!(*x.unwrap(), 5));
+    }
 
-#[test]
-#[should_fail]
-fn test_nested_get_set1() {
-    static key: Key<int> = &Key;
-    set(key, 4);
-    do get(key) |_| {
+    #[test]
+    #[should_fail]
+    fn test_nested_get_set1() {
+        static key: Key<int> = &Key;
         set(key, 4);
+        do get(key) |_| {
+            set(key, 4);
+        }
     }
-}
 
-#[test]
-#[should_fail]
-fn test_nested_get_mut2() {
-    static key: Key<int> = &Key;
-    set(key, 4);
-    do get(key) |_| {
-        get_mut(key, |_| {})
+    #[test]
+    #[should_fail]
+    fn test_nested_get_mut2() {
+        static key: Key<int> = &Key;
+        set(key, 4);
+        do get(key) |_| {
+            get_mut(key, |_| {})
+        }
     }
-}
 
-#[test]
-#[should_fail]
-fn test_nested_get_mut3() {
-    static key: Key<int> = &Key;
-    set(key, 4);
-    do get_mut(key) |_| {
-        get(key, |_| {})
+    #[test]
+    #[should_fail]
+    fn test_nested_get_mut3() {
+        static key: Key<int> = &Key;
+        set(key, 4);
+        do get_mut(key) |_| {
+            get(key, |_| {})
+        }
     }
-}
 
-#[test]
-#[should_fail]
-fn test_nested_get_mut4() {
-    static key: Key<int> = &Key;
-    set(key, 4);
-    do get_mut(key) |_| {
-        get_mut(key, |_| {})
+    #[test]
+    #[should_fail]
+    fn test_nested_get_mut4() {
+        static key: Key<int> = &Key;
+        set(key, 4);
+        do get_mut(key) |_| {
+            get_mut(key, |_| {})
+        }
     }
 }
diff --git a/src/libstd/rt/mod.rs b/src/libstd/rt/mod.rs
index 6dbeb8c0ea9..7728a388c65 100644
--- a/src/libstd/rt/mod.rs
+++ b/src/libstd/rt/mod.rs
@@ -55,10 +55,6 @@ Several modules in `core` are clients of `rt`:
 */
 
 #[doc(hidden)];
-#[deny(unused_imports)];
-#[deny(unused_mut)];
-#[deny(unused_variable)];
-#[deny(unused_unsafe)];
 
 use cell::Cell;
 use clone::Clone;
diff --git a/src/libstd/rt/task.rs b/src/libstd/rt/task.rs
index b1ab7a6cd5d..da81aab0f78 100644
--- a/src/libstd/rt/task.rs
+++ b/src/libstd/rt/task.rs
@@ -16,8 +16,8 @@
 use borrow;
 use cast::transmute;
 use cleanup;
+use local_data;
 use libc::{c_void, uintptr_t};
-use ptr;
 use prelude::*;
 use option::{Option, Some, None};
 use rt::borrowck;
@@ -80,7 +80,7 @@ pub enum SchedHome {
 }
 
 pub struct GarbageCollector;
-pub struct LocalStorage(*c_void, Option<extern "Rust" fn(*c_void)>);
+pub struct LocalStorage(Option<local_data::Map>);
 
 pub struct Unwinder {
     unwinding: bool,
@@ -130,7 +130,7 @@ impl Task {
         Task {
             heap: LocalHeap::new(),
             gc: GarbageCollector,
-            storage: LocalStorage(ptr::null(), None),
+            storage: LocalStorage(None),
             logger: StdErrLogger,
             unwinder: Unwinder { unwinding: false },
             taskgroup: None,
@@ -164,7 +164,7 @@ impl Task {
         Task {
             heap: LocalHeap::new(),
             gc: GarbageCollector,
-            storage: LocalStorage(ptr::null(), None),
+            storage: LocalStorage(None),
             logger: StdErrLogger,
             unwinder: Unwinder { unwinding: false },
             taskgroup: None,
@@ -186,7 +186,7 @@ impl Task {
         Task {
             heap: LocalHeap::new(),
             gc: GarbageCollector,
-            storage: LocalStorage(ptr::null(), None),
+            storage: LocalStorage(None),
             logger: StdErrLogger,
             unwinder: Unwinder { unwinding: false },
             taskgroup: None,
@@ -233,15 +233,8 @@ impl Task {
 
             // Run the task main function, then do some cleanup.
             do f.finally {
-
-                // Destroy task-local storage. This may run user dtors.
-                match self.storage {
-                    LocalStorage(ptr, Some(ref dtor)) => {
-                        (*dtor)(ptr)
-                    }
-                    _ => ()
-                }
-
+                // First, destroy task-local storage. This may run user dtors.
+                //
                 // FIXME #8302: Dear diary. I'm so tired and confused.
                 // There's some interaction in rustc between the box
                 // annihilator and the TLS dtor by which TLS is
@@ -253,7 +246,13 @@ impl Task {
                 // TLS would be reinitialized but never destroyed,
                 // but somehow this works. I have no idea what's going
                 // on but this seems to make things magically work. FML.
-                self.storage = LocalStorage(ptr::null(), None);
+                //
+                // (added after initial comment) A possible interaction here is
+                // that the destructors for the objects in TLS themselves invoke
+                // TLS, or possibly some destructors for those objects being
+                // annihilated invoke TLS. Sadly these two operations seemed to
+                // be intertwined, and miraculously work for now...
+                self.storage.take();
 
                 // Destroy remaining boxes. Also may run user dtors.
                 unsafe { cleanup::annihilate(); }
diff --git a/src/libstd/task/local_data_priv.rs b/src/libstd/task/local_data_priv.rs
deleted file mode 100644
index 2c2dfd8f689..00000000000
--- a/src/libstd/task/local_data_priv.rs
+++ /dev/null
@@ -1,322 +0,0 @@
-// Copyright 2012 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 <LICENSE-APACHE or
-// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
-// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
-// option. This file may not be copied, modified, or distributed
-// except according to those terms.
-
-#[allow(missing_doc)];
-
-use cast;
-use libc;
-use local_data;
-use prelude::*;
-use ptr;
-use unstable::raw;
-use util;
-
-use rt::task::{Task, LocalStorage};
-
-pub enum Handle {
-    NewHandle(*mut LocalStorage)
-}
-
-impl Handle {
-    pub fn new() -> Handle {
-        use rt::local::Local;
-        unsafe {
-            let task: *mut Task = Local::unsafe_borrow();
-            NewHandle(&mut (*task).storage)
-        }
-    }
-}
-
-#[deriving(Eq)]
-enum LoanState {
-    NoLoan, ImmLoan, MutLoan
-}
-
-impl LoanState {
-    fn describe(&self) -> &'static str {
-        match *self {
-            NoLoan => "no loan",
-            ImmLoan => "immutable",
-            MutLoan => "mutable"
-        }
-    }
-}
-
-trait LocalData {}
-impl<T: 'static> LocalData for T {}
-
-// The task-local-map stores all TLS information for the currently running task.
-// It is stored as an owned pointer into the runtime, and it's only allocated
-// when TLS is used for the first time. This map must be very carefully
-// constructed because it has many mutable loans unsoundly handed out on it to
-// the various invocations of TLS requests.
-//
-// One of the most important operations is loaning a value via `get` to a
-// caller. In doing so, the slot that the TLS entry is occupying cannot be
-// invalidated because upon returning it's loan state must be updated. Currently
-// the TLS map is a vector, but this is possibly dangerous because the vector
-// can be reallocated/moved when new values are pushed onto it.
-//
-// This problem currently isn't solved in a very elegant way. Inside the `get`
-// function, it internally "invalidates" all references after the loan is
-// finished and looks up into the vector again. In theory this will prevent
-// pointers from being moved under our feet so long as LLVM doesn't go too crazy
-// with the optimizations.
-//
-// n.b. Other structures are not sufficient right now:
-//          * HashMap uses ~[T] internally (push reallocates/moves)
-//          * TreeMap is plausible, but it's in extra
-//          * dlist plausible, but not in std
-//          * a custom owned linked list was attempted, but difficult to write
-//            and involved a lot of extra code bloat
-//
-// n.b. Has to be stored with a pointer at outermost layer; the foreign call
-//      returns void *.
-//
-// n.b. If TLS is used heavily in future, this could be made more efficient with
-//      a proper map.
-type TaskLocalMap = ~[Option<(*libc::c_void, TLSValue, LoanState)>];
-type TLSValue = ~LocalData:;
-
-fn cleanup_task_local_map(map_ptr: *libc::c_void) {
-    unsafe {
-        assert!(!map_ptr.is_null());
-        // Get and keep the single reference that was created at the
-        // beginning.
-        let _map: TaskLocalMap = cast::transmute(map_ptr);
-        // All local_data will be destroyed along with the map.
-    }
-}
-
-// Gets the map from the runtime. Lazily initialises if not done so already.
-unsafe fn get_local_map(handle: Handle) -> &mut TaskLocalMap {
-
-    unsafe fn newsched_map(local: *mut LocalStorage) -> &mut TaskLocalMap {
-        // This is based on the same idea as the oldsched code above.
-        match &mut *local {
-            // If the at_exit function is already set, then we just need to take
-            // a loan out on the TLS map stored inside
-            &LocalStorage(ref mut map_ptr, Some(_)) => {
-                assert!(map_ptr.is_not_null());
-                return cast::transmute(map_ptr);
-            }
-            // If this is the first time we've accessed TLS, perform similar
-            // actions to the oldsched way of doing things.
-            &LocalStorage(ref mut map_ptr, ref mut at_exit) => {
-                assert!(map_ptr.is_null());
-                assert!(at_exit.is_none());
-                let map: TaskLocalMap = ~[];
-                *map_ptr = cast::transmute(map);
-                *at_exit = Some(cleanup_task_local_map);
-                return cast::transmute(map_ptr);
-            }
-        }
-    }
-
-    match handle {
-        NewHandle(local_storage) => newsched_map(local_storage)
-    }
-}
-
-unsafe fn key_to_key_value<T: 'static>(key: local_data::Key<T>) -> *libc::c_void {
-    let pair: raw::Closure = cast::transmute_copy(&key);
-    return pair.code as *libc::c_void;
-}
-
-pub unsafe fn local_pop<T: 'static>(handle: Handle,
-                                    key: local_data::Key<T>) -> Option<T> {
-    let map = get_local_map(handle);
-    let key_value = key_to_key_value(key);
-
-    for entry in map.mut_iter() {
-        match *entry {
-            Some((k, _, loan)) if k == key_value => {
-                if loan != NoLoan {
-                    fail!("TLS value cannot be removed because it is already \
-                          borrowed as %s", loan.describe());
-                }
-                // Move the data out of the `entry` slot via util::replace. This
-                // is guaranteed to succeed because we already matched on `Some`
-                // above.
-                let data = match util::replace(entry, None) {
-                    Some((_, data, _)) => data,
-                    None => abort(),
-                };
-
-                // Move `data` into transmute to get out the memory that it
-                // owns, we must free it manually later.
-                let (_vtable, box): (uint, ~~T) = cast::transmute(data);
-
-                // Read the box's value (using the compiler's built-in
-                // auto-deref functionality to obtain a pointer to the base)
-                let ret = ptr::read_ptr(cast::transmute::<&T, *mut T>(*box));
-
-                // Finally free the allocated memory. we don't want this to
-                // actually touch the memory inside because it's all duplicated
-                // now, so the box is transmuted to a 0-sized type. We also use
-                // a type which references `T` because currently the layout
-                // could depend on whether T contains managed pointers or not.
-                let _: ~~[T, ..0] = cast::transmute(box);
-
-                // Everything is now deallocated, and we own the value that was
-                // located inside TLS, so we now return it.
-                return Some(ret);
-            }
-            _ => {}
-        }
-    }
-    return None;
-}
-
-pub unsafe fn local_get<T: 'static, U>(handle: Handle,
-                                       key: local_data::Key<T>,
-                                       f: &fn(Option<&T>) -> U) -> U {
-    local_get_with(handle, key, ImmLoan, f)
-}
-
-pub unsafe fn local_get_mut<T: 'static, U>(handle: Handle,
-                                           key: local_data::Key<T>,
-                                           f: &fn(Option<&mut T>) -> U) -> U {
-    do local_get_with(handle, key, MutLoan) |x| {
-        match x {
-            None => f(None),
-            // We're violating a lot of compiler guarantees with this
-            // invocation of `transmute_mut`, but we're doing runtime checks to
-            // ensure that it's always valid (only one at a time).
-            //
-            // there is no need to be upset!
-            Some(x) => { f(Some(cast::transmute_mut(x))) }
-        }
-    }
-}
-
-unsafe fn local_get_with<T: 'static, U>(handle: Handle,
-                                        key: local_data::Key<T>,
-                                        state: LoanState,
-                                        f: &fn(Option<&T>) -> U) -> U {
-    // This function must be extremely careful. Because TLS can store owned
-    // values, and we must have some form of `get` function other than `pop`,
-    // this function has to give a `&` reference back to the caller.
-    //
-    // One option is to return the reference, but this cannot be sound because
-    // the actual lifetime of the object is not known. The slot in TLS could not
-    // be modified until the object goes out of scope, but the TLS code cannot
-    // know when this happens.
-    //
-    // For this reason, the reference is yielded to a specified closure. This
-    // way the TLS code knows exactly what the lifetime of the yielded pointer
-    // is, allowing callers to acquire references to owned data. This is also
-    // sound so long as measures are taken to ensure that while a TLS slot is
-    // loaned out to a caller, it's not modified recursively.
-    let map = get_local_map(handle);
-    let key_value = key_to_key_value(key);
-
-    let pos = map.iter().position(|entry| {
-        match *entry {
-            Some((k, _, _)) if k == key_value => true, _ => false
-        }
-    });
-    match pos {
-        None => { return f(None); }
-        Some(i) => {
-            let ret;
-            let mut return_loan = false;
-            match map[i] {
-                Some((_, ref data, ref mut loan)) => {
-                    match (state, *loan) {
-                        (_, NoLoan) => {
-                            *loan = state;
-                            return_loan = true;
-                        }
-                        (ImmLoan, ImmLoan) => {}
-                        (want, cur) => {
-                            fail!("TLS slot cannot be borrowed as %s because \
-                                   it is already borrowed as %s",
-                                  want.describe(), cur.describe());
-                        }
-                    }
-                    // data was created with `~~T as ~LocalData`, so we extract
-                    // pointer part of the trait, (as ~~T), and then use
-                    // compiler coercions to achieve a '&' pointer.
-                    match *cast::transmute::<&TLSValue, &(uint, ~~T)>(data) {
-                        (_vtable, ref box) => {
-                            let value: &T = **box;
-                            ret = f(Some(value));
-                        }
-                    }
-                }
-                _ => abort()
-            }
-
-            // n.b. 'data' and 'loans' are both invalid pointers at the point
-            // 'f' returned because `f` could have appended more TLS items which
-            // in turn relocated the vector. Hence we do another lookup here to
-            // fixup the loans.
-            if return_loan {
-                match map[i] {
-                    Some((_, _, ref mut loan)) => { *loan = NoLoan; }
-                    None => { abort(); }
-                }
-            }
-            return ret;
-        }
-    }
-}
-
-fn abort() -> ! {
-    #[fixed_stack_segment]; #[inline(never)];
-
-    unsafe { libc::abort() }
-}
-
-pub unsafe fn local_set<T: 'static>(handle: Handle,
-                                    key: local_data::Key<T>,
-                                    data: T) {
-    let map = get_local_map(handle);
-    let keyval = key_to_key_value(key);
-
-    // When the task-local map is destroyed, all the data needs to be cleaned
-    // up. For this reason we can't do some clever tricks to store '~T' as a
-    // '*c_void' or something like that. To solve the problem, we cast
-    // everything to a trait (LocalData) which is then stored inside the map.
-    // Upon destruction of the map, all the objects will be destroyed and the
-    // traits have enough information about them to destroy themselves.
-    //
-    // FIXME(#7673): This should be "~data as ~LocalData" (without the colon at
-    //               the end, and only one sigil)
-    let data = ~~data as ~LocalData:;
-
-    fn insertion_position(map: &mut TaskLocalMap,
-                          key: *libc::c_void) -> Option<uint> {
-        // First see if the map contains this key already
-        let curspot = map.iter().position(|entry| {
-            match *entry {
-                Some((ekey, _, loan)) if key == ekey => {
-                    if loan != NoLoan {
-                        fail!("TLS value cannot be overwritten because it is
-                               already borrowed as %s", loan.describe())
-                    }
-                    true
-                }
-                _ => false,
-            }
-        });
-        // If it doesn't contain the key, just find a slot that's None
-        match curspot {
-            Some(i) => Some(i),
-            None => map.iter().position(|entry| entry.is_none())
-        }
-    }
-
-    match insertion_position(map, keyval) {
-        Some(i) => { map[i] = Some((keyval, data, NoLoan)); }
-        None => { map.push(Some((keyval, data, NoLoan))); }
-    }
-}
diff --git a/src/libstd/task/mod.rs b/src/libstd/task/mod.rs
index c0b331c52ee..b52dd3a906b 100644
--- a/src/libstd/task/mod.rs
+++ b/src/libstd/task/mod.rs
@@ -52,7 +52,6 @@ use util;
 #[cfg(test)] use ptr;
 #[cfg(test)] use task;
 
-mod local_data_priv;
 pub mod spawn;
 
 /**