You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "masaori335 (via GitHub)" <gi...@apache.org> on 2026/10/01 06:30:28 UTC
[PR] Harden shutdown against repeated exit signals [trafficserver]
masaori335 opened a new pull request, #13766:
URL: https://github.com/apache/trafficserver/pull/13766
Re-sending SIGTERM/SIGINT is not supported, but one arriving on a later SignalContinuation tick, e.g. during the shutdown_timeout drain, scheduled another AutoStopCont that re-ran the whole shutdown path: the cache directory sync on shutdown ran twice, and stop_ssl_handshaking() tripped its assert. Make SignalContinuation a two-state machine so the shutdown path is scheduled exactly once.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Harden shutdown against repeated exit signals [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13766:
URL: https://github.com/apache/trafficserver/pull/13766#discussion_r4161352731
##########
src/traffic_server/traffic_server.cc:
##########
@@ -327,14 +327,62 @@ struct AutoStopCont : public Continuation {
class SignalContinuation : public Continuation
{
public:
- SignalContinuation() : Continuation(new_ProxyMutex()) { SET_HANDLER(&SignalContinuation::periodic); }
+ SignalContinuation() : Continuation(new_ProxyMutex()) { SET_HANDLER(&SignalContinuation::state_running); }
int
- periodic(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED */)
+ state_running(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED */)
+ {
+ _handle_user_signals();
+
+ if (_consume_exit_signal()) {
+ auto timeout{RecGetRecordInt("proxy.config.stop.shutdown_timeout")};
+ if (timeout && timeout.value()) {
+ ts::Metrics &metrics = ts::Metrics::instance();
+ metrics[metrics.lookup("proxy.process.proxy.draining")].store(1);
+ TSSystemState::drain(true);
+ // Close listening sockets here only if TS is running standalone
+ if (auto close_sockets{RecGetRecordInt("proxy.config.restart.stop_listening")}; close_sockets && close_sockets.value()) {
+ stop_HttpProxyServer();
+ }
+ }
+
+ Dbg(dbg_ctl_server, "received exit signal, shutting down in %" PRId64 "secs", timeout.value());
+
+ // Shutdown in `timeout` seconds (or now if that is 0).
+ eventProcessor.schedule_in(new AutoStopCont(), HRTIME_SECONDS(timeout.value()));
Review Comment:
Right, there still a possible path to double calls and `AutoStopCont` needs refactoring. But from my understanding `PROXY_AUTO_EXIT` is mainly for testing, so I left it here.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Harden shutdown against repeated exit signals [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #13766:
URL: https://github.com/apache/trafficserver/pull/13766#issuecomment-6042837103
Cherry-picked to the 10.2.x branch as 158e4bf93227c229d27fbc4076b2fd408af0d6ed for the 10.2.1 release.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Harden shutdown against repeated exit signals [trafficserver]
Posted by "brbzull0 (via GitHub)" <gi...@apache.org>.
brbzull0 commented on code in PR #13766:
URL: https://github.com/apache/trafficserver/pull/13766#discussion_r4155564787
##########
src/traffic_server/traffic_server.cc:
##########
@@ -327,14 +327,62 @@ struct AutoStopCont : public Continuation {
class SignalContinuation : public Continuation
{
public:
- SignalContinuation() : Continuation(new_ProxyMutex()) { SET_HANDLER(&SignalContinuation::periodic); }
+ SignalContinuation() : Continuation(new_ProxyMutex()) { SET_HANDLER(&SignalContinuation::state_running); }
int
- periodic(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED */)
+ state_running(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED */)
+ {
+ _handle_user_signals();
+
+ if (_consume_exit_signal()) {
+ auto timeout{RecGetRecordInt("proxy.config.stop.shutdown_timeout")};
+ if (timeout && timeout.value()) {
+ ts::Metrics &metrics = ts::Metrics::instance();
+ metrics[metrics.lookup("proxy.process.proxy.draining")].store(1);
+ TSSystemState::drain(true);
+ // Close listening sockets here only if TS is running standalone
+ if (auto close_sockets{RecGetRecordInt("proxy.config.restart.stop_listening")}; close_sockets && close_sockets.value()) {
+ stop_HttpProxyServer();
+ }
+ }
+
+ Dbg(dbg_ctl_server, "received exit signal, shutting down in %" PRId64 "secs", timeout.value());
+
+ // Shutdown in `timeout` seconds (or now if that is 0).
+ eventProcessor.schedule_in(new AutoStopCont(), HRTIME_SECONDS(timeout.value()));
Review Comment:
thought (non-blocking): The guard is in `SignalContinuation`, but `AutoStopCont::mainEvent` itself can still run more than once. Something else schedules it too: the `PROXY_AUTO_EXIT` path in `main()` (`traffic_server.cc:2541`). If a SIGTERM arrives while that auto-exit is pending, `SignalContinuation` is still in `state_running` and schedules a second `AutoStopCont`. The result is the same double cache-dir sync and the same `stop_ssl_handshaking()` assert. The guard could go in `AutoStopCont::mainEvent` instead, as a once-only check at the top. That would cover every caller, now and later. The state machine could stay as it is for the "already scheduled" log message.
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Harden shutdown against repeated exit signals [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 merged PR #13766:
URL: https://github.com/apache/trafficserver/pull/13766
--
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org