From a89af1fa4cc8548a1c5e0a655a196d94b047ccd7 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 9 Jul 2013 01:02:13 -0700 Subject: [PATCH 1/8] Use purely an owned vector for storing TLS data --- src/libstd/task/local_data_priv.rs | 81 +++++++++++++----------------- src/libstd/task/rt.rs | 4 +- src/rt/rust_builtin.cpp | 10 ++-- src/rt/rustrt.def.in | 1 - 4 files changed, 40 insertions(+), 56 deletions(-) diff --git a/src/libstd/task/local_data_priv.rs b/src/libstd/task/local_data_priv.rs index 8dd96df4545..7162afc6705 100644 --- a/src/libstd/task/local_data_priv.rs +++ b/src/libstd/task/local_data_priv.rs @@ -13,9 +13,10 @@ use cast; use cmp::Eq; use libc; -use prelude::*; -use task::rt; use local_data::LocalDataKey; +use prelude::*; +use sys; +use task::rt; use super::rt::rust_task; use rt::task::{Task, LocalStorage}; @@ -61,7 +62,7 @@ impl Eq for @LocalData { // proper map. type TaskLocalElement = (*libc::c_void, *libc::c_void, @LocalData); // Has to be a pointer at outermost layer; the foreign call returns void *. -type TaskLocalMap = @mut ~[Option]; +type TaskLocalMap = ~[Option]; fn cleanup_task_local_map(map_ptr: *libc::c_void) { unsafe { @@ -74,85 +75,75 @@ fn cleanup_task_local_map(map_ptr: *libc::c_void) { } // Gets the map from the runtime. Lazily initialises if not done so already. -unsafe fn get_local_map(handle: Handle) -> TaskLocalMap { +unsafe fn get_local_map(handle: Handle) -> &mut TaskLocalMap { match handle { OldHandle(task) => get_task_local_map(task), NewHandle(local_storage) => get_newsched_local_map(local_storage) } } -unsafe fn get_task_local_map(task: *rust_task) -> TaskLocalMap { +unsafe fn get_task_local_map(task: *rust_task) -> &mut TaskLocalMap { extern fn cleanup_task_local_map_extern_cb(map_ptr: *libc::c_void) { cleanup_task_local_map(map_ptr); } // Relies on the runtime initialising the pointer to null. - // Note: The map's box lives in TLS invisibly referenced once. Each time - // we retrieve it for get/set, we make another reference, which get/set - // drop when they finish. No "re-storing after modifying" is needed. + // Note: the map is an owned pointer and is "owned" by TLS. It is moved + // into the tls slot for this task, and then mutable loans are taken from + // this slot to modify the map. let map_ptr = rt::rust_get_task_local_data(task); - if map_ptr.is_null() { - let map: TaskLocalMap = @mut ~[]; - // NB: This bumps the ref count before converting to an unsafe pointer, - // keeping the map alive until TLS is destroyed - rt::rust_set_task_local_data(task, cast::transmute(map)); + if (*map_ptr).is_null() { + // First time TLS is used, create a new map and set up the necessary + // TLS information for its safe destruction + let map: TaskLocalMap = ~[]; + *map_ptr = cast::transmute(map); rt::rust_task_local_data_atexit(task, cleanup_task_local_map_extern_cb); - map - } else { - let map = cast::transmute(map_ptr); - let nonmut = cast::transmute::]>(map); - cast::bump_box_refcount(nonmut); - map } + return cast::transmute(map_ptr); } -unsafe fn get_newsched_local_map(local: *mut LocalStorage) -> TaskLocalMap { +unsafe fn get_newsched_local_map(local: *mut LocalStorage) -> &mut TaskLocalMap { + // This is based on the same idea as the oldsched code above. match &mut *local { - &LocalStorage(map_ptr, Some(_)) => { + // 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()); - let map = cast::transmute(map_ptr); - let nonmut = cast::transmute::]>(map); - cast::bump_box_refcount(nonmut); - return map; + 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()); - let map: TaskLocalMap = @mut ~[]; + assert!(map_ptr.is_null()); + assert!(at_exit.is_none()); + let map: TaskLocalMap = ~[]; *map_ptr = cast::transmute(map); - let at_exit_fn: ~fn(*libc::c_void) = |p|cleanup_task_local_map(p); + let at_exit_fn: ~fn(*libc::c_void) = |p| cleanup_task_local_map(p); *at_exit = Some(at_exit_fn); - return map; + return cast::transmute(map_ptr); } } } unsafe fn key_to_key_value(key: LocalDataKey) -> *libc::c_void { - // Keys are closures, which are (fnptr,envptr) pairs. Use fnptr. - // Use reinterpret_cast -- transmute would leak (forget) the closure. - let pair: (*libc::c_void, *libc::c_void) = cast::transmute_copy(&key); - pair.first() + let pair: sys::Closure = cast::transmute(key); + return pair.code as *libc::c_void; } // If returning Some(..), returns with @T with the map's reference. Careful! unsafe fn local_data_lookup( - map: TaskLocalMap, key: LocalDataKey) + map: &mut TaskLocalMap, key: LocalDataKey) -> Option<(uint, *libc::c_void)> { let key_value = key_to_key_value(key); - let map_pos = (*map).iter().position(|entry| + for map.iter().enumerate().advance |(i, entry)| { match *entry { - Some((k,_,_)) => k == key_value, - None => false + Some((k, data, _)) if k == key_value => { return Some((i, data)); } + _ => {} } - ); - do map_pos.map |index| { - // .get() is guaranteed because of "None { false }" above. - let (_, data_ptr, _) = (*map)[*index].get(); - (*index, data_ptr) } + return None; } unsafe fn local_get_helper( @@ -215,7 +206,7 @@ pub unsafe fn local_set( } None => { // Find an empty slot. If not, grow the vector. - match (*map).iter().position(|x| x.is_none()) { + match map.iter().position(|x| x.is_none()) { Some(empty_index) => { map[empty_index] = new_entry; } None => { map.push(new_entry); } } diff --git a/src/libstd/task/rt.rs b/src/libstd/task/rt.rs index 4860ab36f77..76fcad0759a 100644 --- a/src/libstd/task/rt.rs +++ b/src/libstd/task/rt.rs @@ -63,9 +63,7 @@ pub extern { fn rust_task_kill_all(task: *rust_task); #[rust_stack] - fn rust_get_task_local_data(task: *rust_task) -> *libc::c_void; - #[rust_stack] - fn rust_set_task_local_data(task: *rust_task, map: *libc::c_void); + fn rust_get_task_local_data(task: *rust_task) -> *mut *libc::c_void; #[rust_stack] fn rust_task_local_data_atexit(task: *rust_task, cleanup_fn: *u8); } diff --git a/src/rt/rust_builtin.cpp b/src/rt/rust_builtin.cpp index 17f36e810cd..f240c7fa28a 100644 --- a/src/rt/rust_builtin.cpp +++ b/src/rt/rust_builtin.cpp @@ -672,14 +672,10 @@ rust_unlock_little_lock(lock_and_signal *lock) { lock->unlock(); } -// set/get/atexit task_local_data can run on the rust stack for speed. -extern "C" void * +// get/atexit task_local_data can run on the rust stack for speed. +extern "C" void ** rust_get_task_local_data(rust_task *task) { - return task->task_local_data; -} -extern "C" void -rust_set_task_local_data(rust_task *task, void *data) { - task->task_local_data = data; + return &task->task_local_data; } extern "C" void rust_task_local_data_atexit(rust_task *task, void (*cleanup_fn)(void *data)) { diff --git a/src/rt/rustrt.def.in b/src/rt/rustrt.def.in index 0da04e34f49..55caf038227 100644 --- a/src/rt/rustrt.def.in +++ b/src/rt/rustrt.def.in @@ -171,7 +171,6 @@ rust_destroy_little_lock rust_lock_little_lock rust_unlock_little_lock rust_get_task_local_data -rust_set_task_local_data rust_task_local_data_atexit rust_task_ref rust_task_deref From 692a22e69d916f8d0153ee9dfa906272561e364a Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 9 Jul 2013 09:40:50 -0700 Subject: [PATCH 2/8] Change the elements in the task-local map to be actual key-value pairs --- src/libstd/local_data.rs | 8 +- src/libstd/rt/task.rs | 2 +- src/libstd/task/local_data_priv.rs | 230 +++++++++++++---------------- 3 files changed, 112 insertions(+), 128 deletions(-) diff --git a/src/libstd/local_data.rs b/src/libstd/local_data.rs index c5f2c8ae584..fbb11dfaa34 100644 --- a/src/libstd/local_data.rs +++ b/src/libstd/local_data.rs @@ -28,7 +28,7 @@ magic. use prelude::*; -use task::local_data_priv::{local_get, local_pop, local_modify, local_set, Handle}; +use task::local_data_priv::{local_get, local_pop, local_set, Handle}; #[cfg(test)] use task; @@ -83,7 +83,11 @@ pub unsafe fn local_data_modify( key: LocalDataKey, modify_fn: &fn(Option<@T>) -> Option<@T>) { - local_modify(Handle::new(), key, modify_fn) + let cur = local_data_pop(key); + match modify_fn(cur) { + Some(next) => { local_data_set(key, next); } + None => {} + } } #[test] diff --git a/src/libstd/rt/task.rs b/src/libstd/rt/task.rs index f5f5aca71f5..04c1f972a82 100644 --- a/src/libstd/rt/task.rs +++ b/src/libstd/rt/task.rs @@ -32,7 +32,7 @@ pub struct Task { } pub struct GarbageCollector; -pub struct LocalStorage(*c_void, Option<~fn(*c_void)>); +pub struct LocalStorage(*c_void, Option); pub struct Unwinder { unwinding: bool, diff --git a/src/libstd/task/local_data_priv.rs b/src/libstd/task/local_data_priv.rs index 7162afc6705..16600ffab06 100644 --- a/src/libstd/task/local_data_priv.rs +++ b/src/libstd/task/local_data_priv.rs @@ -11,7 +11,6 @@ #[allow(missing_doc)]; use cast; -use cmp::Eq; use libc; use local_data::LocalDataKey; use prelude::*; @@ -44,25 +43,19 @@ impl Handle { } } -pub trait LocalData { } -impl LocalData for @T { } +trait LocalData {} +impl LocalData for T {} -impl Eq for @LocalData { - fn eq(&self, other: &@LocalData) -> bool { - unsafe { - let ptr_a: &(uint, uint) = cast::transmute(self); - let ptr_b: &(uint, uint) = cast::transmute(other); - return ptr_a == ptr_b; - } - } - fn ne(&self, other: &@LocalData) -> bool { !(*self).eq(other) } -} - -// If TLS is used heavily in future, this could be made more efficient with a -// proper map. -type TaskLocalElement = (*libc::c_void, *libc::c_void, @LocalData); -// Has to be a pointer at outermost layer; the foreign call returns void *. -type TaskLocalMap = ~[Option]; +// The task-local-map actuall stores all TLS information. Right now it's a list +// of key-value pairs. Each value is an actual Rust type so that when the map is +// destroyed all of the contents are destroyed. Each of the keys are actually +// addresses which don't need to be destroyed. +// +// n.b. Has to be 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, @LocalData)>]; fn cleanup_task_local_map(map_ptr: *libc::c_void) { unsafe { @@ -76,53 +69,52 @@ fn cleanup_task_local_map(map_ptr: *libc::c_void) { // Gets the map from the runtime. Lazily initialises if not done so already. unsafe fn get_local_map(handle: Handle) -> &mut TaskLocalMap { - match handle { - OldHandle(task) => get_task_local_map(task), - NewHandle(local_storage) => get_newsched_local_map(local_storage) - } -} -unsafe fn get_task_local_map(task: *rust_task) -> &mut TaskLocalMap { - - extern fn cleanup_task_local_map_extern_cb(map_ptr: *libc::c_void) { - cleanup_task_local_map(map_ptr); - } - - // Relies on the runtime initialising the pointer to null. - // Note: the map is an owned pointer and is "owned" by TLS. It is moved - // into the tls slot for this task, and then mutable loans are taken from - // this slot to modify the map. - let map_ptr = rt::rust_get_task_local_data(task); - if (*map_ptr).is_null() { - // First time TLS is used, create a new map and set up the necessary - // TLS information for its safe destruction - let map: TaskLocalMap = ~[]; - *map_ptr = cast::transmute(map); - rt::rust_task_local_data_atexit(task, cleanup_task_local_map_extern_cb); - } - return cast::transmute(map_ptr); -} - -unsafe fn get_newsched_local_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); + unsafe fn oldsched_map(task: *rust_task) -> &mut TaskLocalMap { + extern fn cleanup_extern_cb(map_ptr: *libc::c_void) { + cleanup_task_local_map(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()); + + // Relies on the runtime initialising the pointer to null. + // Note: the map is an owned pointer and is "owned" by TLS. It is moved + // into the tls slot for this task, and then mutable loans are taken + // from this slot to modify the map. + let map_ptr = rt::rust_get_task_local_data(task); + if (*map_ptr).is_null() { + // First time TLS is used, create a new map and set up the necessary + // TLS information for its safe destruction let map: TaskLocalMap = ~[]; *map_ptr = cast::transmute(map); - let at_exit_fn: ~fn(*libc::c_void) = |p| cleanup_task_local_map(p); - *at_exit = Some(at_exit_fn); - return cast::transmute(map_ptr); + rt::rust_task_local_data_atexit(task, cleanup_extern_cb); } + return cast::transmute(map_ptr); + } + + 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 { + OldHandle(task) => oldsched_map(task), + NewHandle(local_storage) => newsched_map(local_storage) } } @@ -132,95 +124,83 @@ unsafe fn key_to_key_value(key: LocalDataKey) -> *libc::c_void { } // If returning Some(..), returns with @T with the map's reference. Careful! -unsafe fn local_data_lookup( - map: &mut TaskLocalMap, key: LocalDataKey) - -> Option<(uint, *libc::c_void)> { +unsafe fn local_data_lookup(map: &TaskLocalMap, + key: LocalDataKey) + -> Option<(uint, @T)> +{ + use managed::raw::BoxRepr; let key_value = key_to_key_value(key); for map.iter().enumerate().advance |(i, entry)| { match *entry { - Some((k, data, _)) if k == key_value => { return Some((i, data)); } + Some((k, ref data)) if k == key_value => { + // We now have the correct 'data' as type @LocalData which we + // need to somehow transmute this back to @T. This was + // originally stored into the map as: + // + // let data = @T; + // let element = @data as @LocalData; + // insert(key, element); + // + // This means that the element stored is a 2-word pair (because + // it's a trait). The second element is the vtable (we don't + // need it), and the first element is actually '@@T'. Not only + // is this @@T, but it's a pointer to the base of the @@T (box + // and all), so we have to traverse this to find the actual + // pointer that we want. + let (_vtable, box) = + *cast::transmute::<&@LocalData, &(uint, *BoxRepr)>(data); + let ptr: &@T = cast::transmute(&(*box).data); + return Some((i, *ptr)); + } _ => {} } } return None; } -unsafe fn local_get_helper( - handle: Handle, key: LocalDataKey, - do_pop: bool) -> Option<@T> { - +pub unsafe fn local_pop(handle: Handle, + key: LocalDataKey) -> Option<@T> { let map = get_local_map(handle); - // Interpreturn our findings from the map - do local_data_lookup(map, key).map |result| { - // A reference count magically appears on 'data' out of thin air. It - // was referenced in the local_data box, though, not here, so before - // overwriting the local_data_box we need to give an extra reference. - // We must also give an extra reference when not removing. - let (index, data_ptr) = *result; - let data: @T = cast::transmute(data_ptr); - cast::bump_box_refcount(data); - if do_pop { + match local_data_lookup(map, key) { + Some((index, data)) => { map[index] = None; + Some(data) } - data + None => None } } - -pub unsafe fn local_pop( - handle: Handle, - key: LocalDataKey) -> Option<@T> { - - local_get_helper(handle, key, true) +pub unsafe fn local_get(handle: Handle, + key: LocalDataKey) -> Option<@T> { + match local_data_lookup(get_local_map(handle), key) { + Some((_, data)) => Some(data), + None => None + } } -pub unsafe fn local_get( - handle: Handle, - key: LocalDataKey) -> Option<@T> { - - local_get_helper(handle, key, false) -} - -pub unsafe fn local_set( - handle: Handle, key: LocalDataKey, data: @T) { - +pub unsafe fn local_set(handle: Handle, + key: LocalDataKey, + data: @T) { let map = get_local_map(handle); - // Store key+data as *voids. Data is invisibly referenced once; key isn't. let keyval = key_to_key_value(key); - // We keep the data in two forms: one as an unsafe pointer, so we can get - // it back by casting; another in an existential box, so the reference we - // own on it can be dropped when the box is destroyed. The unsafe pointer - // does not have a reference associated with it, so it may become invalid - // when the box is destroyed. - let data_ptr = *cast::transmute::<&@T, &*libc::c_void>(&data); - let data_box = @data as @LocalData; - // Construct new entry to store in the map. - let new_entry = Some((keyval, data_ptr, data_box)); - // Find a place to put it. + + // 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. + let entry = Some((keyval, @data as @LocalData)); + match local_data_lookup(map, key) { - Some((index, _old_data_ptr)) => { - // Key already had a value set, _old_data_ptr, whose reference - // will get dropped when the local_data box is overwritten. - map[index] = new_entry; - } + Some((index, _)) => { map[index] = entry; } None => { // Find an empty slot. If not, grow the vector. match map.iter().position(|x| x.is_none()) { - Some(empty_index) => { map[empty_index] = new_entry; } - None => { map.push(new_entry); } + Some(empty_index) => { map[empty_index] = entry; } + None => { map.push(entry); } } } } } - -pub unsafe fn local_modify( - handle: Handle, key: LocalDataKey, - modify_fn: &fn(Option<@T>) -> Option<@T>) { - - // Could be more efficient by doing the lookup work, but this is easy. - let newdata = modify_fn(local_pop(handle, key)); - if newdata.is_some() { - local_set(handle, key, newdata.unwrap()); - } -} From 5c3a2e7eeb1e553c7fc06a012862d99094faa03f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 9 Jul 2013 14:52:01 -0700 Subject: [PATCH 3/8] Change TLS to almost be able to contain owned types --- src/libstd/condition.rs | 2 +- src/libstd/local_data.rs | 18 +-- src/libstd/task/local_data_priv.rs | 170 ++++++++++++++++++++--------- src/libstd/task/spawn.rs | 40 +++---- 4 files changed, 144 insertions(+), 86 deletions(-) diff --git a/src/libstd/condition.rs b/src/libstd/condition.rs index 04f2d815d08..89f91820441 100644 --- a/src/libstd/condition.rs +++ b/src/libstd/condition.rs @@ -26,7 +26,7 @@ pub struct Handler { pub struct Condition<'self, T, U> { name: &'static str, - key: local_data::LocalDataKey<'self, Handler> + key: local_data::LocalDataKey<'self, @Handler> } impl<'self, T, U> Condition<'self, T, U> { diff --git a/src/libstd/local_data.rs b/src/libstd/local_data.rs index fbb11dfaa34..77bbe4f5b97 100644 --- a/src/libstd/local_data.rs +++ b/src/libstd/local_data.rs @@ -46,33 +46,27 @@ use task::local_data_priv::{local_get, local_pop, local_set, Handle}; * * These two cases aside, the interface is safe. */ -pub type LocalDataKey<'self,T> = &'self fn:Copy(v: @T); +pub type LocalDataKey<'self,T> = &'self fn:Copy(v: T); /** * Remove a task-local data value from the table, returning the * reference that was originally created to insert it. */ -pub unsafe fn local_data_pop( - key: LocalDataKey) -> Option<@T> { - +pub unsafe fn local_data_pop(key: LocalDataKey) -> Option { local_pop(Handle::new(), key) } /** * Retrieve a task-local data value. It will also be kept alive in the * table until explicitly removed. */ -pub unsafe fn local_data_get( - key: LocalDataKey) -> Option<@T> { - - local_get(Handle::new(), key) +pub unsafe fn local_data_get(key: LocalDataKey<@T>) -> Option<@T> { + local_get(Handle::new(), key, |loc| loc.map(|&x| *x)) } /** * Store a value in task-local data. If this key already has a value, * that value is overwritten (and its destructor is run). */ -pub unsafe fn local_data_set( - key: LocalDataKey, data: @T) { - +pub unsafe fn local_data_set(key: LocalDataKey<@T>, data: @T) { local_set(Handle::new(), key, data) } /** @@ -80,7 +74,7 @@ pub unsafe fn local_data_set( * data is removed (and its reference dropped). */ pub unsafe fn local_data_modify( - key: LocalDataKey, + key: LocalDataKey<@T>, modify_fn: &fn(Option<@T>) -> Option<@T>) { let cur = local_data_pop(key); diff --git a/src/libstd/task/local_data_priv.rs b/src/libstd/task/local_data_priv.rs index 16600ffab06..07eebea4a62 100644 --- a/src/libstd/task/local_data_priv.rs +++ b/src/libstd/task/local_data_priv.rs @@ -13,9 +13,13 @@ use cast; use libc; use local_data::LocalDataKey; +use managed::raw::BoxRepr; use prelude::*; +use ptr; use sys; use task::rt; +use unstable::intrinsics; +use util; use super::rt::rust_task; use rt::task::{Task, LocalStorage}; @@ -47,15 +51,24 @@ trait LocalData {} impl LocalData for T {} // The task-local-map actuall stores all TLS information. Right now it's a list -// of key-value pairs. Each value is an actual Rust type so that when the map is -// destroyed all of the contents are destroyed. Each of the keys are actually -// addresses which don't need to be destroyed. +// of triples of (key, value, loans). The key is a code pointer (right now at +// least), the value is a trait so destruction can work, and the loans value +// is a count of the number of times the value is currently on loan via +// `local_data_get`. +// +// TLS is designed to be able to store owned data, so `local_data_get` must +// return a borrowed pointer to this data. In order to have a proper lifetime, a +// borrowed pointer is insted yielded to a closure specified to the `get` +// function. As a result, it would be unsound to perform `local_data_set` on the +// same key inside of a `local_data_get`, so we ensure at runtime that this does +// not happen. // // n.b. Has to be 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, @LocalData)>]; +type TaskLocalMap = ~[Option<(*libc::c_void, TLSValue, uint)>]; +type TLSValue = @LocalData; fn cleanup_task_local_map(map_ptr: *libc::c_void) { unsafe { @@ -123,35 +136,65 @@ unsafe fn key_to_key_value(key: LocalDataKey) -> *libc::c_void { return pair.code as *libc::c_void; } -// If returning Some(..), returns with @T with the map's reference. Careful! -unsafe fn local_data_lookup(map: &TaskLocalMap, - key: LocalDataKey) - -> Option<(uint, @T)> -{ - use managed::raw::BoxRepr; +unsafe fn transmute_back<'a, T>(data: &'a TLSValue) -> (*BoxRepr, &'a T) { + // Currently, a TLSValue is an '@Trait' instance which means that its actual + // representation is a pair of (vtable, box). Also, because of issue #7673 + // the box actually points to another box which has the data. Hence, to get + // a pointer to the actual value that we're interested in, we decode the + // trait pointer and pass through one layer of boxes to get to the actual + // data we're interested in. + // + // The reference count of the containing @Trait box is already taken care of + // because the TLSValue is owned by the containing TLS map which means that + // the reference count is at least one. Extra protections are then added at + // runtime to ensure that once a loan on a value in TLS has been given out, + // the value isn't modified by another user. + let (_vt, box) = *cast::transmute::<&TLSValue, &(uint, *BoxRepr)>(data); + return (box, cast::transmute(&(*box).data)); +} + +pub unsafe fn local_pop(handle: Handle, + key: LocalDataKey) -> Option { + // If you've never seen horrendously unsafe code written in rust before, + // just feel free to look a bit farther... + let map = get_local_map(handle); let key_value = key_to_key_value(key); - for map.iter().enumerate().advance |(i, entry)| { + + for map.mut_iter().advance |entry| { match *entry { - Some((k, ref data)) if k == key_value => { - // We now have the correct 'data' as type @LocalData which we - // need to somehow transmute this back to @T. This was - // originally stored into the map as: - // - // let data = @T; - // let element = @data as @LocalData; - // insert(key, element); - // - // This means that the element stored is a 2-word pair (because - // it's a trait). The second element is the vtable (we don't - // need it), and the first element is actually '@@T'. Not only - // is this @@T, but it's a pointer to the base of the @@T (box - // and all), so we have to traverse this to find the actual - // pointer that we want. - let (_vtable, box) = - *cast::transmute::<&@LocalData, &(uint, *BoxRepr)>(data); - let ptr: &@T = cast::transmute(&(*box).data); - return Some((i, *ptr)); + Some((k, _, loans)) if k == key_value => { + if loans != 0 { + fail!("TLS value has been loaned via get already"); + } + // 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 => libc::abort(), + }; + + // First, via some various cheats/hacks, we extract the value + // contained within the TLS box. This leaves a big chunk of + // memory which needs to be deallocated now. + let (chunk, inside) = transmute_back(&data); + let inside = cast::transmute_mut(inside); + let ret = ptr::read_ptr(inside); + + // Forget the trait box because we're about to manually + // deallocate the other box. And for my next trick (kids don't + // try this at home), transmute the chunk of @ memory from the + // @-trait box to a pointer to a zero-sized '@' block which will + // then cause it to get properly deallocated, but it won't touch + // any of the uninitialized memory beyond the end. + cast::forget(data); + let chunk: *mut BoxRepr = cast::transmute(chunk); + (*chunk).header.type_desc = + cast::transmute(intrinsics::get_tydesc::<()>()); + let _: @() = cast::transmute(chunk); + + return Some(ret); } _ => {} } @@ -159,28 +202,32 @@ unsafe fn local_data_lookup(map: &TaskLocalMap, return None; } -pub unsafe fn local_pop(handle: Handle, - key: LocalDataKey) -> Option<@T> { +pub unsafe fn local_get(handle: Handle, + key: LocalDataKey, + f: &fn(Option<&T>) -> U) -> U { + // This does in theory take multiple mutable loans on the tls map, but the + // references returned are never removed because the map is only increasing + // in size (it never shrinks). let map = get_local_map(handle); - match local_data_lookup(map, key) { - Some((index, data)) => { - map[index] = None; - Some(data) + let key_value = key_to_key_value(key); + for map.mut_iter().advance |entry| { + match *entry { + Some((k, ref data, ref mut loans)) if k == key_value => { + *loans = *loans + 1; + let (_, val) = transmute_back(data); + let ret = f(Some(val)); + *loans = *loans - 1; + return ret; + } + _ => {} } - None => None - } -} - -pub unsafe fn local_get(handle: Handle, - key: LocalDataKey) -> Option<@T> { - match local_data_lookup(get_local_map(handle), key) { - Some((_, data)) => Some(data), - None => None } + return f(None); } +// FIXME(#7673): This shouldn't require '@', it should use '~' pub unsafe fn local_set(handle: Handle, - key: LocalDataKey, + key: LocalDataKey<@T>, data: @T) { let map = get_local_map(handle); let keyval = key_to_key_value(key); @@ -191,16 +238,31 @@ pub unsafe fn local_set(handle: Handle, // 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. - let entry = Some((keyval, @data as @LocalData)); + let data = @data as @LocalData; - match local_data_lookup(map, key) { - Some((index, _)) => { map[index] = entry; } - None => { - // Find an empty slot. If not, grow the vector. - match map.iter().position(|x| x.is_none()) { - Some(empty_index) => { map[empty_index] = entry; } - None => { map.push(entry); } + // First, try to insert it if we already have it. + for map.mut_iter().advance |entry| { + match *entry { + Some((key, ref mut value, loans)) if key == keyval => { + if loans != 0 { + fail!("TLS value has been loaned via get already"); + } + util::replace(value, data); + return; + } + _ => {} + } + } + // Next, search for an open spot + for map.mut_iter().advance |entry| { + match *entry { + Some(*) => {} + None => { + *entry = Some((keyval, data, 0)); + return; } } } + // Finally push it on the end of the list + map.push(Some((keyval, data, 0))); } diff --git a/src/libstd/task/spawn.rs b/src/libstd/task/spawn.rs index 190485a720a..7fe640dbf8c 100644 --- a/src/libstd/task/spawn.rs +++ b/src/libstd/task/spawn.rs @@ -477,26 +477,28 @@ fn gen_child_taskgroup(linked: bool, supervised: bool) * Step 1. Get spawner's taskgroup info. *##################################################################*/ let spawner_group: @@mut TCB = - match local_get(OldHandle(spawner), taskgroup_key!()) { - None => { - // Main task, doing first spawn ever. Lazily initialise - // here. - let mut members = new_taskset(); - taskset_insert(&mut members, spawner); - let tasks = exclusive(Some(TaskGroupData { - members: members, - descendants: new_taskset(), - })); - // Main task/group has no ancestors, no notifier, etc. - let group = @@mut TCB(spawner, - tasks, - AncestorList(None), - true, - None); - local_set(OldHandle(spawner), taskgroup_key!(), group); - group + do local_get(OldHandle(spawner), taskgroup_key!()) |group| { + match group { + None => { + // Main task, doing first spawn ever. Lazily initialise + // here. + let mut members = new_taskset(); + taskset_insert(&mut members, spawner); + let tasks = exclusive(Some(TaskGroupData { + members: members, + descendants: new_taskset(), + })); + // Main task/group has no ancestors, no notifier, etc. + let group = @@mut TCB(spawner, + tasks, + AncestorList(None), + true, + None); + local_set(OldHandle(spawner), taskgroup_key!(), group); + group + } + Some(&group) => group } - Some(group) => group }; let spawner_group: &mut TCB = *spawner_group; From cb5b9a477ccd2d04f549e1107af350749d414bba Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 9 Jul 2013 17:25:28 -0700 Subject: [PATCH 4/8] Rename local_data methods/types for less keystrokes --- src/libextra/rl.rs | 4 +- src/libextra/sort.rs | 4 +- src/librustc/middle/trans/base.rs | 8 +- src/librustc/middle/trans/context.rs | 6 +- src/librusti/program.rs | 6 +- src/libstd/condition.rs | 17 ++--- src/libstd/local_data.rs | 107 +++++++++++++++------------ src/libstd/os.rs | 4 +- src/libstd/rand.rs | 4 +- src/libstd/task/local_data_priv.rs | 10 +-- src/libsyntax/ast_util.rs | 4 +- src/libsyntax/parse/token.rs | 4 +- 12 files changed, 94 insertions(+), 84 deletions(-) diff --git a/src/libextra/rl.rs b/src/libextra/rl.rs index 693e3ecb53f..210c6593a18 100644 --- a/src/libextra/rl.rs +++ b/src/libextra/rl.rs @@ -72,11 +72,11 @@ fn complete_key(_v: @CompletionCb) {} /// Bind to the main completion callback pub unsafe fn complete(cb: CompletionCb) { - local_data::local_data_set(complete_key, @(cb)); + local_data::set(complete_key, @(cb)); extern fn callback(line: *c_char, completions: *()) { unsafe { - let cb = *local_data::local_data_get(complete_key) + let cb = *local_data::get(complete_key) .get(); do cb(str::raw::from_c_str(line)) |suggestion| { diff --git a/src/libextra/sort.rs b/src/libextra/sort.rs index 68a678869da..50fa9c35d7e 100644 --- a/src/libextra/sort.rs +++ b/src/libextra/sort.rs @@ -1204,11 +1204,11 @@ mod big_tests { #[unsafe_destructor] impl<'self> Drop for LVal<'self> { fn drop(&self) { - let x = unsafe { local_data::local_data_get(self.key) }; + let x = unsafe { local_data::get(self.key) }; match x { Some(@y) => { unsafe { - local_data::local_data_set(self.key, @(y+1)); + local_data::set(self.key, @(y+1)); } } _ => fail!("Expected key to work"), diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 80fc3803ae7..bc22f89da7e 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -92,7 +92,7 @@ fn task_local_insn_key(_v: @~[&'static str]) {} pub fn with_insn_ctxt(blk: &fn(&[&'static str])) { unsafe { - let opt = local_data::local_data_get(task_local_insn_key); + let opt = local_data::get(task_local_insn_key); if opt.is_some() { blk(*opt.unwrap()); } @@ -101,7 +101,7 @@ pub fn with_insn_ctxt(blk: &fn(&[&'static str])) { pub fn init_insn_ctxt() { unsafe { - local_data::local_data_set(task_local_insn_key, @~[]); + local_data::set(task_local_insn_key, @~[]); } } @@ -111,7 +111,7 @@ pub struct _InsnCtxt { _x: () } impl Drop for _InsnCtxt { fn drop(&self) { unsafe { - do local_data::local_data_modify(task_local_insn_key) |c| { + do local_data::modify(task_local_insn_key) |c| { do c.map_consume |ctx| { let mut ctx = copy *ctx; ctx.pop(); @@ -125,7 +125,7 @@ impl Drop for _InsnCtxt { pub fn push_ctxt(s: &'static str) -> _InsnCtxt { debug!("new InsnCtxt: %s", s); unsafe { - do local_data::local_data_modify(task_local_insn_key) |c| { + do local_data::modify(task_local_insn_key) |c| { do c.map_consume |ctx| { let mut ctx = copy *ctx; ctx.push(s); diff --git a/src/librustc/middle/trans/context.rs b/src/librustc/middle/trans/context.rs index 2880c68c1e0..dff365d79d2 100644 --- a/src/librustc/middle/trans/context.rs +++ b/src/librustc/middle/trans/context.rs @@ -241,14 +241,14 @@ impl Drop for CrateContext { fn task_local_llcx_key(_v: @ContextRef) {} pub fn task_llcx() -> ContextRef { - let opt = unsafe { local_data::local_data_get(task_local_llcx_key) }; + let opt = unsafe { local_data::get(task_local_llcx_key) }; *opt.expect("task-local LLVMContextRef wasn't ever set!") } unsafe fn set_task_llcx(c: ContextRef) { - local_data::local_data_set(task_local_llcx_key, @c); + local_data::set(task_local_llcx_key, @c); } unsafe fn unset_task_llcx() { - local_data::local_data_pop(task_local_llcx_key); + local_data::pop(task_local_llcx_key); } diff --git a/src/librusti/program.rs b/src/librusti/program.rs index e15cc04fa9b..3af5e98ebaf 100644 --- a/src/librusti/program.rs +++ b/src/librusti/program.rs @@ -144,7 +144,7 @@ impl Program { let key = ::std::sys::Closure{ code: %? as *(), env: ::std::ptr::null() }; let key = ::std::cast::transmute(key); - *::std::local_data::local_data_get(key).unwrap() + *::std::local_data::get(key).unwrap() };\n", key.code as uint)); // Using this __tls_map handle, deserialize each variable binding that @@ -227,7 +227,7 @@ impl Program { map.insert(copy *name, @copy value.data); } unsafe { - local_data::local_data_set(tls_key, @map); + local_data::set(tls_key, @map); } } @@ -236,7 +236,7 @@ impl Program { /// it updates this cache with the new values of each local variable. pub fn consume_cache(&mut self) { let map = unsafe { - local_data::local_data_pop(tls_key).expect("tls is empty") + local_data::pop(tls_key).expect("tls is empty") }; do map.consume |name, value| { match self.local_vars.find_mut(&name) { diff --git a/src/libstd/condition.rs b/src/libstd/condition.rs index 89f91820441..10900b7172a 100644 --- a/src/libstd/condition.rs +++ b/src/libstd/condition.rs @@ -12,7 +12,6 @@ #[allow(missing_doc)]; -use local_data::{local_data_pop, local_data_set}; use local_data; use prelude::*; @@ -26,14 +25,14 @@ pub struct Handler { pub struct Condition<'self, T, U> { name: &'static str, - key: local_data::LocalDataKey<'self, @Handler> + key: local_data::Key<'self, @Handler> } impl<'self, T, U> Condition<'self, T, U> { pub fn trap(&'self self, h: &'self fn(T) -> U) -> Trap<'self, T, U> { unsafe { let p : *RustClosure = ::cast::transmute(&h); - let prev = local_data::local_data_get(self.key); + let prev = local_data::get(self.key); let h = @Handler { handle: *p, prev: prev }; Trap { cond: self, handler: h } } @@ -46,7 +45,7 @@ impl<'self, T, U> Condition<'self, T, U> { pub fn raise_default(&self, t: T, default: &fn() -> U) -> U { unsafe { - match local_data_pop(self.key) { + match local_data::pop(self.key) { None => { debug!("Condition.raise: found no handler"); default() @@ -55,12 +54,12 @@ impl<'self, T, U> Condition<'self, T, U> { debug!("Condition.raise: found handler"); match handler.prev { None => {} - Some(hp) => local_data_set(self.key, hp) + Some(hp) => local_data::set(self.key, hp) } let handle : &fn(T) -> U = ::cast::transmute(handler.handle); let u = handle(t); - local_data_set(self.key, handler); + local_data::set(self.key, handler); u } } @@ -78,7 +77,7 @@ impl<'self, T, U> Trap<'self, T, U> { unsafe { let _g = Guard { cond: self.cond }; debug!("Trap: pushing handler to TLS"); - local_data_set(self.cond.key, self.handler); + local_data::set(self.cond.key, self.handler); inner() } } @@ -93,12 +92,12 @@ impl<'self, T, U> Drop for Guard<'self, T, U> { fn drop(&self) { unsafe { debug!("Guard: popping handler from TLS"); - let curr = local_data_pop(self.cond.key); + let curr = local_data::pop(self.cond.key); match curr { None => {} Some(h) => match h.prev { None => {} - Some(hp) => local_data_set(self.cond.key, hp) + Some(hp) => local_data::set(self.cond.key, hp) } } } diff --git a/src/libstd/local_data.rs b/src/libstd/local_data.rs index 77bbe4f5b97..d7ab7236b5c 100644 --- a/src/libstd/local_data.rs +++ b/src/libstd/local_data.rs @@ -12,14 +12,28 @@ Task local data management -Allows storing boxes with arbitrary types inside, to be accessed -anywhere within a task, keyed by a pointer to a global finaliser -function. Useful for dynamic variables, singletons, and interfacing -with foreign code with bad callback interfaces. +Allows storing boxes with arbitrary types inside, to be accessed anywhere within +a task, keyed by a pointer to a global finaliser function. Useful for dynamic +variables, singletons, and interfacing with foreign code with bad callback +interfaces. -To use, declare a monomorphic global function at the type to store, -and use it as the 'key' when accessing. See the 'tls' tests below for -examples. +To use, declare a monomorphic (no type parameters) global function at the type +to store, and use it as the 'key' when accessing. + +~~~{.rust} +use std::local_data; + +fn key_int(_: @int) {} +fn key_vector(_: @~[int]) {} + +unsafe { + local_data::set(key_int, @3); + assert!(local_data::get(key_int) == Some(@3)); + + local_data::set(key_vector, @~[3]); + assert!(local_data::get(key_vector).unwrap()[0] == 3); +} +~~~ Casting 'Arcane Sight' reveals an overwhelming aura of Transmutation magic. @@ -46,40 +60,37 @@ use task::local_data_priv::{local_get, local_pop, local_set, Handle}; * * These two cases aside, the interface is safe. */ -pub type LocalDataKey<'self,T> = &'self fn:Copy(v: T); +pub type Key<'self,T> = &'self fn:Copy(v: T); /** * Remove a task-local data value from the table, returning the * reference that was originally created to insert it. */ -pub unsafe fn local_data_pop(key: LocalDataKey) -> Option { +pub unsafe fn pop(key: Key) -> Option { local_pop(Handle::new(), key) } /** * Retrieve a task-local data value. It will also be kept alive in the * table until explicitly removed. */ -pub unsafe fn local_data_get(key: LocalDataKey<@T>) -> Option<@T> { +pub unsafe fn get(key: Key<@T>) -> Option<@T> { local_get(Handle::new(), key, |loc| loc.map(|&x| *x)) } /** * Store a value in task-local data. If this key already has a value, * that value is overwritten (and its destructor is run). */ -pub unsafe fn local_data_set(key: LocalDataKey<@T>, data: @T) { +pub unsafe fn set(key: Key<@T>, data: @T) { local_set(Handle::new(), key, data) } /** * Modify a task-local data value. If the function returns 'None', the * data is removed (and its reference dropped). */ -pub unsafe fn local_data_modify( - key: LocalDataKey<@T>, - modify_fn: &fn(Option<@T>) -> Option<@T>) { - - let cur = local_data_pop(key); - match modify_fn(cur) { - Some(next) => { local_data_set(key, next); } +pub unsafe fn modify(key: Key<@T>, + f: &fn(Option<@T>) -> Option<@T>) { + match f(pop(key)) { + Some(next) => { set(key, next); } None => {} } } @@ -88,19 +99,19 @@ pub unsafe fn local_data_modify( fn test_tls_multitask() { unsafe { fn my_key(_x: @~str) { } - local_data_set(my_key, @~"parent data"); + set(my_key, @~"parent data"); do task::spawn { // TLS shouldn't carry over. - assert!(local_data_get(my_key).is_none()); - local_data_set(my_key, @~"child data"); - assert!(*(local_data_get(my_key).get()) == + assert!(get(my_key).is_none()); + set(my_key, @~"child data"); + assert!(*(get(my_key).get()) == ~"child data"); // should be cleaned up for us } // Must work multiple times - assert!(*(local_data_get(my_key).get()) == ~"parent data"); - assert!(*(local_data_get(my_key).get()) == ~"parent data"); - assert!(*(local_data_get(my_key).get()) == ~"parent data"); + assert!(*(get(my_key).get()) == ~"parent data"); + assert!(*(get(my_key).get()) == ~"parent data"); + assert!(*(get(my_key).get()) == ~"parent data"); } } @@ -108,9 +119,9 @@ fn test_tls_multitask() { fn test_tls_overwrite() { unsafe { fn my_key(_x: @~str) { } - local_data_set(my_key, @~"first data"); - local_data_set(my_key, @~"next data"); // Shouldn't leak. - assert!(*(local_data_get(my_key).get()) == ~"next data"); + set(my_key, @~"first data"); + set(my_key, @~"next data"); // Shouldn't leak. + assert!(*(get(my_key).get()) == ~"next data"); } } @@ -118,10 +129,10 @@ fn test_tls_overwrite() { fn test_tls_pop() { unsafe { fn my_key(_x: @~str) { } - local_data_set(my_key, @~"weasel"); - assert!(*(local_data_pop(my_key).get()) == ~"weasel"); + set(my_key, @~"weasel"); + assert!(*(pop(my_key).get()) == ~"weasel"); // Pop must remove the data from the map. - assert!(local_data_pop(my_key).is_none()); + assert!(pop(my_key).is_none()); } } @@ -129,20 +140,20 @@ fn test_tls_pop() { fn test_tls_modify() { unsafe { fn my_key(_x: @~str) { } - local_data_modify(my_key, |data| { + modify(my_key, |data| { match data { Some(@ref val) => fail!("unwelcome value: %s", *val), None => Some(@~"first data") } }); - local_data_modify(my_key, |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!(*(local_data_pop(my_key).get()) == ~"next data"); + assert!(*(pop(my_key).get()) == ~"next data"); } } @@ -156,7 +167,7 @@ fn test_tls_crust_automorestack_memorial_bug() { // a stack smaller than 1 MB. fn my_key(_x: @~str) { } do task::spawn { - unsafe { local_data_set(my_key, @~"hax"); } + unsafe { set(my_key, @~"hax"); } } } @@ -167,9 +178,9 @@ fn test_tls_multiple_types() { fn int_key(_x: @int) { } do task::spawn { unsafe { - local_data_set(str_key, @~"string data"); - local_data_set(box_key, @@()); - local_data_set(int_key, @42); + set(str_key, @~"string data"); + set(box_key, @@()); + set(int_key, @42); } } } @@ -181,12 +192,12 @@ fn test_tls_overwrite_multiple_types() { fn int_key(_x: @int) { } do task::spawn { unsafe { - local_data_set(str_key, @~"string data"); - local_data_set(int_key, @42); + 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. - local_data_set(int_key, @31337); + set(int_key, @31337); } } } @@ -199,17 +210,17 @@ fn test_tls_cleanup_on_failure() { fn str_key(_x: @~str) { } fn box_key(_x: @@()) { } fn int_key(_x: @int) { } - local_data_set(str_key, @~"parent data"); - local_data_set(box_key, @@()); + set(str_key, @~"parent data"); + set(box_key, @@()); do task::spawn { // spawn_linked - local_data_set(str_key, @~"string data"); - local_data_set(box_key, @@()); - local_data_set(int_key, @42); + set(str_key, @~"string data"); + set(box_key, @@()); + set(int_key, @42); fail!(); } // Not quite nondeterministic. - local_data_set(int_key, @31337); + set(int_key, @31337); fail!(); } } @@ -219,6 +230,6 @@ fn test_static_pointer() { unsafe { fn key(_x: @&'static int) { } static VALUE: int = 0; - local_data_set(key, @&VALUE); + set(key, @&VALUE); } } diff --git a/src/libstd/os.rs b/src/libstd/os.rs index 50acbee697f..3c8294bdfd1 100644 --- a/src/libstd/os.rs +++ b/src/libstd/os.rs @@ -1230,7 +1230,7 @@ fn overridden_arg_key(_v: @OverriddenArgs) {} /// `os::set_args` function. pub fn args() -> ~[~str] { unsafe { - match local_data::local_data_get(overridden_arg_key) { + match local_data::get(overridden_arg_key) { None => real_args(), Some(args) => copy args.val } @@ -1243,7 +1243,7 @@ pub fn args() -> ~[~str] { pub fn set_args(new_args: ~[~str]) { unsafe { let overridden_args = @OverriddenArgs { val: copy new_args }; - local_data::local_data_set(overridden_arg_key, overridden_args); + local_data::set(overridden_arg_key, overridden_args); } } diff --git a/src/libstd/rand.rs b/src/libstd/rand.rs index 02c8694bf76..860a69bd1e0 100644 --- a/src/libstd/rand.rs +++ b/src/libstd/rand.rs @@ -850,13 +850,13 @@ fn tls_rng_state(_v: @@mut IsaacRng) {} pub fn task_rng() -> @mut IsaacRng { let r : Option<@@mut IsaacRng>; unsafe { - r = local_data::local_data_get(tls_rng_state); + r = local_data::get(tls_rng_state); } match r { None => { unsafe { let rng = @@mut IsaacRng::new_seeded(seed()); - local_data::local_data_set(tls_rng_state, rng); + local_data::set(tls_rng_state, rng); *rng } } diff --git a/src/libstd/task/local_data_priv.rs b/src/libstd/task/local_data_priv.rs index 07eebea4a62..17d534cfd03 100644 --- a/src/libstd/task/local_data_priv.rs +++ b/src/libstd/task/local_data_priv.rs @@ -12,7 +12,7 @@ use cast; use libc; -use local_data::LocalDataKey; +use local_data; use managed::raw::BoxRepr; use prelude::*; use ptr; @@ -131,7 +131,7 @@ unsafe fn get_local_map(handle: Handle) -> &mut TaskLocalMap { } } -unsafe fn key_to_key_value(key: LocalDataKey) -> *libc::c_void { +unsafe fn key_to_key_value(key: local_data::Key) -> *libc::c_void { let pair: sys::Closure = cast::transmute(key); return pair.code as *libc::c_void; } @@ -155,7 +155,7 @@ unsafe fn transmute_back<'a, T>(data: &'a TLSValue) -> (*BoxRepr, &'a T) { } pub unsafe fn local_pop(handle: Handle, - key: LocalDataKey) -> Option { + key: local_data::Key) -> Option { // If you've never seen horrendously unsafe code written in rust before, // just feel free to look a bit farther... let map = get_local_map(handle); @@ -203,7 +203,7 @@ pub unsafe fn local_pop(handle: Handle, } pub unsafe fn local_get(handle: Handle, - key: LocalDataKey, + key: local_data::Key, f: &fn(Option<&T>) -> U) -> U { // This does in theory take multiple mutable loans on the tls map, but the // references returned are never removed because the map is only increasing @@ -227,7 +227,7 @@ pub unsafe fn local_get(handle: Handle, // FIXME(#7673): This shouldn't require '@', it should use '~' pub unsafe fn local_set(handle: Handle, - key: LocalDataKey<@T>, + key: local_data::Key<@T>, data: @T) { let map = get_local_map(handle); let keyval = key_to_key_value(key); diff --git a/src/libsyntax/ast_util.rs b/src/libsyntax/ast_util.rs index 78be8e6f180..a224d686cea 100644 --- a/src/libsyntax/ast_util.rs +++ b/src/libsyntax/ast_util.rs @@ -698,10 +698,10 @@ pub fn get_sctable() -> @mut SCTable { let sctable_key = (cast::transmute::<(uint, uint), &fn:Copy(v: @@mut SCTable)>( (-4 as uint, 0u))); - match local_data::local_data_get(sctable_key) { + match local_data::get(sctable_key) { None => { let new_table = @@mut new_sctable_internal(); - local_data::local_data_set(sctable_key,new_table); + local_data::set(sctable_key,new_table); *new_table }, Some(intr) => *intr diff --git a/src/libsyntax/parse/token.rs b/src/libsyntax/parse/token.rs index 09d6ecb40fc..1ddc42b4a4e 100644 --- a/src/libsyntax/parse/token.rs +++ b/src/libsyntax/parse/token.rs @@ -490,11 +490,11 @@ pub fn get_ident_interner() -> @ident_interner { (cast::transmute::<(uint, uint), &fn:Copy(v: @@::parse::token::ident_interner)>( (-3 as uint, 0u))); - match local_data::local_data_get(key) { + match local_data::get(key) { Some(interner) => *interner, None => { let interner = mk_fresh_ident_interner(); - local_data::local_data_set(key, @interner); + local_data::set(key, @interner); interner } } From e3fb7062aa2d7113c4ff4cb41a27bfb637465d57 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 10 Jul 2013 22:14:40 -0700 Subject: [PATCH 5/8] Work around stage0 to remove '@' requirements from TLS --- src/libstd/local_data.rs | 40 ++++ src/libstd/rt/task.rs | 10 +- src/libstd/task/local_data_priv.rs | 124 ++++++------ src/libstd/task/local_data_priv_stage0.rs | 229 ++++++++++++++++++++++ src/libstd/task/mod.rs | 4 + 5 files changed, 333 insertions(+), 74 deletions(-) create mode 100644 src/libstd/task/local_data_priv_stage0.rs diff --git a/src/libstd/local_data.rs b/src/libstd/local_data.rs index d7ab7236b5c..a117d461cfd 100644 --- a/src/libstd/local_data.rs +++ b/src/libstd/local_data.rs @@ -66,6 +66,15 @@ pub type Key<'self,T> = &'self fn:Copy(v: T); * Remove a task-local data value from the table, returning the * reference that was originally created to insert it. */ +#[cfg(stage0)] +pub unsafe fn pop(key: Key<@T>) -> Option<@T> { + local_pop(Handle::new(), key) +} +/** + * Remove a task-local data value from the table, returning the + * reference that was originally created to insert it. + */ +#[cfg(not(stage0))] pub unsafe fn pop(key: Key) -> Option { local_pop(Handle::new(), key) } @@ -73,6 +82,15 @@ pub unsafe fn pop(key: Key) -> Option { * Retrieve a task-local data value. It will also be kept alive in the * table until explicitly removed. */ +#[cfg(stage0)] +pub unsafe fn get(key: Key<@T>) -> Option<@T> { + local_get(Handle::new(), key, |loc| loc.map(|&x| *x)) +} +/** + * Retrieve a task-local data value. It will also be kept alive in the + * table until explicitly removed. + */ +#[cfg(not(stage0))] pub unsafe fn get(key: Key<@T>) -> Option<@T> { local_get(Handle::new(), key, |loc| loc.map(|&x| *x)) } @@ -80,13 +98,23 @@ pub unsafe fn get(key: Key<@T>) -> Option<@T> { * Store a value in task-local data. If this key already has a value, * that value is overwritten (and its destructor is run). */ +#[cfg(stage0)] pub unsafe fn set(key: Key<@T>, data: @T) { local_set(Handle::new(), key, data) } +/** + * Store a value in task-local data. If this key already has a value, + * that value is overwritten (and its destructor is run). + */ +#[cfg(not(stage0))] +pub unsafe fn set(key: Key, data: T) { + local_set(Handle::new(), key, data) +} /** * Modify a task-local data value. If the function returns 'None', the * data is removed (and its reference dropped). */ +#[cfg(stage0)] pub unsafe fn modify(key: Key<@T>, f: &fn(Option<@T>) -> Option<@T>) { match f(pop(key)) { @@ -94,6 +122,18 @@ pub unsafe fn modify(key: Key<@T>, None => {} } } +/** + * Modify a task-local data value. If the function returns 'None', the + * data is removed (and its reference dropped). + */ +#[cfg(not(stage0))] +pub unsafe fn modify(key: Key, + f: &fn(Option) -> Option) { + match f(pop(key)) { + Some(next) => { set(key, next); } + None => {} + } +} #[test] fn test_tls_multitask() { diff --git a/src/libstd/rt/task.rs b/src/libstd/rt/task.rs index 04c1f972a82..55e633e8496 100644 --- a/src/libstd/rt/task.rs +++ b/src/libstd/rt/task.rs @@ -167,15 +167,15 @@ mod test { #[test] fn tls() { - use local_data::*; + use local_data; do run_in_newsched_task() { unsafe { fn key(_x: @~str) { } - local_data_set(key, @~"data"); - assert!(*local_data_get(key).get() == ~"data"); + local_data::set(key, @~"data"); + assert!(*local_data::get(key).get() == ~"data"); fn key2(_x: @~str) { } - local_data_set(key2, @~"data"); - assert!(*local_data_get(key2).get() == ~"data"); + local_data::set(key2, @~"data"); + assert!(*local_data::get(key2).get() == ~"data"); } } } diff --git a/src/libstd/task/local_data_priv.rs b/src/libstd/task/local_data_priv.rs index 17d534cfd03..66a459c23e6 100644 --- a/src/libstd/task/local_data_priv.rs +++ b/src/libstd/task/local_data_priv.rs @@ -13,12 +13,10 @@ use cast; use libc; use local_data; -use managed::raw::BoxRepr; use prelude::*; use ptr; use sys; use task::rt; -use unstable::intrinsics; use util; use super::rt::rust_task; @@ -50,7 +48,7 @@ impl Handle { trait LocalData {} impl LocalData for T {} -// The task-local-map actuall stores all TLS information. Right now it's a list +// The task-local-map actually stores all TLS information. Right now it's a list // of triples of (key, value, loans). The key is a code pointer (right now at // least), the value is a trait so destruction can work, and the loans value // is a count of the number of times the value is currently on loan via @@ -58,7 +56,7 @@ impl LocalData for T {} // // TLS is designed to be able to store owned data, so `local_data_get` must // return a borrowed pointer to this data. In order to have a proper lifetime, a -// borrowed pointer is insted yielded to a closure specified to the `get` +// borrowed pointer is instead yielded to a closure specified to the `get` // function. As a result, it would be unsound to perform `local_data_set` on the // same key inside of a `local_data_get`, so we ensure at runtime that this does // not happen. @@ -68,7 +66,7 @@ impl LocalData for T {} // 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, uint)>]; -type TLSValue = @LocalData; +type TLSValue = ~LocalData:; fn cleanup_task_local_map(map_ptr: *libc::c_void) { unsafe { @@ -136,28 +134,8 @@ unsafe fn key_to_key_value(key: local_data::Key) -> *libc::c_void return pair.code as *libc::c_void; } -unsafe fn transmute_back<'a, T>(data: &'a TLSValue) -> (*BoxRepr, &'a T) { - // Currently, a TLSValue is an '@Trait' instance which means that its actual - // representation is a pair of (vtable, box). Also, because of issue #7673 - // the box actually points to another box which has the data. Hence, to get - // a pointer to the actual value that we're interested in, we decode the - // trait pointer and pass through one layer of boxes to get to the actual - // data we're interested in. - // - // The reference count of the containing @Trait box is already taken care of - // because the TLSValue is owned by the containing TLS map which means that - // the reference count is at least one. Extra protections are then added at - // runtime to ensure that once a loan on a value in TLS has been given out, - // the value isn't modified by another user. - let (_vt, box) = *cast::transmute::<&TLSValue, &(uint, *BoxRepr)>(data); - - return (box, cast::transmute(&(*box).data)); -} - pub unsafe fn local_pop(handle: Handle, key: local_data::Key) -> Option { - // If you've never seen horrendously unsafe code written in rust before, - // just feel free to look a bit farther... let map = get_local_map(handle); let key_value = key_to_key_value(key); @@ -175,25 +153,23 @@ pub unsafe fn local_pop(handle: Handle, None => libc::abort(), }; - // First, via some various cheats/hacks, we extract the value - // contained within the TLS box. This leaves a big chunk of - // memory which needs to be deallocated now. - let (chunk, inside) = transmute_back(&data); - let inside = cast::transmute_mut(inside); - let ret = ptr::read_ptr(inside); + // 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); - // Forget the trait box because we're about to manually - // deallocate the other box. And for my next trick (kids don't - // try this at home), transmute the chunk of @ memory from the - // @-trait box to a pointer to a zero-sized '@' block which will - // then cause it to get properly deallocated, but it won't touch - // any of the uninitialized memory beyond the end. - cast::forget(data); - let chunk: *mut BoxRepr = cast::transmute(chunk); - (*chunk).header.type_desc = - cast::transmute(intrinsics::get_tydesc::<()>()); - let _: @() = cast::transmute(chunk); + // 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); } _ => {} @@ -213,9 +189,17 @@ pub unsafe fn local_get(handle: Handle, for map.mut_iter().advance |entry| { match *entry { Some((k, ref data, ref mut loans)) if k == key_value => { + let ret; *loans = *loans + 1; - let (_, val) = transmute_back(data); - let ret = f(Some(val)); + // 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)); + } + } *loans = *loans - 1; return ret; } @@ -225,44 +209,46 @@ pub unsafe fn local_get(handle: Handle, return f(None); } -// FIXME(#7673): This shouldn't require '@', it should use '~' pub unsafe fn local_set(handle: Handle, - key: local_data::Key<@T>, - data: @T) { + key: local_data::Key, + 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 + // 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. - let data = @data as @LocalData; + // + // FIXME(#7673): This should be "~data as ~LocalData" (without the colon at + // the end, and only one sigil) + let data = ~~data as ~LocalData:; - // First, try to insert it if we already have it. - for map.mut_iter().advance |entry| { - match *entry { - Some((key, ref mut value, loans)) if key == keyval => { - if loans != 0 { - fail!("TLS value has been loaned via get already"); + fn insertion_position(map: &mut TaskLocalMap, + key: *libc::c_void) -> Option { + // First see if the map contains this key already + let curspot = map.iter().position(|entry| { + match *entry { + Some((ekey, _, loans)) if key == ekey => { + if loans != 0 { + fail!("TLS value has been loaned via get already"); + } + true } - util::replace(value, data); - return; + _ => 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()) } } - // Next, search for an open spot - for map.mut_iter().advance |entry| { - match *entry { - Some(*) => {} - None => { - *entry = Some((keyval, data, 0)); - return; - } - } + + match insertion_position(map, keyval) { + Some(i) => { map[i] = Some((keyval, data, 0)); } + None => { map.push(Some((keyval, data, 0))); } } - // Finally push it on the end of the list - map.push(Some((keyval, data, 0))); } diff --git a/src/libstd/task/local_data_priv_stage0.rs b/src/libstd/task/local_data_priv_stage0.rs new file mode 100644 index 00000000000..fe80ec06c69 --- /dev/null +++ b/src/libstd/task/local_data_priv_stage0.rs @@ -0,0 +1,229 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[allow(missing_doc)]; + +use cast; +use cmp::Eq; +use libc; +use local_data; +use prelude::*; +use sys; +use task::rt; + +use super::rt::rust_task; +use rt::task::{Task, LocalStorage}; + +pub enum Handle { + OldHandle(*rust_task), + NewHandle(*mut LocalStorage) +} + +impl Handle { + pub fn new() -> Handle { + use rt::{context, OldTaskContext}; + use rt::local::Local; + unsafe { + match context() { + OldTaskContext => { + OldHandle(rt::rust_get_task()) + } + _ => { + let task = Local::unsafe_borrow::(); + NewHandle(&mut (*task).storage) + } + } + } + } +} + +pub trait LocalData { } +impl LocalData for @T { } + +impl Eq for @LocalData { + fn eq(&self, other: &@LocalData) -> bool { + unsafe { + let ptr_a: &(uint, uint) = cast::transmute(self); + let ptr_b: &(uint, uint) = cast::transmute(other); + return ptr_a == ptr_b; + } + } + fn ne(&self, other: &@LocalData) -> bool { !(*self).eq(other) } +} + +// If TLS is used heavily in future, this could be made more efficient with a +// proper map. +type TaskLocalElement = (*libc::c_void, *libc::c_void, @LocalData); +// Has to be a pointer at outermost layer; the foreign call returns void *. +type TaskLocalMap = ~[Option]; + +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 { + match handle { + OldHandle(task) => get_task_local_map(task), + NewHandle(local_storage) => get_newsched_local_map(local_storage) + } +} + +unsafe fn get_task_local_map(task: *rust_task) -> &mut TaskLocalMap { + + extern fn cleanup_task_local_map_extern_cb(map_ptr: *libc::c_void) { + cleanup_task_local_map(map_ptr); + } + + // Relies on the runtime initialising the pointer to null. + // Note: the map is an owned pointer and is "owned" by TLS. It is moved + // into the tls slot for this task, and then mutable loans are taken from + // this slot to modify the map. + let map_ptr = rt::rust_get_task_local_data(task); + if (*map_ptr).is_null() { + // First time TLS is used, create a new map and set up the necessary + // TLS information for its safe destruction + let map: TaskLocalMap = ~[]; + *map_ptr = cast::transmute(map); + rt::rust_task_local_data_atexit(task, cleanup_task_local_map_extern_cb); + } + return cast::transmute(map_ptr); +} + +unsafe fn get_newsched_local_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); + } + } +} + +unsafe fn key_to_key_value(key: local_data::Key<@T>) -> *libc::c_void { + let pair: sys::Closure = cast::transmute(key); + return pair.code as *libc::c_void; +} + +// If returning Some(..), returns with @T with the map's reference. Careful! +unsafe fn local_data_lookup( + map: &mut TaskLocalMap, key: local_data::Key<@T>) + -> Option<(uint, *libc::c_void)> { + + let key_value = key_to_key_value(key); + for map.iter().enumerate().advance |(i, entry)| { + match *entry { + Some((k, data, _)) if k == key_value => { return Some((i, data)); } + _ => {} + } + } + return None; +} + +unsafe fn local_get_helper( + handle: Handle, key: local_data::Key<@T>, + do_pop: bool) -> Option<@T> { + + let map = get_local_map(handle); + // Interpreturn our findings from the map + do local_data_lookup(map, key).map |result| { + // A reference count magically appears on 'data' out of thin air. It + // was referenced in the local_data box, though, not here, so before + // overwriting the local_data_box we need to give an extra reference. + // We must also give an extra reference when not removing. + let (index, data_ptr) = *result; + let data: @T = cast::transmute(data_ptr); + cast::bump_box_refcount(data); + if do_pop { + map[index] = None; + } + data + } +} + + +pub unsafe fn local_pop( + handle: Handle, + key: local_data::Key<@T>) -> Option<@T> { + + local_get_helper(handle, key, true) +} + +pub unsafe fn local_get( + handle: Handle, + key: local_data::Key<@T>, + f: &fn(Option<&@T>) -> U) -> U { + + match local_get_helper(handle, key, false) { + Some(ref x) => f(Some(x)), + None => f(None) + } +} + +pub unsafe fn local_set( + handle: Handle, key: local_data::Key<@T>, data: @T) { + + let map = get_local_map(handle); + // Store key+data as *voids. Data is invisibly referenced once; key isn't. + let keyval = key_to_key_value(key); + // We keep the data in two forms: one as an unsafe pointer, so we can get + // it back by casting; another in an existential box, so the reference we + // own on it can be dropped when the box is destroyed. The unsafe pointer + // does not have a reference associated with it, so it may become invalid + // when the box is destroyed. + let data_ptr = *cast::transmute::<&@T, &*libc::c_void>(&data); + let data_box = @data as @LocalData; + // Construct new entry to store in the map. + let new_entry = Some((keyval, data_ptr, data_box)); + // Find a place to put it. + match local_data_lookup(map, key) { + Some((index, _old_data_ptr)) => { + // Key already had a value set, _old_data_ptr, whose reference + // will get dropped when the local_data box is overwritten. + map[index] = new_entry; + } + None => { + // Find an empty slot. If not, grow the vector. + match map.iter().position(|x| x.is_none()) { + Some(empty_index) => { map[empty_index] = new_entry; } + None => { map.push(new_entry); } + } + } + } +} + +pub unsafe fn local_modify( + handle: Handle, key: local_data::Key<@T>, + modify_fn: &fn(Option<@T>) -> Option<@T>) { + + // Could be more efficient by doing the lookup work, but this is easy. + let newdata = modify_fn(local_pop(handle, key)); + if newdata.is_some() { + local_set(handle, key, newdata.unwrap()); + } +} diff --git a/src/libstd/task/mod.rs b/src/libstd/task/mod.rs index a8e8cfd163a..b012a834ed0 100644 --- a/src/libstd/task/mod.rs +++ b/src/libstd/task/mod.rs @@ -54,6 +54,10 @@ use util; #[cfg(test)] use ptr; #[cfg(test)] use task; +#[cfg(stage0)] +#[path="local_data_priv_stage0.rs"] +mod local_data_priv; +#[cfg(not(stage0))] mod local_data_priv; pub mod rt; pub mod spawn; From 11c63eaad2311bbeea67ec9a9300686dbe400e23 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 11 Jul 2013 00:19:23 -0700 Subject: [PATCH 6/8] Fix a soundness problem with `get` --- src/libstd/local_data.rs | 8 +++ src/libstd/task/local_data_priv.rs | 102 ++++++++++++++++++++--------- 2 files changed, 79 insertions(+), 31 deletions(-) diff --git a/src/libstd/local_data.rs b/src/libstd/local_data.rs index a117d461cfd..711ed15fa9d 100644 --- a/src/libstd/local_data.rs +++ b/src/libstd/local_data.rs @@ -273,3 +273,11 @@ fn test_static_pointer() { set(key, @&VALUE); } } + +#[test] +fn test_owned() { + unsafe { + fn key(_x: ~int) { } + set(key, ~1); + } +} diff --git a/src/libstd/task/local_data_priv.rs b/src/libstd/task/local_data_priv.rs index 66a459c23e6..42cfcbc16db 100644 --- a/src/libstd/task/local_data_priv.rs +++ b/src/libstd/task/local_data_priv.rs @@ -48,23 +48,36 @@ impl Handle { trait LocalData {} impl LocalData for T {} -// The task-local-map actually stores all TLS information. Right now it's a list -// of triples of (key, value, loans). The key is a code pointer (right now at -// least), the value is a trait so destruction can work, and the loans value -// is a count of the number of times the value is currently on loan via -// `local_data_get`. +// 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. // -// TLS is designed to be able to store owned data, so `local_data_get` must -// return a borrowed pointer to this data. In order to have a proper lifetime, a -// borrowed pointer is instead yielded to a closure specified to the `get` -// function. As a result, it would be unsound to perform `local_data_set` on the -// same key inside of a `local_data_get`, so we ensure at runtime that this does -// not happen. +// 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. // -// n.b. Has to be a pointer at outermost layer; the foreign call returns void *. +// 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. +// a proper map. type TaskLocalMap = ~[Option<(*libc::c_void, TLSValue, uint)>]; type TLSValue = ~LocalData:; @@ -181,32 +194,59 @@ pub unsafe fn local_pop(handle: Handle, pub unsafe fn local_get(handle: Handle, key: local_data::Key, f: &fn(Option<&T>) -> U) -> U { - // This does in theory take multiple mutable loans on the tls map, but the - // references returned are never removed because the map is only increasing - // in size (it never shrinks). + // 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); - for map.mut_iter().advance |entry| { + + let pos = map.iter().position(|entry| { match *entry { - Some((k, ref data, ref mut loans)) if k == key_value => { - let ret; - *loans = *loans + 1; - // 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)); + Some((k, _, _)) if k == key_value => true, _ => false + } + }); + match pos { + None => { return f(None); } + Some(i) => { + let ret; + match map[i] { + Some((_, ref data, ref mut loans)) => { + *loans = *loans + 1; + // 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)); + } } } - *loans = *loans - 1; - return ret; + _ => libc::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. + match map[i] { + Some((_, _, ref mut loans)) => { *loans = *loans - 1; } + None => { libc::abort(); } + } + return ret; } } - return f(None); } pub unsafe fn local_set(handle: Handle, From f9bf69d253e43f9caf279953876d44f6219e71de Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 11 Jul 2013 00:35:43 -0700 Subject: [PATCH 7/8] Remove all external requirements of `@` from TLS Closes #6004 --- src/libextra/rl.rs | 2 +- src/libextra/sort.rs | 2 +- src/librustc/middle/trans/base.rs | 2 +- src/librustc/middle/trans/context.rs | 2 +- src/librusti/program.rs | 2 +- src/libstd/condition.rs | 2 +- src/libstd/local_data.rs | 22 +++++++++++----------- src/libstd/os.rs | 2 +- src/libstd/rand.rs | 2 +- src/libstd/rt/task.rs | 4 ++-- src/libsyntax/ast_util.rs | 2 +- src/libsyntax/parse/token.rs | 2 +- 12 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/libextra/rl.rs b/src/libextra/rl.rs index 210c6593a18..c1eeb5005b2 100644 --- a/src/libextra/rl.rs +++ b/src/libextra/rl.rs @@ -76,7 +76,7 @@ pub unsafe fn complete(cb: CompletionCb) { extern fn callback(line: *c_char, completions: *()) { unsafe { - let cb = *local_data::get(complete_key) + let cb = *local_data::get(complete_key, |k| k.map(|&k| *k)) .get(); do cb(str::raw::from_c_str(line)) |suggestion| { diff --git a/src/libextra/sort.rs b/src/libextra/sort.rs index 50fa9c35d7e..d4d6162a919 100644 --- a/src/libextra/sort.rs +++ b/src/libextra/sort.rs @@ -1204,7 +1204,7 @@ mod big_tests { #[unsafe_destructor] impl<'self> Drop for LVal<'self> { fn drop(&self) { - let x = unsafe { local_data::get(self.key) }; + let x = unsafe { local_data::get(self.key, |k| k.map(|&k| *k)) }; match x { Some(@y) => { unsafe { diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index bc22f89da7e..dc62206fb34 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -92,7 +92,7 @@ fn task_local_insn_key(_v: @~[&'static str]) {} pub fn with_insn_ctxt(blk: &fn(&[&'static str])) { unsafe { - let opt = local_data::get(task_local_insn_key); + let opt = local_data::get(task_local_insn_key, |k| k.map(|&k| *k)); if opt.is_some() { blk(*opt.unwrap()); } diff --git a/src/librustc/middle/trans/context.rs b/src/librustc/middle/trans/context.rs index dff365d79d2..77a565e675f 100644 --- a/src/librustc/middle/trans/context.rs +++ b/src/librustc/middle/trans/context.rs @@ -241,7 +241,7 @@ impl Drop for CrateContext { fn task_local_llcx_key(_v: @ContextRef) {} pub fn task_llcx() -> ContextRef { - let opt = unsafe { local_data::get(task_local_llcx_key) }; + let opt = unsafe { local_data::get(task_local_llcx_key, |k| k.map(|&k| *k)) }; *opt.expect("task-local LLVMContextRef wasn't ever set!") } diff --git a/src/librusti/program.rs b/src/librusti/program.rs index 3af5e98ebaf..03a48117cd4 100644 --- a/src/librusti/program.rs +++ b/src/librusti/program.rs @@ -144,7 +144,7 @@ impl Program { let key = ::std::sys::Closure{ code: %? as *(), env: ::std::ptr::null() }; let key = ::std::cast::transmute(key); - *::std::local_data::get(key).unwrap() + *::std::local_data::get(key, |k| k.map(|&x| *x)).unwrap() };\n", key.code as uint)); // Using this __tls_map handle, deserialize each variable binding that diff --git a/src/libstd/condition.rs b/src/libstd/condition.rs index 10900b7172a..d6d09527f83 100644 --- a/src/libstd/condition.rs +++ b/src/libstd/condition.rs @@ -32,7 +32,7 @@ impl<'self, T, U> Condition<'self, T, U> { pub fn trap(&'self self, h: &'self fn(T) -> U) -> Trap<'self, T, U> { unsafe { let p : *RustClosure = ::cast::transmute(&h); - let prev = local_data::get(self.key); + let prev = local_data::get(self.key, |k| k.map(|&x| *x)); let h = @Handler { handle: *p, prev: prev }; Trap { cond: self, handler: h } } diff --git a/src/libstd/local_data.rs b/src/libstd/local_data.rs index 711ed15fa9d..fa981d273e2 100644 --- a/src/libstd/local_data.rs +++ b/src/libstd/local_data.rs @@ -83,16 +83,16 @@ pub unsafe fn pop(key: Key) -> Option { * table until explicitly removed. */ #[cfg(stage0)] -pub unsafe fn get(key: Key<@T>) -> Option<@T> { - local_get(Handle::new(), key, |loc| loc.map(|&x| *x)) +pub unsafe fn get(key: Key<@T>, f: &fn(Option<&@T>) -> U) -> U { + local_get(Handle::new(), key, f) } /** * Retrieve a task-local data value. It will also be kept alive in the * table until explicitly removed. */ #[cfg(not(stage0))] -pub unsafe fn get(key: Key<@T>) -> Option<@T> { - local_get(Handle::new(), key, |loc| loc.map(|&x| *x)) +pub unsafe fn get(key: Key, f: &fn(Option<&T>) -> U) -> U { + local_get(Handle::new(), key, f) } /** * Store a value in task-local data. If this key already has a value, @@ -142,16 +142,16 @@ fn test_tls_multitask() { set(my_key, @~"parent data"); do task::spawn { // TLS shouldn't carry over. - assert!(get(my_key).is_none()); + assert!(get(my_key, |k| k.map(|&k| *k)).is_none()); set(my_key, @~"child data"); - assert!(*(get(my_key).get()) == + assert!(*(get(my_key, |k| k.map(|&k| *k)).get()) == ~"child data"); // should be cleaned up for us } // Must work multiple times - assert!(*(get(my_key).get()) == ~"parent data"); - assert!(*(get(my_key).get()) == ~"parent data"); - assert!(*(get(my_key).get()) == ~"parent data"); + assert!(*(get(my_key, |k| k.map(|&k| *k)).get()) == ~"parent data"); + assert!(*(get(my_key, |k| k.map(|&k| *k)).get()) == ~"parent data"); + assert!(*(get(my_key, |k| k.map(|&k| *k)).get()) == ~"parent data"); } } @@ -161,7 +161,7 @@ fn test_tls_overwrite() { fn my_key(_x: @~str) { } set(my_key, @~"first data"); set(my_key, @~"next data"); // Shouldn't leak. - assert!(*(get(my_key).get()) == ~"next data"); + assert!(*(get(my_key, |k| k.map(|&k| *k)).get()) == ~"next data"); } } @@ -170,7 +170,7 @@ fn test_tls_pop() { unsafe { fn my_key(_x: @~str) { } set(my_key, @~"weasel"); - assert!(*(pop(my_key).get()) == ~"weasel"); + assert!(*(pop(my_key, |k| k.map(|&k| *k)).get()) == ~"weasel"); // Pop must remove the data from the map. assert!(pop(my_key).is_none()); } diff --git a/src/libstd/os.rs b/src/libstd/os.rs index 3c8294bdfd1..2e511f91b5a 100644 --- a/src/libstd/os.rs +++ b/src/libstd/os.rs @@ -1230,7 +1230,7 @@ fn overridden_arg_key(_v: @OverriddenArgs) {} /// `os::set_args` function. pub fn args() -> ~[~str] { unsafe { - match local_data::get(overridden_arg_key) { + match local_data::get(overridden_arg_key, |k| k.map(|&k| *k)) { None => real_args(), Some(args) => copy args.val } diff --git a/src/libstd/rand.rs b/src/libstd/rand.rs index 860a69bd1e0..bc80bd9dc6a 100644 --- a/src/libstd/rand.rs +++ b/src/libstd/rand.rs @@ -850,7 +850,7 @@ fn tls_rng_state(_v: @@mut IsaacRng) {} pub fn task_rng() -> @mut IsaacRng { let r : Option<@@mut IsaacRng>; unsafe { - r = local_data::get(tls_rng_state); + r = local_data::get(tls_rng_state, |k| k.map(|&k| *k)); } match r { None => { diff --git a/src/libstd/rt/task.rs b/src/libstd/rt/task.rs index 55e633e8496..2100e71e9fd 100644 --- a/src/libstd/rt/task.rs +++ b/src/libstd/rt/task.rs @@ -172,10 +172,10 @@ mod test { unsafe { fn key(_x: @~str) { } local_data::set(key, @~"data"); - assert!(*local_data::get(key).get() == ~"data"); + assert!(*local_data::get(key, |k| k.map(|&k| *k)).get() == ~"data"); fn key2(_x: @~str) { } local_data::set(key2, @~"data"); - assert!(*local_data::get(key2).get() == ~"data"); + assert!(*local_data::get(key2, |k| k.map(|&k| *k)).get() == ~"data"); } } } diff --git a/src/libsyntax/ast_util.rs b/src/libsyntax/ast_util.rs index a224d686cea..1942cb6ad56 100644 --- a/src/libsyntax/ast_util.rs +++ b/src/libsyntax/ast_util.rs @@ -698,7 +698,7 @@ pub fn get_sctable() -> @mut SCTable { let sctable_key = (cast::transmute::<(uint, uint), &fn:Copy(v: @@mut SCTable)>( (-4 as uint, 0u))); - match local_data::get(sctable_key) { + match local_data::get(sctable_key, |k| k.map(|&k| *k)) { None => { let new_table = @@mut new_sctable_internal(); local_data::set(sctable_key,new_table); diff --git a/src/libsyntax/parse/token.rs b/src/libsyntax/parse/token.rs index 1ddc42b4a4e..46e0ef32321 100644 --- a/src/libsyntax/parse/token.rs +++ b/src/libsyntax/parse/token.rs @@ -490,7 +490,7 @@ pub fn get_ident_interner() -> @ident_interner { (cast::transmute::<(uint, uint), &fn:Copy(v: @@::parse::token::ident_interner)>( (-3 as uint, 0u))); - match local_data::get(key) { + match local_data::get(key, |k| k.map(|&k| *k)) { Some(interner) => *interner, None => { let interner = mk_fresh_ident_interner(); From a15c1b4464099fa65ec5da389381db83c22801ec Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 11 Jul 2013 01:03:37 -0700 Subject: [PATCH 8/8] Fix tests --- src/librusti/program.rs | 8 ++++---- src/libstd/local_data.rs | 2 +- src/test/compile-fail/core-tls-store-pointer.rs | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/librusti/program.rs b/src/librusti/program.rs index 03a48117cd4..716c7a2481e 100644 --- a/src/librusti/program.rs +++ b/src/librusti/program.rs @@ -58,7 +58,7 @@ struct LocalVariable { } type LocalCache = @mut HashMap<~str, @~[u8]>; -fn tls_key(_k: @LocalCache) {} +fn tls_key(_k: LocalCache) {} impl Program { pub fn new() -> Program { @@ -132,7 +132,7 @@ impl Program { "); let key: sys::Closure = unsafe { - let tls_key: &'static fn(@LocalCache) = tls_key; + let tls_key: &'static fn(LocalCache) = tls_key; cast::transmute(tls_key) }; // First, get a handle to the tls map which stores all the local @@ -144,7 +144,7 @@ impl Program { let key = ::std::sys::Closure{ code: %? as *(), env: ::std::ptr::null() }; let key = ::std::cast::transmute(key); - *::std::local_data::get(key, |k| k.map(|&x| *x)).unwrap() + ::std::local_data::get(key, |k| k.map(|&x| *x)).unwrap() };\n", key.code as uint)); // Using this __tls_map handle, deserialize each variable binding that @@ -227,7 +227,7 @@ impl Program { map.insert(copy *name, @copy value.data); } unsafe { - local_data::set(tls_key, @map); + local_data::set(tls_key, map); } } diff --git a/src/libstd/local_data.rs b/src/libstd/local_data.rs index fa981d273e2..b241de88700 100644 --- a/src/libstd/local_data.rs +++ b/src/libstd/local_data.rs @@ -170,7 +170,7 @@ fn test_tls_pop() { unsafe { fn my_key(_x: @~str) { } set(my_key, @~"weasel"); - assert!(*(pop(my_key, |k| k.map(|&k| *k)).get()) == ~"weasel"); + assert!(*(pop(my_key).get()) == ~"weasel"); // Pop must remove the data from the map. assert!(pop(my_key).is_none()); } diff --git a/src/test/compile-fail/core-tls-store-pointer.rs b/src/test/compile-fail/core-tls-store-pointer.rs index 63bbaf80177..13c99669228 100644 --- a/src/test/compile-fail/core-tls-store-pointer.rs +++ b/src/test/compile-fail/core-tls-store-pointer.rs @@ -10,12 +10,12 @@ // Testing that we can't store a borrowed pointer it task-local storage -use std::local_data::*; +use std::local_data; fn key(_x: @&int) { } fn main() { unsafe { - local_data_set(key, @&0); //~ ERROR does not fulfill `'static` + local_data::set(key, @&0); //~ ERROR does not fulfill `'static` } }