Repository: poem-web/poem (language: rust)

Issue:
serve_connection should handle panic?
https://github.com/poem-web/poem/blob/c66d78a54812709768f4723f0bda0cc79a6b7c82/poem/src/server.rs#L168-L197

We have a situation that `serve_connection.await` panic and thus `alive_connections.fetch_sub` doesn't execute, so that outer `notify.notified().await` blocks forever.

We should either handle panic, or do a timeout for waiting connections to close.


Discussion / hints:

Fix patch:
diff --git a/poem/src/server.rs b/poem/src/server.rs
index 77507215b4..dfaa4b7e29 100644
--- a/poem/src/server.rs
+++ b/poem/src/server.rs
@@ -3,6 +3,7 @@ use std::{
     future::Future,
     io,
     io::IoSlice,
+    panic::AssertUnwindSafe,
     pin::Pin,
     sync::{
         atomic::{AtomicUsize, Ordering},
@@ -11,6 +12,7 @@ use std::{
     task::{Context, Poll},
 };
 
+use futures_util::FutureExt;
 use http::uri::Scheme;
 use hyper::body::Incoming;
 use hyper_util::server::conn::auto;
@@ -164,8 +166,9 @@ where
                         let notify = notify.clone();
                         let timeout_token = timeout_token.clone();
                         let server_graceful_shutdown_token = server_graceful_shutdown_token.clone();
+                        let server_graceful_shutdown_token_clone = server_graceful_shutdown_token.clone();
 
-                        tokio::spawn(async move {
+                        let spawn_fut = AssertUnwindSafe(async move {
                             let serve_connection = serve_connection(socket, local_addr, remote_addr, scheme, ep, server_graceful_shutdown_token.clone(), idle_timeout);
 
                             if timeout.is_some() {
@@ -176,14 +179,22 @@ where
                             } else {
                                serve_connection.await;
                             }
+                        });
+
+                        tokio::spawn(async move {
+                            let result = spawn_fut.catch_unwind().await;
 
                             if alive_connections.fetch_sub(1, Ordering::Acquire) == 1 {
                                 // notify only if shutdown is initiated, to prevent notification when server is active.
                                 // It's a valid state to have 0 alive connections when server is not shutting down.
-                                if server_graceful_shutdown_token.is_cancelled() {
+                                if server_graceful_shutdown_token_clone.is_cancelled() {
                                     notify.notify_one();
                                 }
                             }
+
+                            if let Err(err) = result {
+                                std::panic::resume_unwind(err);
+                            }
                         });
                     }
                 }


Test patch: