Wrap the webserver's and Resolver::tryGetSOASerial objects into smart pointers

pdns/resolver.cc

@@ -217,7 +217,7 @@ static int parseResult(MOADNSParser& mdp, const DNSName& origQname, uint16_t ori
 
 bool Resolver::tryGetSOASerial(DNSName *domain, uint32_t *theirSerial, uint32_t *theirInception, uint32_t *theirExpire, uint16_t* id)
 {
-  struct pollfd *fds = new struct pollfd[locals.size()];
+  auto fds = std::unique_ptr<struct pollfd[]>(new struct pollfd[locals.size()]);
   size_t i = 0, k;
   int sock;
 
@@ -226,8 +226,7 @@ bool Resolver::tryGetSOASerial(DNSName *domain, uint32_t *theirSerial, uint32_t
     fds[i].events = POLLIN;
   }
 
-  if (poll(fds, i, 250) < 1) { // wait for 0.25s
-    delete [] fds;
+  if (poll(fds.get(), i, 250) < 1) { // wait for 0.25s
     return false;
   }
 
@@ -241,8 +240,6 @@ bool Resolver::tryGetSOASerial(DNSName *domain, uint32_t *theirSerial, uint32_t
     }
   }
 
-  delete [] fds;
-
   if (sock < 0) return false; // false alarm
 
   int err;

pdns/webserver.cc

@@ -25,6 +25,7 @@
 #include "utility.hh"
 #include "webserver.hh"
 #include "misc.hh"
+#include <thread>
 #include <vector>
 #include "logger.hh"
 #include <stdio.h>
@@ -34,11 +35,6 @@
 #include "arguments.hh"
 #include <yahttp/router.hpp>
 
-struct connectionThreadData {
-  WebServer* webServer{nullptr};
-  Socket* client{nullptr};
-};
-
 json11::Json HttpRequest::json()
 {
   string err;
@@ -198,18 +194,12 @@ void WebServer::registerWebHandler(const string& url, HandlerFunction handler) {
   registerBareHandler(url, f);
 }
 
-static void *WebServerConnectionThreadStart(void *p) {
-  connectionThreadData* data = static_cast<connectionThreadData*>(p);
-  pthread_detach(pthread_self());
-  data->webServer->serveConnection(data->client);
-
-  delete data->client; // close socket
-  delete data;
-
-  return NULL;
+static void *WebServerConnectionThreadStart(const WebServer* webServer, std::shared_ptr<Socket> client) {
+  webServer->serveConnection(client);
+  return nullptr;
 }
 
-void WebServer::handleRequest(HttpRequest& req, HttpResponse& resp)
+void WebServer::handleRequest(HttpRequest& req, HttpResponse& resp) const
 {
   // set default headers
   resp.headers["Content-Type"] = "text/html; charset=utf-8";
@@ -286,7 +276,7 @@ void WebServer::handleRequest(HttpRequest& req, HttpResponse& resp)
   }
 }
 
-void WebServer::serveConnection(Socket *client)
+void WebServer::serveConnection(std::shared_ptr<Socket> client) const
 try {
   HttpRequest req;
   YaHTTP::AsyncRequestLoader yarl;
@@ -331,7 +321,7 @@ catch(...) {
   L<<Logger::Error<<"HTTP: Unknown exception"<<endl;
 }
 
-WebServer::WebServer(const string &listenaddress, int port) : d_server(NULL)
+WebServer::WebServer(const string &listenaddress, int port) : d_server(nullptr)
 {
   d_listenaddress=listenaddress;
   d_port=port;
@@ -345,7 +335,7 @@ void WebServer::bind()
   }
   catch(NetworkError &e) {
     L<<Logger::Error<<"Listening on HTTP socket failed: "<<e.what()<<endl;
-    d_server = NULL;
+    d_server = nullptr;
   }
 }
 
@@ -354,41 +344,29 @@ void WebServer::go()
   if(!d_server)
     return;
   try {
-    pthread_t tid;
-
     NetmaskGroup acl;
     acl.toMasks(::arg()["webserver-allow-from"]);
 
     while(true) {
-      // data and data->client will be freed by thread
-      connectionThreadData *data = new connectionThreadData;
-      data->webServer = this;
       try {
-        data->client = d_server->accept();
-        if (data->client->acl(acl)) {
-          pthread_create(&tid, 0, &WebServerConnectionThreadStart, (void *)data);
+        auto client = d_server->accept();
+        if (client->acl(acl)) {
+          std::thread webHandler(WebServerConnectionThreadStart, this, client);
+          webHandler.detach();
         } else {
           ComboAddress remote;
-          if (data->client->getRemote(remote))
+          if (client->getRemote(remote))
             L<<Logger::Error<<"Webserver closing socket: remote ("<< remote.toString() <<") does not match 'webserver-allow-from'"<<endl;
-          delete data->client; // close socket
-          delete data;
         }
       }
       catch(PDNSException &e) {
         L<<Logger::Error<<"PDNSException while accepting a connection in main webserver thread: "<<e.reason<<endl;
-        delete data->client;
-        delete data;
       }
       catch(std::exception &e) {
         L<<Logger::Error<<"STL Exception while accepting a connection in main webserver thread: "<<e.what()<<endl;
-        delete data->client;
-        delete data;
       }
       catch(...) {
         L<<Logger::Error<<"Unknown exception while accepting a connection in main webserver thread"<<endl;
-        delete data->client;
-        delete data;
       }
     }
   }

pdns/webserver.hh

@@ -122,11 +122,12 @@ public:
     d_server_socket.bind(d_local);
     d_server_socket.listen();
   }
+  virtual ~Server() { };
 
   ComboAddress d_local;
 
-  Socket *accept() {
-    return d_server_socket.accept();
+  std::shared_ptr<Socket> accept() {
+    return std::shared_ptr<Socket>(d_server_socket.accept());
   }
 
 protected:
@@ -137,11 +138,12 @@ class WebServer : public boost::noncopyable
 {
 public:
   WebServer(const string &listenaddress, int port);
+  virtual ~WebServer() { };
   void bind();
   void go();
 
-  void serveConnection(Socket *client);
-  void handleRequest(HttpRequest& request, HttpResponse& resp);
+  void serveConnection(std::shared_ptr<Socket> client) const;
+  void handleRequest(HttpRequest& request, HttpResponse& resp) const;
 
   typedef boost::function<void(HttpRequest* req, HttpResponse* resp)> HandlerFunction;
   void registerApiHandler(const string& url, HandlerFunction handler);
@@ -150,14 +152,14 @@ public:
 protected:
   void registerBareHandler(const string& url, HandlerFunction handler);
 
-  virtual Server* createServer() {
-    return new Server(d_listenaddress, d_port);
+  virtual std::shared_ptr<Server> createServer() {
+    return std::make_shared<Server>(d_listenaddress, d_port);
   }
 
   string d_listenaddress;
   int d_port;
   string d_password;
-  Server* d_server;
+  std::shared_ptr<Server> d_server;
 };
 
 #endif /* WEBSERVER_HH */

pdns/ws-recursor.cc

@@ -379,7 +379,7 @@ static void apiServerCacheFlush(HttpRequest* req, HttpResponse* resp) {
 
 #include "htmlfiles.h"
 
-void serveStuff(HttpRequest* req, HttpResponse* resp) 
+static void serveStuff(HttpRequest* req, HttpResponse* resp)
 {
   resp->headers["Cache-Control"] = "max-age=86400";
 
@@ -540,9 +540,8 @@ void RecursorWebServer::jsonstat(HttpRequest* req, HttpResponse *resp)
 void AsyncServerNewConnectionMT(void *p) {
   AsyncServer *server = (AsyncServer*)p;
   try {
-    Socket* socket = server->accept();
+    auto socket = server->accept();
     server->d_asyncNewConnectionCallback(socket);
-    delete socket;
   } catch (NetworkError &e) {
     // we're running in a shared process/thread, so can't just terminate/abort.
     return;
@@ -561,7 +560,7 @@ void AsyncServer::newConnection()
 }
 
 // This is an entry point from FDM, so it needs to catch everything.
-void AsyncWebServer::serveConnection(Socket *client)
+void AsyncWebServer::serveConnection(std::shared_ptr<Socket> client) const
 try {
   HttpRequest req;
   YaHTTP::AsyncRequestLoader yarl;
@@ -571,7 +570,7 @@ try {
   string data;
   try {
     while(!req.complete) {
-      int bytes = arecvtcp(data, 16384, client, true);
+      int bytes = arecvtcp(data, 16384, client.get(), true);
       if (bytes > 0) {
         req.complete = yarl.feed(data);
       } else {
@@ -591,7 +590,7 @@ try {
   data = ss.str();
 
   // now send the reply
-  if (asendtcp(data, client) == -1 || data.empty()) {
+  if (asendtcp(data, client.get()) == -1 || data.empty()) {
     L<<Logger::Error<<"Failed sending reply to HTTP client"<<endl;
   }
 }
@@ -609,5 +608,8 @@ catch(...) {
 void AsyncWebServer::go() {
   if (!d_server)
     return;
-  ((AsyncServer*)d_server)->asyncWaitForConnections(d_fdm, boost::bind(&AsyncWebServer::serveConnection, this, _1));
+  auto server = std::dynamic_pointer_cast<AsyncServer>(d_server);
+  if (!server)
+    return;
+  server->asyncWaitForConnections(d_fdm, boost::bind(&AsyncWebServer::serveConnection, this, _1));
 }

pdns/ws-recursor.hh

@@ -36,7 +36,7 @@ public:
 
   friend void AsyncServerNewConnectionMT(void *p);
 
-  typedef boost::function< void(Socket*) > newconnectioncb_t;
+  typedef boost::function< void(std::shared_ptr<Socket>) > newconnectioncb_t;
   void asyncWaitForConnections(FDMultiplexer* fdm, const newconnectioncb_t& callback);
 
 private:
@@ -54,11 +54,11 @@ public:
 
 private:
   FDMultiplexer* d_fdm;
-  void serveConnection(Socket *socket);
+  void serveConnection(std::shared_ptr<Socket> socket) const;
 
 protected:
-  virtual Server* createServer() {
-    return new AsyncServer(d_listenaddress, d_port);
+  virtual std::shared_ptr<Server> createServer() override {
+    return std::make_shared<AsyncServer>(d_listenaddress, d_port);
   };
 };