From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Tue, 4 Jul 2017 13:51:12 +0000 (+0200)
Subject: Wrap the webserver's objects into smart pointers
X-Git-Tag: dnsdist-1.2.0~52^2~2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d4c53d8c770057f7bc4585e6d6b47074d9f739bf;p=pdns

Wrap the webserver's objects into smart pointers
---

diff --git a/pdns/webserver.cc b/pdns/webserver.cc
index f6c77c06b..d89268cb5 100644
--- a/pdns/webserver.cc
+++ b/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>
@@ -36,7 +37,7 @@
 
 struct connectionThreadData {
   WebServer* webServer{nullptr};
-  Socket* client{nullptr};
+  std::shared_ptr<Socket> client{nullptr};
 };
 
 json11::Json HttpRequest::json()
@@ -198,15 +199,9 @@ 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());
+static void *WebServerConnectionThreadStart(std::shared_ptr<connectionThreadData> data) {
   data->webServer->serveConnection(data->client);
-
-  delete data->client; // close socket
-  delete data;
-
-  return NULL;
+  return nullptr;
 }
 
 void WebServer::handleRequest(HttpRequest& req, HttpResponse& resp)
@@ -286,7 +281,7 @@ void WebServer::handleRequest(HttpRequest& req, HttpResponse& resp)
   }
 }
 
-void WebServer::serveConnection(Socket *client)
+void WebServer::serveConnection(std::shared_ptr<Socket> client)
 try {
   HttpRequest req;
   YaHTTP::AsyncRequestLoader yarl;
@@ -354,41 +349,32 @@ 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;
+      auto data = std::make_shared<connectionThreadData>();
       data->webServer = this;
       try {
         data->client = d_server->accept();
         if (data->client->acl(acl)) {
-          pthread_create(&tid, 0, &WebServerConnectionThreadStart, (void *)data);
+          std::thread webHandler(WebServerConnectionThreadStart, data);
+          webHandler.detach();
         } else {
           ComboAddress remote;
           if (data->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;
       }
     }
   }
diff --git a/pdns/webserver.hh b/pdns/webserver.hh
index d784cdf9f..3972162bc 100644
--- a/pdns/webserver.hh
+++ b/pdns/webserver.hh
@@ -126,8 +126,8 @@ public:
 
   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:
@@ -142,7 +142,7 @@ public:
   void bind();
   void go();
 
-  void serveConnection(Socket *client);
+  void serveConnection(std::shared_ptr<Socket> client);
   void handleRequest(HttpRequest& request, HttpResponse& resp);
 
   typedef boost::function<void(HttpRequest* req, HttpResponse* resp)> HandlerFunction;
diff --git a/pdns/ws-recursor.cc b/pdns/ws-recursor.cc
index c495918d7..404c5f4f7 100644
--- a/pdns/ws-recursor.cc
+++ b/pdns/ws-recursor.cc
@@ -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)
 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;
   }
 }
diff --git a/pdns/ws-recursor.hh b/pdns/ws-recursor.hh
index f4a91cf74..8b6974f91 100644
--- a/pdns/ws-recursor.hh
+++ b/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,7 +54,7 @@ public:
 
 private:
   FDMultiplexer* d_fdm;
-  void serveConnection(Socket *socket);
+  void serveConnection(std::shared_ptr<Socket> socket);
 
 protected:
   virtual std::shared_ptr<Server> createServer() override {