Skip to content

Commit

Permalink
Demonstratively simplify ServerPool
Browse files Browse the repository at this point in the history
`self.created` was never being incremented, therefore `self.created < self.max_size`
always evaluated to true. We can therefore drop the two fields from the
struct, and also drop two conditions from `get_async`, all without
changing the behaviour.

Now this makes it obvious that the "pool" doesn't really behave as a
pool. Every time a server is requested (consuming 5 file descriptors and
at least one thread), a new one is created and added to the end of the
"pool". Recycled servers are being reused indeed, but the pool grows
over its max size easily, eventually hitting the process file descriptor
limit.
  • Loading branch information
liskin committed Jun 6, 2023
1 parent 3f21138 commit ea7e583
Showing 1 changed file with 4 additions and 18 deletions.
22 changes: 4 additions & 18 deletions src/server_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,23 +52,15 @@ impl Drop for ServerGuard {
}

pub(crate) struct ServerPool {
max_size: usize,
created: usize,
semaphore: Semaphore,
state: Arc<Mutex<VecDeque<Server>>>,
}

impl ServerPool {
fn new(max_size: usize) -> ServerPool {
let created = 0;
let semaphore = Semaphore::new(max_size);
let state = Arc::new(Mutex::new(VecDeque::new()));
ServerPool {
max_size,
created,
semaphore,
state,
}
ServerPool { semaphore, state }
}

pub(crate) async fn get_async(&'static self) -> Result<ServerGuard, Error> {
Expand All @@ -78,21 +70,15 @@ 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 server = Server::try_new_with_port_async(0).await?;

let mut state = self.state.lock().unwrap();

if let Some(server) = server {
state.push_back(server);
}
state.push_back(server);

if let Some(server) = state.pop_front() {
Ok(ServerGuard::new(server, permit))
} else {
// can't happen, we just unconditionally pushed a new server
Err(Error::new(ErrorKind::ServerBusy))
}
}
Expand Down

0 comments on commit ea7e583

Please sign in to comment.