Store Header Items behind an RWLock

This removes the need to receive `&mut self` in `get_ref` and `get.`

Since the interior mutability of the RWLock is needed only once,
it is safe to change the lifetime of the pointer given by read locks
as by then all mutation has been done.

Since `set` still requires `&mut self` there is no way to use the interior
mutability of the RWLock to modify an existing `&`-reference. However,
the use of interior mutability in `get_ref` means that `get_raw` is now
actually an unsafe operation because the (now `*const`) pointer could be
invalidated by a subsequent call to `get_ref.`

Fixes #47

src/header/mod.rs

@@ -13,6 +13,7 @@ use std::raw::TraitObject;
 use std::str::{from_utf8, SendStr, Slice, Owned};
 use std::string::raw;
 use std::collections::hashmap::{HashMap, Entries, Occupied, Vacant};
+use std::sync::RWLock;
 
 use uany::UncheckedAnyDowncast;
 use typeable::Typeable;
@@ -24,7 +25,7 @@ use {HttpResult};
 pub mod common;
 
 /// A trait for any object that will represent a header field and value.
-pub trait Header: Typeable {
+pub trait Header: Typeable + Send + Sync {
     /// Returns the name of the header field this belongs to.
     ///
     /// The market `Option` is to hint to the type system which implementation
@@ -68,7 +69,7 @@ fn header_name<T: Header>() -> &'static str {
 
 /// A map of header fields on requests and responses.
 pub struct Headers {
-    data: HashMap<CaseInsensitive, Item>
+    data: HashMap<CaseInsensitive, RWLock<Item>>
 }
 
 impl Headers {
@@ -92,13 +93,14 @@ impl Headers {
                         raw::from_utf8(name)
                     };
 
-                    let item = match headers.data.entry(CaseInsensitive(Owned(name))) {
+                    let name = CaseInsensitive(Owned(name));
-                        Vacant(entry) => entry.set(Raw(vec![])),
+                    let item = match headers.data.entry(name) {
+                        Vacant(entry) => entry.set(RWLock::new(Raw(vec![]))),
                         Occupied(entry) => entry.into_mut()
                     };
 
-                    match *item {
+                    match &mut *item.write() {
-                        Raw(ref mut raw) => raw.push(value),
+                        &Raw(ref mut raw) => raw.push(value),
                         // Unreachable
                         _ => {}
                     };
@@ -113,7 +115,7 @@ impl Headers {
     ///
     /// The field is determined by the type of the value being set.
     pub fn set<H: Header>(&mut self, value: H) {
-        self.data.insert(CaseInsensitive(Slice(header_name::<H>())), Typed(box value as Box<Header>));
+        self.data.insert(CaseInsensitive(Slice(header_name::<H>())), RWLock::new(Typed(box value as Box<Header + Send + Sync>)));
     }
 
     /// Get a clone of the header field's value, if it exists.
@@ -126,7 +128,7 @@ impl Headers {
     /// # let mut headers = Headers::new();
     /// let content_type = headers.get::<ContentType>();
     /// ```
-    pub fn get<H: Header + Clone>(&mut self) -> Option<H> {
+    pub fn get<H: Header + Clone>(&self) -> Option<H> {
         self.get_ref().map(|v: &H| v.clone())
     }
 
@@ -136,26 +138,29 @@ impl Headers {
     /// If the header field has already been parsed into a typed header,
     /// then you *must* access it through that representation.
     ///
+    /// This operation is unsafe because the raw representation can be
+    /// invalidated by lasting too long or by the header being parsed
+    /// while you still have a reference to the data.
+    ///
     /// Example:
     /// ```
     /// # use hyper::header::Headers;
     /// # let mut headers = Headers::new();
     /// let raw_content_type = unsafe { headers.get_raw("content-type") };
     /// ```
-    pub unsafe fn get_raw(&self, name: &'static str) -> Option<&[Vec<u8>]> {
+    pub unsafe fn get_raw(&self, name: &'static str) -> Option<*const [Vec<u8>]> {
         self.data.find(&CaseInsensitive(Slice(name))).and_then(|item| {
-            match *item {
+            match *item.read() {
-                Raw(ref raw) => Some(raw.as_slice()),
+                Raw(ref raw) => Some(raw.as_slice() as *const [Vec<u8>]),
                 _ => None
             }
         })
     }
 
     /// Get a reference to the header field's value, if it exists.
-    pub fn get_ref<H: Header>(&mut self) -> Option<&H> {
+    pub fn get_ref<H: Header>(&self) -> Option<&H> {
-        self.data.find_mut(&CaseInsensitive(Slice(header_name::<H>()))).and_then(|item| {
+        self.data.find(&CaseInsensitive(Slice(header_name::<H>()))).and_then(|item| {
-            debug!("get_ref, name={}, val={}", header_name::<H>(), item);
+            let header = match *item.read() {
-            let header = match *item {
                 // Huge borrowck hack here, should be refactored to just return here.
                 Typed(ref typed) if typed.is::<H>() => None,
                 // Typed, wrong type
@@ -170,7 +175,7 @@ impl Headers {
 
             match header {
                 Some(header) => {
-                    *item = Typed(box header as Box<Header>);
+                    *item.write() = Typed(box header as Box<Header + Send + Sync>);
                     Some(item)
                 },
                 None => {
@@ -178,14 +183,15 @@ impl Headers {
                 }
             }
         }).and_then(|item| {
-            debug!("downcasting {}", item);
+            let read = item.read();
-            let ret = match *item {
+            debug!("downcasting {}", *read);
+            let ret = match *read {
                 Typed(ref val) => {
                     unsafe { Some(val.downcast_ref_unchecked()) }
                 },
                 _ => unreachable!()
             };
-            ret
+            unsafe { transmute::<Option<&H>, Option<&H>>(ret) }
         })
     }
 
@@ -229,7 +235,7 @@ impl fmt::Show for Headers {
 
 /// An `Iterator` over the fields in a `Headers` map.
 pub struct HeadersItems<'a> {
-    inner: Entries<'a, CaseInsensitive, Item>
+    inner: Entries<'a, CaseInsensitive, RWLock<Item>>
 }
 
 impl<'a> Iterator<(&'a str, HeaderView<'a>)> for HeadersItems<'a> {
@@ -242,12 +248,12 @@ impl<'a> Iterator<(&'a str, HeaderView<'a>)> for HeadersItems<'a> {
 }
 
 /// Returned with the `HeadersItems` iterator.
-pub struct HeaderView<'a>(&'a Item);
+pub struct HeaderView<'a>(&'a RWLock<Item>);
 
 impl<'a> fmt::Show for HeaderView<'a> {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         let HeaderView(item) = *self;
-        item.fmt(fmt)
+        item.read().fmt(fmt)
     }
 }
 
@@ -265,7 +271,7 @@ impl Mutable for Headers {
 
 enum Item {
     Raw(Vec<Vec<u8>>),
-    Typed(Box<Header>)
+    Typed(Box<Header + Send + Sync>)
 }
 
 impl fmt::Show for Item {
@@ -341,7 +347,7 @@ mod tests {
 
     #[test]
     fn test_from_raw() {
-        let mut headers = Headers::from_raw(&mut mem("Content-Length: 10\r\n\r\n")).unwrap();
+        let headers = Headers::from_raw(&mut mem("Content-Length: 10\r\n\r\n")).unwrap();
         assert_eq!(headers.get_ref(), Some(&ContentLength(10)));
     }
 
@@ -380,8 +386,9 @@ mod tests {
 
     #[test]
     fn test_different_structs_for_same_header() {
-        let mut headers = Headers::from_raw(&mut mem("Content-Length: 10\r\n\r\n")).unwrap();
+        let headers = Headers::from_raw(&mut mem("Content-Length: 10\r\n\r\n")).unwrap();
         let ContentLength(_) = headers.get::<ContentLength>().unwrap();
         assert!(headers.get::<CrazyLength>().is_none());
     }
 }
+