]> granicus.if.org Git - pdns/commitdiff
simplify StateHolder API and make std::shared_ptr an invisible implementation detail
authorbert hubert <bert.hubert@netherlabs.nl>
Tue, 10 Mar 2015 12:20:54 +0000 (13:20 +0100)
committerbert hubert <bert.hubert@netherlabs.nl>
Tue, 10 Mar 2015 12:20:54 +0000 (13:20 +0100)
pdns/dnsdist-lua.cc
pdns/dnsdist.cc
pdns/sholder.hh

index 5ef4048e73ad74b4502afbec609da758788d562e..20f510634a0955ce32c48ec5ae033f842ea26202 100644 (file)
@@ -72,8 +72,8 @@ vector<std::function<void(void)>> setupLua(bool client)
                        }
 
                        auto states = g_dstates.getCopy();
-                       states->push_back(ret);
-                       std::stable_sort(states->begin(), states->end(), [](const decltype(ret)& a, const decltype(ret)& b) {
+                       states.push_back(ret);
+                       std::stable_sort(states.begin(), states.end(), [](const decltype(ret)& a, const decltype(ret)& b) {
                            return a->order < b->order;
                          });
                        g_dstates.setState(states);
@@ -88,19 +88,19 @@ vector<std::function<void(void)>> setupLua(bool client)
                      { 
                        auto states = g_dstates.getCopy();
                        if(auto* rem = boost::get<shared_ptr<DownstreamState>>(&var))
-                         states->erase(remove(states->begin(), states->end(), *rem), states->end());
+                         states.erase(remove(states.begin(), states.end(), *rem), states.end());
                        else
-                         states->erase(states->begin() + boost::get<int>(var));
+                         states.erase(states.begin() + boost::get<int>(var));
                        g_dstates.setState(states);
                      } );
 
 
   g_lua.writeFunction("setServerPolicy", [](ServerPolicy policy)  {
-      g_policy.setState(std::make_shared<ServerPolicy>(policy));
+      g_policy.setState(policy);
     });
 
   g_lua.writeFunction("setServerPolicyLua", [](string name, policy_t policy)  {
-      g_policy.setState(std::make_shared<ServerPolicy>(ServerPolicy{name, policy}));
+      g_policy.setState(ServerPolicy{name, policy});
     });
 
   g_lua.writeFunction("showServerPolicy", []() {
@@ -116,9 +116,7 @@ vector<std::function<void(void)>> setupLua(bool client)
   g_lua.writeVariable("wrandom", ServerPolicy{"wrandom", wrandom});
   g_lua.writeVariable("leastOutstanding", ServerPolicy{"leastOutstanding", leastOutstanding});
   g_lua.writeFunction("addACL", [](const std::string& domain) {
-      auto state=g_ACL.getCopy();
-      state->addMask(domain);
-      g_ACL.setState(state);
+      g_ACL.modify([domain](NetmaskGroup& nmg) { nmg.addMask(domain); });
     });
 
   g_lua.writeFunction("addLocal", [client](const std::string& addr) {
@@ -133,16 +131,16 @@ vector<std::function<void(void)>> setupLua(bool client)
       }
     });
   g_lua.writeFunction("setACL", [](const vector<pair<int, string>>& parts) {
-      auto nmg = std::make_shared<NetmaskGroup>();
+      NetmaskGroup nmg;
       for(const auto& p : parts) {
-       nmg->addMask(p.second);
+       nmg.addMask(p.second);
       }
       g_ACL.setState(nmg);
   });
   g_lua.writeFunction("showACL", []() {
       vector<string> vec;
 
-      g_ACL.getCopy()->toStringVector(&vec);
+      g_ACL.getCopy().toStringVector(&vec);
 
       for(const auto& s : vec)
         g_outputBuffer+=s+"\n";
@@ -167,7 +165,7 @@ vector<std::function<void(void)>> setupLua(bool client)
       uint64_t totQPS{0}, totQueries{0}, totDrops{0};
       int counter=0;
       auto states = g_dstates.getCopy();
-      for(const auto& s : *states) {
+      for(const auto& s : states) {
        string status;
        if(s->availability == DownstreamState::Availability::Up) 
          status = "UP";
@@ -233,7 +231,7 @@ vector<std::function<void(void)>> setupLua(bool client)
       boost::format fmt("%-3d %-50s %s\n");
       g_outputBuffer += (fmt % "#" % "Object" % "Pool").str();
       int num=0;
-      for(const auto& lim : *g_poolrules.getCopy()) {  
+      for(const auto& lim : g_poolrules.getCopy()) {  
        string name;
        if(auto nmg=boost::get<NetmaskGroup>(&lim.first)) {
          name=nmg->toString();
@@ -285,7 +283,7 @@ vector<std::function<void(void)>> setupLua(bool client)
       boost::format fmt("%-3d %-50s %7d %8d %8d\n");
       g_outputBuffer += (fmt % "#" % "Object" % "Lim" % "Passed" % "Blocked").str();
       int num=0;
-      for(const auto& lim : *g_limiters.getCopy()) {  
+      for(const auto& lim : g_limiters.getCopy()) {  
        string name;
        if(auto nmg=boost::get<NetmaskGroup>(&lim.first)) {
          name=nmg->toString();
@@ -302,13 +300,13 @@ vector<std::function<void(void)>> setupLua(bool client)
   g_lua.writeFunction("getServers", []() {
       vector<pair<int, std::shared_ptr<DownstreamState> > > ret;
       int count=1;
-      for(const auto& s : *g_dstates.getCopy()) {
+      for(const auto& s : g_dstates.getCopy()) {
        ret.push_back(make_pair(count++, s));
       }
       return ret;
     });
 
-  g_lua.writeFunction("getServer", [](int i) { return (*g_dstates.getCopy())[i]; });
+  g_lua.writeFunction("getServer", [](int i) { return g_dstates.getCopy().at(i); });
 
   g_lua.registerFunction<void(DownstreamState::*)(int)>("setQPS", [](DownstreamState& s, int lim) { s.qps = lim ? QPSLimiter(lim, lim) : QPSLimiter(); });
   g_lua.registerFunction<void(DownstreamState::*)(string)>("addPool", [](DownstreamState& s, string pool) { s.pools.insert(pool);});
index 40a9008deb8e43247e560201fbc9634fb407f1a0..44644aeba1562ad55889c700ead34394c61f522a 100644 (file)
 /* Known sins:
    No centralized statistics
    We neglect to do recvfromto() on 0.0.0.0
-   Receiver is currently singlethreaded (not that bad actually)
+   Receiver is currently singlethreaded
+      not *that* bad actually, but now that we are thread safe, might want to scale
    lack of help()
+   lack of autocomplete
 */
 
 namespace po = boost::program_options;
@@ -254,7 +256,7 @@ ComboAddress g_serverControl{"127.0.0.1:5199"};
 servers_t getDownstreamCandidates(const std::string& pool)
 {
   servers_t ret;
-  for(const auto& s : *g_dstates.getCopy()) 
+  for(const auto& s : g_dstates.getCopy()) 
     if((pool.empty() && s->pools.empty()) || s->pools.count(pool))
       ret.push_back(s);
   
@@ -444,11 +446,11 @@ catch(...)
 
 int getTCPDownstream(DownstreamState** ds, const ComboAddress& remote, const DNSName& qname, uint16_t qtype, dnsheader* dh)
 {
-  auto policy=g_policy.getCopy()->policy;
+  auto policy=g_policy.getCopy().policy;
   
   {
     std::lock_guard<std::mutex> lock(g_luamutex);
-    *ds = policy(*g_dstates.getCopy(), remote, qname, qtype, dh).get(); // XXX I think this misses pool selection!
+    *ds = policy(g_dstates.getCopy(), remote, qname, qtype, dh).get(); // XXX I think this misses pool selection!
   }
   
   vinfolog("TCP connecting to downstream %s", (*ds)->remote.toStringWithPort());
@@ -660,14 +662,11 @@ void* maintThread()
     if(g_tcpclientthreads.d_queued > 1 && g_tcpclientthreads.d_numthreads < 10)
       g_tcpclientthreads.addTCPClientThread();
 
-    for(auto& dss : *(g_dstates.getCopy())) { // this points to the actual shared_ptrs!
+    for(auto& dss : g_dstates.getCopy()) { // this points to the actual shared_ptrs!
       if(dss->availability==DownstreamState::Availability::Auto) {
        bool newState=upCheck(dss->remote);
        if(newState != dss->upStatus) {
-         cout<<endl;
          warnlog("Marking downstream %s as '%s'", dss->remote.toStringWithPort(), newState ? "up" : "down");
-         cout<<"> ";
-         cout.flush();
        }
        dss->upStatus = newState;
       }
@@ -954,7 +953,7 @@ try
 
   ServerPolicy leastOutstandingPol{"leastOutstanding", leastOutstanding};
 
-  g_policy.setState(std::make_shared<ServerPolicy>(leastOutstandingPol));
+  g_policy.setState(leastOutstandingPol);
   if(g_vm.count("client") || g_vm.count("command")) {
     setupLua(true);
     doClient(g_serverControl);
@@ -998,7 +997,7 @@ try
 
   auto acl = g_ACL.getCopy();
   for(auto& addr : {"127.0.0.0/8", "10.0.0.0/8", "100.64.0.0/10", "169.254.0.0/16", "192.168.0.0/16", "172.16.0.0/12", "::1/128", "fc00::/7", "fe80::/10"})
-    acl->addMask(addr);
+    acl.addMask(addr);
   g_ACL.setState(acl);
 
   if(g_vm.count("remotes")) {
@@ -1009,7 +1008,7 @@ try
     }
   }
 
-  for(auto& dss : *g_dstates.getCopy()) {
+  for(auto& dss : g_dstates.getCopy()) { // it is a copy, but the internal shared_ptrs are the real deal
     if(dss->availability==DownstreamState::Availability::Auto) {
       bool newState=upCheck(dss->remote);
       warnlog("Marking downstream %s as '%s'", dss->remote.toStringWithPort(), newState ? "up" : "down");
index ef57cd33e75666e54db027749d1da780ebbdd6c0..3fa17eb4928e3d9e49b713bf03d0bad565f12c72 100644 (file)
@@ -35,7 +35,7 @@ public:
   explicit LocalStateHolder(GlobalStateHolder<T>* source) : d_source(source)
   {}
 
-  const T* operator->()  // only const access, but see "read-only" above
+  const T* operator->()  // fast const-only access, but see "read-only" above
   {
     if(d_source->getGeneration() != d_generation) {
       d_source->getState(&d_state, & d_generation);
@@ -43,13 +43,9 @@ public:
 
     return d_state.get();
   }
-  const T& operator*()  // only const access, but see "read-only" above
+  const T& operator*()  // fast const-only access, but see "read-only" above
   {
-    if(d_source->getGeneration() != d_generation) {
-      d_source->getState(&d_state, &d_generation);
-    }
-
-    return *d_state;
+    return *operator->();
   }
 
   void reset()
@@ -73,29 +69,21 @@ public:
   {
     return LocalStateHolder<T>(this);
   }
-  void setState(std::shared_ptr<T> state)
+
+  void setState(T state) //!< Safely & slowly change the global state
   {
     std::lock_guard<std::mutex> l(d_lock);
-    d_state = state;
+    d_state = std::make_shared<T>(T(state));
     d_generation++;
   }
-  unsigned int getGeneration() const
-  {
-    return d_generation;
-  }
-  void getState(std::shared_ptr<T>* state, unsigned int* generation) const
-  {
-    std::lock_guard<std::mutex> l(d_lock);
-    *state=d_state;
-    *generation = d_generation;
-  }
-  std::shared_ptr<T> getCopy() const
+
+  T getCopy() const  //!< Safely & slowly get a copy of the global state
   {
     std::lock_guard<std::mutex> l(d_lock);
-    shared_ptr<T> ret = shared_ptr<T>(new T(*d_state));
-    return d_state;
+    return *d_state;
   }
   
+  //! Safely & slowly modify the global state
   template<typename F>
   void modify(F act) {
     std::lock_guard<std::mutex> l(d_lock);
@@ -106,6 +94,17 @@ public:
 
   typedef T value_type;
 private:
+  unsigned int getGeneration() const
+  {
+    return d_generation;
+  }
+  void getState(std::shared_ptr<T>* state, unsigned int* generation) const
+  {
+    std::lock_guard<std::mutex> l(d_lock);
+    *state=d_state;
+    *generation = d_generation;
+  }
+  friend class LocalStateHolder<T>;
   mutable std::mutex d_lock;
   std::shared_ptr<T> d_state;
   std::atomic<unsigned int> d_generation{1};