Copilot commented on code in PR #13114:
URL: https://github.com/apache/trafficserver/pull/13114#discussion_r3120798220


##########
src/proxy/http/HttpSM.cc:
##########
@@ -7777,6 +7777,21 @@ HttpSM::update_stats()
   ATS_PROBE2(milestone_sm_finish, sm_id, static_cast<int>(status_code));
   milestones[TS_MILESTONE_SM_FINISH] = ink_get_hrtime();
 
+  // 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 so the
+  // accounting balances and 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);
+  }

Review Comment:
   The background_fill_current_count decrement is done inside 
HttpSM::update_stats(), but update_stats() is only invoked when 
proxy.config.http.enable_http_stats is enabled (see kill_this()). If http stats 
are disabled and the SM is torn down while background_fill is still STARTED, 
the gauge can still leak. Consider moving this STARTED->ABORTED normalization + 
gauge decrement to an unconditional teardown path (e.g., just before the 
enable_http_stats check in kill_this(), or another always-run cleanup hook) so 
the gauge is balanced regardless of http stats config.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to