From 82a15573b43bf26376327e3d43946fb6162a4970 Mon Sep 17 00:00:00 2001 From: Brian Neradt Date: Tue, 21 Apr 2026 23:04:14 +0000 Subject: [PATCH] Fix bg fill teardown crash in update_size_and_time_stats Production crash logs show ink_assert(0) firing in HttpTransact::update_size_and_time_stats from HttpSM::update_stats via HttpSM::kill_this, triggered by an Http2Stream event re-entering the state machine while background_fill was still in the STARTED state. The background_fill state is normally driven to a terminal value (COMPLETED or ABORTED) by tunnel_handler_server, but when the SM is torn down before that handler runs the state stays STARTED and the unreachable default branch in update_size_and_time_stats asserts. The same path also leaks proxy.process.http.background_fill_current_count because tunnel_handler_server is the only site that decrements the gauge after tunnel_handler_ua incremented it. This normalizes STARTED to ABORTED inside HttpSM::kill_this, before the enable_http_stats gate that guards update_stats, and decrements the gauge there. Doing the normalization in kill_this rather than in update_stats keeps the gauge accounting balanced regardless of whether http stats are enabled, and ensures the downstream stats helper sees a terminal state. As a defensive backstop this also folds the STARTED case into the ABORTED branch of update_size_and_time_stats so a future caller that reaches this point mid-fill records the bytes against background_fill_bytes_aborted instead of crashing the server. --- src/proxy/http/HttpSM.cc | 17 +++++++++++++++++ src/proxy/http/HttpTransact.cc | 7 ++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/proxy/http/HttpSM.cc b/src/proxy/http/HttpSM.cc index de97e3a2002..7be28885fa6 100644 --- a/src/proxy/http/HttpSM.cc +++ b/src/proxy/http/HttpSM.cc @@ -7706,6 +7706,23 @@ HttpSM::kill_this() // we must check it again if (kill_this_async_done == true) { pending_action = nullptr; + + // The background fill state is normally driven to a terminal value + // (COMPLETED or ABORTED) by tunnel_handler_server when the server-side + // tunnel finishes. If the SM is torn down before that handler runs (for + // example, when an Http2Stream event re-enters the SM and drives + // kill_this while the bg fill is still in flight), the state can remain + // STARTED. In that case the background_fill_current_count gauge would + // also leak, because tunnel_handler_server is the only place that + // decrements it after tunnel_handler_ua incremented it. Normalize the + // state here, before the enable_http_stats gate, so the accounting + // balances regardless of whether http stats are enabled and so + // update_size_and_time_stats does not see an unexpected value. + if (background_fill == BackgroundFill_t::STARTED) { + background_fill = BackgroundFill_t::ABORTED; + Metrics::Gauge::decrement(http_rsb.background_fill_current_count); + } + if (t_state.http_config_param->enable_http_stats) { update_stats(); } diff --git a/src/proxy/http/HttpTransact.cc b/src/proxy/http/HttpTransact.cc index dba7105f3c8..d96810593bd 100644 --- a/src/proxy/http/HttpTransact.cc +++ b/src/proxy/http/HttpTransact.cc @@ -8957,6 +8957,12 @@ HttpTransact::update_size_and_time_stats(State *s, ink_hrtime total_time, ink_hr Metrics::Counter::increment(http_rsb.background_fill_bytes_completed, bg_size); break; } + // STARTED is normally transitioned to COMPLETED or ABORTED by + // tunnel_handler_server, and HttpSM::kill_this() normalizes any leftover + // STARTED state to ABORTED before reaching here. Treat STARTED the same as + // ABORTED defensively in case a future code path reaches this point with + // the bg fill still in flight, so we record the bytes rather than abort. + case BackgroundFill_t::STARTED: case BackgroundFill_t::ABORTED: { int64_t bg_size = origin_server_response_body_size - user_agent_response_body_size; @@ -8968,7 +8974,6 @@ HttpTransact::update_size_and_time_stats(State *s, ink_hrtime total_time, ink_hr } case BackgroundFill_t::NONE: break; - case BackgroundFill_t::STARTED: default: ink_assert(0); }