Merge pull request #172 from lipanski/fix-server-pool

Implement created counter on the server pool properly and lower the pool size
lipanski · Jun 17, 2023 · de712a8 · de712a8
2 parents 3f24304 + 2597e11
commit de712a8
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 44 deletions.
diff --git a/src/server.rs b/src/server.rs
@@ -112,7 +112,7 @@ impl State {
 ///
 /// Mockito uses a server pool to manage running servers. Once the pool reaches capacity,
 /// new requests will have to wait for a free server. The size of the server pool
-/// is set to 100.
+/// is set to 50.
 ///
 /// Most of the times, you should initialize new servers with `Server::new`, which fetches
 /// the next available instance from the pool:

diff --git a/src/server_pool.rs b/src/server_pool.rs
@@ -6,7 +6,7 @@ use std::ops::{Deref, DerefMut, Drop};
 use std::sync::{Arc, Mutex};
 use tokio::sync::{Semaphore, SemaphorePermit};
 
-const DEFAULT_POOL_SIZE: usize = 100;
+const DEFAULT_POOL_SIZE: usize = 50;
 
 lazy_static! {
     pub(crate) static ref SERVER_POOL: ServerPool = ServerPool::new(DEFAULT_POOL_SIZE);
@@ -53,14 +53,14 @@ impl Drop for ServerGuard {
 
 pub(crate) struct ServerPool {
     max_size: usize,
-    created: usize,
+    created: Arc<Mutex<usize>>,
     semaphore: Semaphore,
     state: Arc<Mutex<VecDeque<Server>>>,
 }
 
 impl ServerPool {
     fn new(max_size: usize) -> ServerPool {
-        let created = 0;
+        let created = Arc::new(Mutex::new(0));
         let semaphore = Semaphore::new(max_size);
         let state = Arc::new(Mutex::new(VecDeque::new()));
         ServerPool {
@@ -78,10 +78,23 @@ impl ServerPool {
             .await
             .map_err(|err| Error::new_with_context(ErrorKind::Deadlock, err))?;
 
-        let server = if self.created < self.max_size {
-            Some(Server::try_new_with_port_async(0).await?)
-        } else {
-            None
+        let should_create = {
+            let created_mutex = self.created.clone();
+            let mut created = created_mutex.lock().unwrap();
+            if *created < self.max_size {
+                *created += 1;
+                true
+            } else {
+                false
+            }
+        };
+
+        let server = {
+            if should_create {
+                Some(Server::try_new_with_port_async(0).await?)
+            } else {
+                None
+            }
         };
 
         let state_mutex = self.state.clone();

diff --git a/tests/lib.rs b/tests/lib.rs
@@ -1902,42 +1902,61 @@ fn test_running_multiple_servers() {
 }
 
 #[test]
-fn test_running_lots_of_servers_wont_block() {
-    let mut s1 = Server::new();
-    let _s2 = Server::new();
-    let _s3 = Server::new();
-    let _s4 = Server::new();
-    let _s5 = Server::new();
-    let _s7 = Server::new();
-    let _s8 = Server::new();
-    let _s9 = Server::new();
-    let _s10 = Server::new();
-    let _s11 = Server::new();
-    let _s12 = Server::new();
-    let _s13 = Server::new();
-    let _s14 = Server::new();
-    let _s15 = Server::new();
-    let _s17 = Server::new();
-    let _s18 = Server::new();
-    let _s19 = Server::new();
-    let _s20 = Server::new();
-    let _s21 = Server::new();
-    let _s22 = Server::new();
-    let _s23 = Server::new();
-    let _s24 = Server::new();
-    let _s25 = Server::new();
-    let _s27 = Server::new();
-    let _s28 = Server::new();
-    let _s29 = Server::new();
-    let mut s30 = Server::new();
-
-    let m1 = s1.mock("GET", "/pool").create();
-    let (_, _, _) = request_with_body(&s1.host_with_port(), "GET /pool", "", "");
-    m1.assert();
-
-    let m30 = s30.mock("GET", "/pool").create();
-    let (_, _, _) = request_with_body(&s30.host_with_port(), "GET /pool", "", "");
-    m30.assert();
+#[allow(clippy::vec_init_then_push)]
+fn test_server_pool() {
+    // If the pool is not working, this will hit the FD limit (Too many open files)
+    for _ in 0..5 {
+        // The pool size is 50, anything beyond that will block
+        for _ in 0..50 {
+            let mut servers = vec![];
+            servers.push(Server::new());
+
+            let s = servers.first_mut().unwrap();
+            let m = s.mock("GET", "/pool").create();
+            let (_, _, _) = request_with_body(&s.host_with_port(), "GET /pool", "", "");
+            m.assert();
+        }
+
+        for _ in 0..50 {
+            let mut servers = vec![];
+            servers.push(Server::new());
+
+            let s = servers.first_mut().unwrap();
+            let m = s.mock("GET", "/pool").create();
+            let (_, _, _) = request_with_body(&s.host_with_port(), "GET /pool", "", "");
+            m.assert();
+        }
+
+        for _ in 0..50 {
+            let mut servers = vec![];
+            servers.push(Server::new());
+
+            let s = servers.first_mut().unwrap();
+            let m = s.mock("GET", "/pool").create();
+            let (_, _, _) = request_with_body(&s.host_with_port(), "GET /pool", "", "");
+            m.assert();
+        }
+
+        for _ in 0..50 {
+            let mut servers = vec![];
+            servers.push(Server::new());
+
+            let s = servers.first_mut().unwrap();
+            let m = s.mock("GET", "/pool").create();
+            let (_, _, _) = request_with_body(&s.host_with_port(), "GET /pool", "", "");
+            m.assert();
+        }
+
+        for _ in 0..50 {
+            let mut servers = vec![];
+            servers.push(Server::new());
+
+            let s = servers.first_mut().unwrap();
+            let m = s.mock("GET", "/pool").create();
+            let (_, _, _) = request_with_body(&s.host_with_port(), "GET /pool", "", "");
+            m.assert();
+        }
+    }
 }
 
 #[tokio::test]