@@ -756,21 +756,25 @@ async fn handle_active_connection(
756756 }
757757 let _ = auth_cancel_task. await ;
758758
759- crate :: handlers:: close:: release_connection_subscriptions ( & conn, & state) . await ;
760- state. conn_manager . deregister ( conn. conn_id ) ;
761- if let Some ( auth_ctx) = authenticated {
762- if !state. conn_manager . has_admitted_connection (
763- conn. tenant . community ( ) ,
764- auth_ctx. pubkey . to_bytes ( ) . as_slice ( ) ,
765- ) {
766- let _ = state
767- . pubsub
768- . clear_presence ( & conn. tenant , & auth_ctx. pubkey )
769- . await ;
759+ // Release application state and wait on the peer concurrently: cleanup
760+ // never waits on the client, and a stalled Redis cannot hold the socket
761+ // open past the bounded peer wait.
762+ let cleanup = async {
763+ crate :: handlers:: close:: release_connection_subscriptions ( & conn, & state) . await ;
764+ state. conn_manager . deregister ( conn. conn_id ) ;
765+ if let Some ( auth_ctx) = authenticated {
766+ if !state. conn_manager . has_admitted_connection (
767+ conn. tenant . community ( ) ,
768+ auth_ctx. pubkey . to_bytes ( ) . as_slice ( ) ,
769+ ) {
770+ let _ = state
771+ . pubsub
772+ . clear_presence ( & conn. tenant , & auth_ctx. pubkey )
773+ . await ;
774+ }
770775 }
771- }
772- // Application state is released above; only the transport waits on the peer.
773- await_client_close ( ws_recv) . await ;
776+ } ;
777+ tokio:: join!( cleanup, await_client_close( ws_recv) ) ;
774778 metrics:: gauge!( "buzz_ws_connections_active" ) . decrement ( 1.0 ) ;
775779 info ! ( conn_id = %conn_id, addr = %addr, "WebSocket connection closed" ) ;
776780
@@ -3253,6 +3257,16 @@ mod terminal_close_delivery_tests {
32533257 async fn serve (
32543258 assertion : VerifiedAssertion ,
32553259 prefire_expiry : bool ,
3260+ ) -> ( std:: net:: SocketAddr , RelayUrl , Arc < AppState > , Uuid ) {
3261+ serve_with_cleanup_pause ( assertion, prefire_expiry, None ) . await
3262+ }
3263+
3264+ /// Like `serve`, but runs the handler inside `cleanup_pause` so a test
3265+ /// can stall the connection's final subscription cleanup.
3266+ async fn serve_with_cleanup_pause (
3267+ assertion : VerifiedAssertion ,
3268+ prefire_expiry : bool ,
3269+ cleanup_pause : Option < Arc < crate :: handlers:: close:: test_seam:: Pause > > ,
32563270 ) -> ( std:: net:: SocketAddr , RelayUrl , Arc < AppState > , Uuid ) {
32573271 let state = crate :: state:: tests:: test_state ( ) . await ;
32583272 let served_state = Arc :: clone ( & state) ;
@@ -3268,8 +3282,12 @@ mod terminal_close_delivery_tests {
32683282 let app = Router :: new ( ) . route (
32693283 "/" ,
32703284 get ( move |ws : WebSocketUpgrade | {
3271- let ( state, tenant, assertion) =
3272- ( Arc :: clone ( & state) , tenant. clone ( ) , assertion. clone ( ) ) ;
3285+ let ( state, tenant, assertion, cleanup_pause) = (
3286+ Arc :: clone ( & state) ,
3287+ tenant. clone ( ) ,
3288+ assertion. clone ( ) ,
3289+ cleanup_pause. clone ( ) ,
3290+ ) ;
32733291 async move {
32743292 ws. on_upgrade ( move |socket| async move {
32753293 let cancel = CancellationToken :: new ( ) ;
@@ -3288,7 +3306,7 @@ mod terminal_close_delivery_tests {
32883306 ) ;
32893307 ( gate, tx, rx, Some ( expiry) )
32903308 } ) ;
3291- handle_active_connection (
3309+ let handler = handle_active_connection (
32923310 socket,
32933311 state,
32943312 "127.0.0.1:1234" . parse ( ) . expect ( "client address" ) ,
@@ -3298,8 +3316,15 @@ mod terminal_close_delivery_tests {
32983316 Some ( assertion) ,
32993317 chrono:: Utc :: now ( ) ,
33003318 pre_built,
3301- )
3302- . await ;
3319+ ) ;
3320+ match cleanup_pause {
3321+ Some ( pause) => {
3322+ crate :: handlers:: close:: test_seam:: CLEANUP_PAUSE
3323+ . scope ( pause, handler)
3324+ . await ;
3325+ }
3326+ None => handler. await ,
3327+ }
33033328 } )
33043329 }
33053330 } ) ,
@@ -3470,7 +3495,7 @@ mod terminal_close_delivery_tests {
34703495 /// (seen live as `1006`). This raw client never replies: the server must
34713496 /// hold the socket for its full bounded wait, which starts only after
34723497 /// connect, so timing from before connect makes the lower bound
3473- /// independent of scheduler delay. The wait runs after cleanup , so the
3498+ /// independent of scheduler delay. Cleanup runs alongside the wait , so the
34743499 /// connection is already deregistered once the socket ends.
34753500 #[ tokio:: test]
34763501 async fn server_holds_socket_for_client_close_reply ( ) {
@@ -3490,8 +3515,8 @@ mod terminal_close_delivery_tests {
34903515 }
34913516
34923517 /// The client withholds its Close reply until the connection has
3493- /// deregistered, so cleanup must run before the bounded peer wait : a
3494- /// server that cleans up after the wait, or ignores the reply, needs at
3518+ /// deregistered, so cleanup must not wait on the peer: a server that
3519+ /// cleans up after the wait, or ignores the reply, needs at
34953520 /// least `WS_TERMINAL_FLUSH_TIMEOUT` from connect.
34963521 #[ tokio:: test]
34973522 async fn server_finishes_promptly_when_client_answers_close ( ) {
@@ -3519,4 +3544,20 @@ mod terminal_close_delivery_tests {
35193544 "server must finish on the client's Close, not the timeout; took {elapsed:?}"
35203545 ) ;
35213546 }
3547+
3548+ /// Final cleanup can block on Redis with no deadline (the pool has no
3549+ /// wait timeout). The peer wait runs alongside it, so a stalled cleanup
3550+ /// still lets the socket end once the bounded wait expires.
3551+ #[ tokio:: test]
3552+ async fn stalled_cleanup_does_not_hold_socket_open ( ) {
3553+ let pause = Arc :: new ( crate :: handlers:: close:: test_seam:: Pause :: default ( ) ) ;
3554+ let ( address, _, _, _) =
3555+ serve_with_cleanup_pause ( expired_assertion ( ) , true , Some ( Arc :: clone ( & pause) ) ) . await ;
3556+ let started = std:: time:: Instant :: now ( ) ;
3557+ let mut socket = raw_client_after_server_close ( address) . await ;
3558+
3559+ pause. reached . notified ( ) . await ;
3560+ let elapsed = time_to_eof ( & mut socket, started) . await ;
3561+ assert ! ( elapsed >= WS_TERMINAL_FLUSH_TIMEOUT ) ;
3562+ }
35223563}
0 commit comments