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: [email protected]

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

Reply via email to