[squid-dev] [PATCH] convert DNS shutdown to runner

[Amos Jeffries]

This patch adds a Runner to manage the DNS component state on shutdown
(and the matching reconfigure port closures). The actions taken on
reconfigure and shutdown are not changed, just their timing.

Visible differences are now that Squid logs when DNS ports are being
closed (and the reason), also that the still-open FD table report no
longer lists the DNS listening FD as being open on shutdown when
comm_exit() is called and forces all fd_table entries to close.

NP: I have not moved the Dns::Init() operations to the runner at this
stage because it needs some deeper analysis and redesign as to which
Squid processes DNS is enabled/initialized. Currently everything from
coordinator to Diskers enable the internal-DNS state.

Amos
-------------- next part --------------
=== modified file 'src/base/RunnersRegistry.h'
--- src/base/RunnersRegistry.h	2016-09-12 11:27:33 +0000
+++ src/base/RunnersRegistry.h	2016-10-02 09:38:01 +0000
@@ -57,6 +57,11 @@
 
     /* Reconfiguration events */
 
+    /// Called after receiving a reconfigure request and before parsing squid.conf.
+    /// Meant for halting any active modules that could be triggered by external
+    /// events (such as listening ports) while changing their configuration.
+    virtual void startReconfigure() {}
+
     /// Called after parsing squid.conf during reconfiguration.
     /// Meant for adjusting the module state based on configuration changes.
     virtual void syncConfig() {}

=== modified file 'src/dns/forward.h'
--- src/dns/forward.h	2016-10-01 12:02:44 +0000
+++ src/dns/forward.h	2016-10-02 09:35:36 +0000
@@ -22,7 +22,6 @@
 class LookupDetails;
 
 void Init(void);
-void Shutdown(void);
 
 } // namespace Dns
 

=== modified file 'src/dns_internal.cc'
--- src/dns_internal.cc	2016-04-22 11:39:23 +0000
+++ src/dns_internal.cc	2016-10-02 12:18:51 +0000
@@ -10,6 +10,7 @@
 
 #include "squid.h"
 #include "base/InstanceId.h"
+#include "base/RunnersRegistry.h"
 #include "comm.h"
 #include "comm/Connection.h"
 #include "comm/ConnOpener.h"
@@ -209,6 +210,21 @@
     nsvc *vc;
 };
 
+namespace Dns
+{
+
+/// manage DNS internal component
+class ConfigRr : public RegisteredRunner
+{
+public:
+    virtual void startReconfigure() override;
+    virtual void endingShutdown() override;
+};
+
+RunnerRegistrationEntry(ConfigRr);
+
+} // namespace Dns
+
 struct _sp {
     char domain[NS_MAXDNAME];
     int queries;
@@ -903,7 +919,7 @@
 {
     delete queue;
     delete msg;
-    if (ns < nns) // XXX: Dns::Shutdown may have freed nameservers[]
+    if (ns < nns) // XXX: idnsShutdownAndFreeState may have freed nameservers[]
         nameservers[ns].vc = NULL;
 }
 
@@ -1641,12 +1657,14 @@
     Mgr::RegisterAction("idns", "Internal DNS Statistics", idnsStats, 0, 1);
 }
 
-void
-Dns::Shutdown(void)
+static void
+idnsShutdownAndFreeState(const char *reason)
 {
     if (DnsSocketA < 0 && DnsSocketB < 0)
         return;
 
+    debugs(78, DBG_IMPORTANT, reason << ": Closing DNS sockets");
+
     if (DnsSocketA >= 0 ) {
         comm_close(DnsSocketA);
         DnsSocketA = -1;
@@ -1669,6 +1687,18 @@
     idnsFreeSearchpath();
 }
 
+void
+Dns::ConfigRr::endingShutdown()
+{
+    idnsShutdownAndFreeState("Shutdown");
+}
+
+void
+Dns::ConfigRr::startReconfigure()
+{
+    idnsShutdownAndFreeState("Reconfigure");
+}
+
 static int
 idnsCachedLookup(const char *key, IDNSCB * callback, void *data)
 {

=== modified file 'src/main.cc'
--- src/main.cc	2016-06-25 14:35:41 +0000
+++ src/main.cc	2016-10-02 12:19:21 +0000
@@ -851,13 +851,14 @@
     debugs(1, DBG_IMPORTANT, "Reconfiguring Squid Cache (version " << version_string << ")...");
     reconfiguring = 1;
 
+    RunRegisteredHere(RegisteredRunner::startReconfigure);
+
     // Initiate asynchronous closing sequence
     serverConnectionsClose();
     icpClosePorts();
 #if USE_HTCP
     htcpClosePorts();
 #endif
-    Dns::Shutdown();
 #if USE_SSL_CRTD
     Ssl::Helper::GetInstance()->Shutdown();
 #endif
@@ -2006,7 +2007,6 @@
 #endif
 
     debugs(1, DBG_IMPORTANT, "Shutting down...");
-    Dns::Shutdown();
 #if USE_SSL_CRTD
     Ssl::Helper::GetInstance()->Shutdown();
 #endif