Fix incorrect handling of nested 'attach' calls.

lib/src/fairing/fairings.rs

@@ -17,15 +17,18 @@ impl Fairings {
     }
 
     pub fn attach(&mut self, fairing: Box<Fairing>, mut rocket: Rocket) -> Rocket {
-        // Get the kind information.
-        let kind = fairing.info().kind;
-
         // Run the `on_attach` callback if this is an 'attach' fairing.
+        let kind = fairing.info().kind;
         if kind.is(Kind::Attach) {
             rocket = fairing.on_attach(rocket)
                 .unwrap_or_else(|r| { self.attach_failure = true; r })
         }
 
+        self.add(fairing);
+        rocket
+    }
+
+    fn add(&mut self, fairing: Box<Fairing>) {
         // The `Fairings` structure separates `all_fairings` into kind groups so
         // we don't have to search through all fairings and do a comparison at
         // runtime. We need references since a single structure can be multiple
@@ -44,6 +47,7 @@ impl Fairings {
         // reallocations there are irrelvant. Instead, it points into the heap.
         //
         // Also, we don't save attach fairings since we don't need them anymore.
+        let kind = fairing.info().kind;
         if !kind.is_exactly(Kind::Attach) {
             let ptr: &'static Fairing = unsafe { ::std::mem::transmute(&*fairing) };
 
@@ -52,8 +56,12 @@ impl Fairings {
             if kind.is(Kind::Request) { self.request.push(ptr); }
             if kind.is(Kind::Response) { self.response.push(ptr); }
         }
+    }
 
-        rocket
+    pub fn append(&mut self, others: Fairings) {
+        for fairing in others.all_fairings {
+            self.add(fairing);
+        }
     }
 
     #[inline(always)]

lib/src/rocket.rs

@@ -631,7 +631,8 @@ impl Rocket {
         let mut fairings = mem::replace(&mut self.fairings, Fairings::new());
         self = fairings.attach(Box::new(fairing), self);
 
-        // Make sure we keep the fairings around!
+        // Make sure we keep all fairings around: the old and newly added ones!
+        fairings.append(self.fairings);
         self.fairings = fairings;
         self
     }

lib/tests/nested-fairing-attaches.rs

@@ -0,0 +1,61 @@
+#![feature(plugin, decl_macro)]
+#![plugin(rocket_codegen)]
+
+extern crate rocket;
+
+use std::sync::atomic::{AtomicUsize, Ordering};
+
+use rocket::State;
+use rocket::fairing::AdHoc;
+use rocket::http::Method;
+
+#[derive(Default)]
+struct Counter {
+    attach: AtomicUsize,
+    get: AtomicUsize,
+}
+
+#[get("/")]
+fn index(counter: State<Counter>) -> String {
+    let attaches = counter.attach.load(Ordering::Relaxed);
+    let gets = counter.get.load(Ordering::Acquire);
+    format!("{}, {}", attaches, gets)
+}
+
+fn rocket() -> rocket::Rocket {
+    rocket::ignite()
+        .mount("/", routes![index])
+        .attach(AdHoc::on_attach(|rocket| {
+            let counter = Counter::default();
+            counter.attach.fetch_add(1, Ordering::Relaxed);
+            let rocket = rocket.manage(counter)
+                .attach(AdHoc::on_request(|req, _| {
+                    if req.method() == Method::Get {
+                        let counter = req.guard::<State<Counter>>().unwrap();
+                        counter.get.fetch_add(1, Ordering::Release);
+                    }
+                }));
+
+            Ok(rocket)
+        }))
+}
+
+mod nested_fairing_attaches_tests {
+    use super::*;
+    use rocket::local::Client;
+
+    #[test]
+    fn test_counts() {
+        let client = Client::new(rocket()).unwrap();
+        let mut response = client.get("/").dispatch();
+        assert_eq!(response.body_string(), Some("1, 1".into()));
+
+        let mut response = client.get("/").dispatch();
+        assert_eq!(response.body_string(), Some("1, 2".into()));
+
+        client.get("/").dispatch();
+        client.get("/").dispatch();
+        let mut response = client.get("/").dispatch();
+        assert_eq!(response.body_string(), Some("1, 5".into()));
+    }
+}