use a non leak pool for HttpRequestInner

src/app_service.rs

@@ -13,7 +13,7 @@ use crate::config::{AppConfig, AppService};
 use crate::data::{DataFactory, FnDataFactory};
 use crate::error::Error;
 use crate::guard::Guard;
-use crate::request::{HttpRequest, HttpRequestPool};
+use crate::request::HttpRequest;
 use crate::rmap::ResourceMap;
 use crate::service::{AppServiceFactory, ServiceRequest, ServiceResponse};
 
@@ -144,7 +144,6 @@ where
                 rmap,
                 config,
                 app_data: Rc::new(app_data),
-                pool: HttpRequestPool::create(),
             })
         })
     }
@@ -159,7 +158,6 @@ where
     rmap: Rc<ResourceMap>,
     config: AppConfig,
     app_data: Rc<Extensions>,
-    pool: &'static HttpRequestPool,
 }
 
 impl<T, B> Service<Request> for AppInitService<T, B>
@@ -177,7 +175,7 @@ where
     fn call(&mut self, req: Request) -> Self::Future {
         let (head, payload) = req.into_parts();
 
-        let req = if let Some(mut req) = self.pool.get_request() {
+        let req = if let Some(mut req) = self.config.pool().get() {
             let inner = Rc::get_mut(&mut req.inner).unwrap();
             inner.path.get_mut().update(&head.uri);
             inner.path.reset();
@@ -192,19 +190,19 @@ where
                 self.rmap.clone(),
                 self.config.clone(),
                 self.app_data.clone(),
-                self.pool,
             )
         };
         self.service.call(ServiceRequest::new(req))
     }
 }
 
+// TODO: remove the drop impl as pool is not leaked anymore.
 impl<T, B> Drop for AppInitService<T, B>
 where
     T: Service<ServiceRequest, Response = ServiceResponse<B>, Error = Error>,
 {
     fn drop(&mut self) {
-        self.pool.clear();
+        self.config.pool().clear();
     }
 }
 

src/config.rs

@@ -8,6 +8,7 @@ use actix_service::{boxed, IntoServiceFactory, ServiceFactory};
 use crate::data::{Data, DataFactory};
 use crate::error::Error;
 use crate::guard::Guard;
+use crate::request::HttpRequestPool;
 use crate::resource::Resource;
 use crate::rmap::ResourceMap;
 use crate::route::Route;
@@ -131,11 +132,17 @@ struct AppConfigInner {
     secure: bool,
     host: String,
     addr: SocketAddr,
+    pool: HttpRequestPool,
 }
 
 impl AppConfig {
     pub(crate) fn new(secure: bool, addr: SocketAddr, host: String) -> Self {
-        AppConfig(Rc::new(AppConfigInner { secure, addr, host }))
+        AppConfig(Rc::new(AppConfigInner {
+            secure,
+            addr,
+            host,
+            pool: HttpRequestPool::default(),
+        }))
     }
 
     /// Server host name.
@@ -158,6 +165,11 @@ impl AppConfig {
     pub fn local_addr(&self) -> SocketAddr {
         self.0.addr
     }
+
+    #[inline]
+    pub(crate) fn pool(&self) -> &HttpRequestPool {
+        &self.0.pool
+    }
 }
 
 impl Default for AppConfig {

src/request.rs

@@ -30,7 +30,6 @@ pub(crate) struct HttpRequestInner {
     pub(crate) app_data: SmallVec<[Rc<Extensions>; 4]>,
     rmap: Rc<ResourceMap>,
     config: AppConfig,
-    pool: &'static HttpRequestPool,
 }
 
 impl HttpRequest {
@@ -42,7 +41,6 @@ impl HttpRequest {
         rmap: Rc<ResourceMap>,
         config: AppConfig,
         app_data: Rc<Extensions>,
-        pool: &'static HttpRequestPool,
     ) -> HttpRequest {
         let mut data = SmallVec::<[Rc<Extensions>; 4]>::new();
         data.push(app_data);
@@ -55,7 +53,6 @@ impl HttpRequest {
                 rmap,
                 config,
                 app_data: data,
-                pool,
             }),
         }
     }
@@ -287,14 +284,16 @@ impl Drop for HttpRequest {
 
         // This relies on no Weak<HttpRequestInner> exists anywhere.(There is none)
         if let Some(inner) = Rc::get_mut(&mut self.inner) {
-            let v = &mut inner.pool.0.borrow_mut();
-            if v.len() < 128 {
+            if inner.config.pool().is_available() {
                 // clear additional app_data and keep the root one for reuse.
                 inner.app_data.truncate(1);
                 // inner is borrowed mut here. get head's Extension mutably
                 // to reduce borrow check
                 inner.head.extensions.get_mut().clear();
-                v.push(self.inner.clone());
+
+                // a re-borrow of pool is necessary here.
+                let req = self.inner.clone();
+                self.inner.config.pool().push(req);
             }
         }
     }
@@ -363,25 +362,50 @@ impl fmt::Debug for HttpRequest {
 /// Request objects are added when they are dropped (see `<HttpRequest as Drop>::drop`) and re-used
 /// in `<AppInitService as Service>::call` when there are available objects in the list.
 ///
-/// The pool's initial capacity is 128 items.
-pub(crate) struct HttpRequestPool(RefCell<Vec<Rc<HttpRequestInner>>>);
+/// The pool's default capacity is 128 items.
+pub(crate) struct HttpRequestPool {
+    inner: RefCell<Vec<Rc<HttpRequestInner>>>,
+    cap: usize,
+}
+
+impl Default for HttpRequestPool {
+    fn default() -> Self {
+        Self::with_capacity(128)
+    }
+}
 
 impl HttpRequestPool {
-    /// Allocates a slab of memory for pool use.
-    pub(crate) fn create() -> &'static HttpRequestPool {
-        let pool = HttpRequestPool(RefCell::new(Vec::with_capacity(128)));
-        Box::leak(Box::new(pool))
+    pub(crate) fn with_capacity(cap: usize) -> Self {
+        HttpRequestPool {
+            inner: RefCell::new(Vec::with_capacity(cap)),
+            cap,
+        }
     }
 
     /// Re-use a previously allocated (but now completed/discarded) HttpRequest object.
     #[inline]
-    pub(crate) fn get_request(&self) -> Option<HttpRequest> {
-        self.0.borrow_mut().pop().map(|inner| HttpRequest { inner })
+    pub(crate) fn get(&self) -> Option<HttpRequest> {
+        self.inner
+            .borrow_mut()
+            .pop()
+            .map(|inner| HttpRequest { inner })
+    }
+
+    /// Check if the pool still has capacity for request storage.
+    #[inline]
+    pub(crate) fn is_available(&self) -> bool {
+        self.inner.borrow_mut().len() < self.cap
+    }
+
+    /// Push a request to pool.
+    #[inline]
+    pub(crate) fn push(&self, req: Rc<HttpRequestInner>) {
+        self.inner.borrow_mut().push(req);
     }
 
     /// Clears all allocated HttpRequest objects.
     pub(crate) fn clear(&self) {
-        self.0.borrow_mut().clear()
+        self.inner.borrow_mut().clear()
     }
 }
 

src/test.rs

@@ -30,7 +30,6 @@ pub use actix_http::test::TestBuffer;
 use crate::config::AppConfig;
 use crate::data::Data;
 use crate::dev::{Body, MessageBody, Payload, Server};
-use crate::request::HttpRequestPool;
 use crate::rmap::ResourceMap;
 use crate::service::{ServiceRequest, ServiceResponse};
 use crate::{Error, HttpRequest, HttpResponse};
@@ -549,7 +548,6 @@ impl TestRequest {
             Rc::new(self.rmap),
             self.config.clone(),
             Rc::new(self.app_data),
-            HttpRequestPool::create(),
         ))
     }
 
@@ -571,7 +569,6 @@ impl TestRequest {
             Rc::new(self.rmap),
             self.config.clone(),
             Rc::new(self.app_data),
-            HttpRequestPool::create(),
         )
     }
 
@@ -588,7 +585,6 @@ impl TestRequest {
             Rc::new(self.rmap),
             self.config.clone(),
             Rc::new(self.app_data),
-            HttpRequestPool::create(),
         );
 
         (req, payload)