Skip to content

Commit

Permalink
serve: don't panic if server already running
Browse files Browse the repository at this point in the history
Signed-off-by: Dan Bond <[email protected]>
  • Loading branch information
loshz committed Jun 12, 2022
1 parent 6db926a commit 959d45c
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 13 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "metrics_server"
version = "0.8.0"
version = "0.8.1"
authors = ["Dan Bond <[email protected]>"]
edition = "2021"
rust-version = "1.58"
Expand Down
19 changes: 9 additions & 10 deletions src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ impl MetricsServer {
///
/// # Panics
///
/// Panics if given an invalid address.
/// Panics if given an invalid address or incorrect TLS credentials.
pub fn new<A>(addr: A, certificate: Option<Vec<u8>>, private_key: Option<Vec<u8>>) -> Self
where
A: ToSocketAddrs,
Expand Down Expand Up @@ -98,13 +98,12 @@ impl MetricsServer {
/// Start serving requests on the underlying server.
///
/// The server will only respond synchronously as it blocks until receiving new requests.
///
/// # Panics
///
/// Panics if called on a server that has already been started.
pub fn serve(mut self) -> Self {
if self.thread.is_some() {
panic!("metrics_server already started")
// Check if we already have a thread running.
if let Some(thread) = &self.thread {
if !thread.is_finished() {
return self;
}
}

// Invoking clone on Arc produces a new Arc instance, which points to the
Expand Down Expand Up @@ -161,14 +160,14 @@ impl MetricsServer {
}

impl Drop for MetricsServer {
// TODO: should I really be doing this inside drop? It _could_ panic,
// so maybe a shutdown method would be better?
// TODO: should we really be doing this inside drop? It _could_ panic,
// so maybe a shutdown/stop method would be better?
fn drop(&mut self) {
// Signal that we should stop handling requests and unblock the server.
self.shared.stop.store(true, Ordering::Relaxed);
self.shared.server.unblock();

// Because join takes ownership of the thread, we need call the take method
// Because join takes ownership of the thread, we need to call the take method
// on the Option to move the value out of the Some variant and leave a None
// variant in its place.
if let Some(thread) = self.thread.take() {
Expand Down
4 changes: 2 additions & 2 deletions tests/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ fn test_new_server_invalid_key() {
}

#[test]
#[should_panic]
fn test_new_server_already_running() {
let srv = MetricsServer::new("localhost:8002", None, None).serve();

// Attempt to start an already running server.
// Attempt to start an already running server should be ok
// as we will return the pre-existing thread.
srv.serve();
}

Expand Down

0 comments on commit 959d45c

Please sign in to comment.