From 971ba3eee14b4191ccddd6ddae0a5c135ae789e9 Mon Sep 17 00:00:00 2001
From: Rob Ede <robjtede@icloud.com>
Date: Sat, 18 Jul 2020 16:17:00 +0100
Subject: [PATCH] fix continous growth of app data in pooled requests (#1609)

fixes #1606
fixes #1607
---
 CHANGES.md         |  4 ++++
 src/app_service.rs |  3 ++-
 src/request.rs     | 22 +++++++++++++++-------
 3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index 149ff8fcb..fca46d1c1 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -1,6 +1,10 @@
 # Changes
 
 ## Unreleased - 2020-xx-xx
+### Fixed
+* Memory leak of app data in pooled requests. [#1609]
+
+[#1609]: https://github.com/actix/actix-web/pull/1609
 
 
 ## 3.0.0-beta.1 - 2020-07-13
diff --git a/src/app_service.rs b/src/app_service.rs
index 233cfc0d5..d41cee9fd 100644
--- a/src/app_service.rs
+++ b/src/app_service.rs
@@ -10,6 +10,7 @@ use actix_router::{Path, ResourceDef, ResourceInfo, Router, Url};
 use actix_service::boxed::{self, BoxService, BoxServiceFactory};
 use actix_service::{fn_service, Service, ServiceFactory};
 use futures_util::future::{join_all, ok, FutureExt, LocalBoxFuture};
+use tinyvec::tiny_vec;
 
 use crate::config::{AppConfig, AppService};
 use crate::data::{DataFactory, FnDataFactory};
@@ -245,7 +246,7 @@ where
             inner.path.reset();
             inner.head = head;
             inner.payload = payload;
-            inner.app_data.push(self.data.clone());
+            inner.app_data = tiny_vec![self.data.clone()];
             req
         } else {
             HttpRequest::new(
diff --git a/src/request.rs b/src/request.rs
index 85f409016..a1b42f926 100644
--- a/src/request.rs
+++ b/src/request.rs
@@ -276,6 +276,7 @@ impl HttpMessage for HttpRequest {
 
 impl Drop for HttpRequest {
     fn drop(&mut self) {
+        // if possible, contribute to current worker's HttpRequest allocation pool
         if Rc::strong_count(&self.0) == 1 {
             let v = &mut self.0.pool.0.borrow_mut();
             if v.len() < 128 {
@@ -340,25 +341,32 @@ impl fmt::Debug for HttpRequest {
     }
 }
 
-/// Request's objects pool
+/// Slab-allocated `HttpRequest` Pool
+///
+/// Since request processing may yield for asynchronous events to complete, a worker may have many
+/// requests in-flight at any time. Pooling requests like this amortizes the performance and memory
+/// costs of allocating and de-allocating HttpRequest objects as frequently as they otherwise would.
+///
+/// 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>>>);
 
 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))
     }
 
-    /// Get message from the pool
+    /// Re-use a previously allocated (but now completed/discarded) HttpRequest object.
     #[inline]
     pub(crate) fn get_request(&self) -> Option<HttpRequest> {
-        if let Some(inner) = self.0.borrow_mut().pop() {
-            Some(HttpRequest(inner))
-        } else {
-            None
-        }
+        self.0.borrow_mut().pop().map(HttpRequest)
     }
 
+    /// Clears all allocated HttpRequest objects.
     pub(crate) fn clear(&self) {
         self.0.borrow_mut().clear()
     }