Fixed a race condition in get_ref

If two threads attempted to `get_ref` the same Header under two
representations, then it was possible that the second thread would
overwrite the work of the first thread, causing the first thread
to do an unchecked downcast to the wrong type.

In the case where they were the same Header representation, then the
only damage would be that it would be parsed twice and would possibly
return a false negative in one thread.

The new code checks that it was not a queued lock and will not override
the work of another thread, but will accept it if the other thread parsed
the header as the same type. This also prevents duplicate parsing.

src/header/mod.rs

@@ -160,38 +160,53 @@ impl Headers {
     /// Get a reference to the header field's value, if it exists.
     pub fn get_ref<H: Header>(&self) -> Option<&H> {
         self.data.find(&CaseInsensitive(Slice(header_name::<H>()))).and_then(|item| {
-            let header = match *item.read() {
+            let done = match *item.read() {
                 // Huge borrowck hack here, should be refactored to just return here.
-                Typed(ref typed) if typed.is::<H>() => None,
+                Typed(ref typed) if typed.is::<H>() => true,
-                // Typed, wrong type
+
+                // Typed, wrong type.
                 Typed(_) => return None,
-                Raw(ref raw) => match Header::parse_header(raw.as_slice()) {
+
-                    Some::<H>(h) => {
+                // Raw, work to do.
-                        Some(h)
+                Raw(_) => false,
-                    },
-                    None => return None
-                },
             };
 
-            match header {
+            // borrowck hack continued
-                Some(header) => {
+            if done { return Some(item); }
-                    *item.write() = Typed(box header as Box<Header + Send + Sync>);
+
-                    Some(item)
+            // Take out a write lock to do the parsing and mutation.
-                },
+            let mut write = item.write();
-                None => {
+
-                    Some(item)
+            let header = match *write {
+                // Since this lock can queue, it's possible another thread just
+                // did the work for us.
+                //
+                // Check they inserted the correct type and move on.
+                Typed(ref typed) if typed.is::<H>() => return Some(item),
+
+                // Wrong type, another thread got here before us and parsed
+                // as a different representation.
+                Typed(_) => return None,
+
+                // We are first in the queue or the only ones, so do the actual
+                // work of parsing and mutation.
+                Raw(ref raw) => match Header::parse_header(raw.as_slice()) {
+                    Some::<H>(h) => h,
+                    None => return None
                 }
-            }
+            };
-        }).and_then(|item| {
+
+            // Mutate in the raw case.
+            *write = Typed(box header as Box<Header + Send + Sync>);
+            Some(item)
+        }).map(|item| {
             let read = item.read();
             debug!("downcasting {}", *read);
             let ret = match *read {
-                Typed(ref val) => {
+                Typed(ref val) => unsafe { val.downcast_ref_unchecked() },
-                    unsafe { Some(val.downcast_ref_unchecked()) }
-                },
                 _ => unreachable!()
             };
-            unsafe { transmute::<Option<&H>, Option<&H>>(ret) }
+            unsafe { transmute::<&H, &H>(ret) }
         })
     }