RequestManager ist jetzt Thread-sicher

2 files changed, 110 insertions, 117 deletions

diff --git a/src/Common/RequestManager.cpp b/src/Common/RequestManager.cpp
index c36bd76..8940339 100644
--- a/src/Common/RequestManager.cpp
+++ b/src/Common/RequestManager.cpp
@@ -29,12 +29,7 @@ namespace Common {
 RequestManager RequestManager::requestManager;
 
 
-RequestManager::RequestMap::~RequestMap() {
-  for(iterator it = begin(); it != end(); ++it)
-    delete it->second;
-}
-
-bool RequestManager::RequestMap::addRequest(uint16_t id, RequestHandler *info) {
+bool RequestManager::RequestMap::addRequest(boost::uint16_t id, boost::shared_ptr<RequestHandler> info) {
   if(!insert(std::make_pair(id, info)).second)
     return false;
 
@@ -43,28 +38,23 @@ bool RequestManager::RequestMap::addRequest(uint16_t id, RequestHandler *info) {
   return true;
 }
 
-RequestHandler* RequestManager::RequestMap::findRequest(uint16_t id) {
+boost::shared_ptr<RequestHandler> RequestManager::RequestMap::findRequest(boost::uint16_t id) {
   iterator it = find(id);
   if(it == end())
-    return 0;
+    return boost::shared_ptr<RequestHandler>();
 
   return it->second;
 }
 
-bool RequestManager::RequestMap::deleteRequest(uint16_t id) {
-  iterator it = find(id);
-  if(it == end())
-    return false;
-
-  delete it->second;
-
-  erase(it);
-  return true;
+bool RequestManager::RequestMap::deleteRequest(boost::uint16_t id) {
+  return erase(id);
 }
 
 
-void RequestManager::receiveHandler(Connection *connection, const XmlPacket &packet, uint16_t requestId) {
-  std::map<Connection*,RequestMap*>::iterator it = requestMaps.find(connection);
+void RequestManager::receiveHandler(Connection *connection, const XmlPacket &packet, boost::uint16_t requestId) {
+  boost::upgrade_lock<boost::shared_mutex> lock(mutex);
+
+  std::map<Connection*, boost::shared_ptr<RequestMap> >::iterator it = requestMaps.find(connection);
   if(it == requestMaps.end()) {
     // TODO: Error
     Logger::log(Logger::ERROR, "Received a packet from an unregistered connection.");
@@ -72,25 +62,33 @@ void RequestManager::receiveHandler(Connection *connection, const XmlPacket &pac
     return;
   }
 
-  RequestMap *requestMap = it->second;
-  RequestHandler *request = requestMap->findRequest(requestId);
+  boost::shared_ptr<RequestMap> requestMap = it->second;
+  boost::shared_ptr<RequestHandler> request = requestMap->findRequest(requestId);
 
   if(request) {
+    lock.unlock();
     request->handlePacket(packet);
 
     return;
   }
 
-  std::map<std::string,RequestHandlerFactory*>::iterator factoryIt = requestHandlerFactories.find(packet.getType());
+  std::map<std::string, boost::shared_ptr<RequestHandlerFactory> >::iterator factoryIt = requestHandlerFactories.find(packet.getType());
   if(factoryIt != requestHandlerFactories.end()) {
-    request = factoryIt->second->createRequestHandler(connection, requestId);
+    {
+      boost::upgrade_to_unique_lock<boost::shared_mutex> upgradeLock(lock);
+
+      request = factoryIt->second->createRequestHandler(connection, requestId);
+      requestMap->addRequest(requestId, request);
+    }
 
-    requestMap->addRequest(requestId, request);
+    lock.unlock();
     request->handlePacket(packet);
 
     return;
   }
 
+  lock.unlock();
+
   Logger::logf(Logger::ERROR, "Received an unexpected packet with type '%s'.", packet.getType().c_str());
 
   XmlPacket ret;
@@ -100,62 +98,34 @@ void RequestManager::receiveHandler(Connection *connection, const XmlPacket &pac
   connection->sendPacket(ret, requestId);
 }
 
-RequestManager::RequestMap* RequestManager::getUnusedRequestId(Connection *connection, uint16_t *requestId) {
-  std::map<Connection*,RequestMap*>::iterator it = requestMaps.find(connection);
+boost::shared_ptr<RequestManager::RequestMap> RequestManager::_getUnusedRequestId(Connection *connection, boost::uint16_t *requestId) {
+  std::map<Connection*, boost::shared_ptr<RequestMap> >::iterator it = requestMaps.find(connection);
 
   if(it == requestMaps.end()) {
     Logger::log(Logger::CRITICAL, "Trying to send a request over an unregistered connecion.");
-    return 0;
+    return boost::shared_ptr<RequestMap>();
   }
 
-  RequestMap *requestMap = it->second;
+  boost::shared_ptr<RequestMap> requestMap = it->second;
 
   do {
-    *requestId = getRequestId();
+    *requestId = _getRequestId();
   } while(requestMap->findRequest(*requestId));
 
   return requestMap;
 }
 
 void RequestManager::registerConnection(Connection *connection) {
+  boost::lock_guard<boost::shared_mutex> lock(mutex);
+
   requestMaps.insert(std::make_pair(connection, new RequestMap()));
 
   connection->signalReceive().connect(boost::bind(&RequestManager::receiveHandler, this, connection, _1, _2));
 }
 
-void RequestManager::unregisterConnection(Connection *connection) {
-  std::map<Connection*,RequestMap*>::iterator it = requestMaps.find(connection);
-
-  if(it != requestMaps.end()) {
-    delete it->second;
-    requestMaps.erase(it);
-  }
-}
-
-void RequestManager::unregisterPacketType(const std::string &type) {
-  std::map<std::string,RequestHandlerFactory*>::iterator it = requestHandlerFactories.find(type);
-
-  if(it == requestHandlerFactories.end())
-    return;
-
-  delete it->second;
-
-  requestHandlerFactories.erase(it);
-}
-
 RequestManager::RequestManager() : server(false), lastRequestId(-1) {
   registerPacketType<RequestHandlers::DisconnectRequestHandler>("Disconnect");
 }
 
-RequestManager::~RequestManager() {
-  unregisterPacketType("Disconnect");
-
-  for(std::map<Connection*,RequestMap*>::iterator it = requestMaps.begin(); it != requestMaps.end(); ++it)
-    delete it->second;
-
-  for(std::map<std::string,RequestHandlerFactory*>::iterator it = requestHandlerFactories.begin(); it != requestHandlerFactories.end(); ++it)
-    delete it->second;
-}
-
 }
 }
diff --git a/src/Common/RequestManager.h b/src/Common/RequestManager.h
index e9ce189..d9e7fbb 100644
--- a/src/Common/RequestManager.h
+++ b/src/Common/RequestManager.h
@@ -26,7 +26,8 @@
 #include <memory>
 #include <string>
 
-#include <stdint.h>
+#include <boost/thread/locks.hpp>
+#include <boost/thread/shared_mutex.hpp>
 
 namespace Mad {
 
@@ -36,21 +37,13 @@ class Packet;
 
 namespace Common {
 
-class RequestManager {
+class RequestManager : boost::noncopyable {
   private:
-    class RequestMap : private std::map<uint16_t,RequestHandler*> {
-      private:
-        // Prevent shallow copy
-        RequestMap(const RequestMap &o);
-        RequestMap& operator=(const RequestMap &o);
-
+    class RequestMap : private std::map<boost::uint16_t, boost::shared_ptr<RequestHandler> >, boost::noncopyable {
       public:
-        RequestMap() {}
-        ~RequestMap();
-
-        bool addRequest(uint16_t id, RequestHandler *info);
-        RequestHandler* findRequest(uint16_t id);
-        bool deleteRequest(uint16_t id);
+        bool addRequest(boost::uint16_t id, boost::shared_ptr<RequestHandler> info);
+        boost::shared_ptr<RequestHandler> findRequest(boost::uint16_t id);
+        bool deleteRequest(boost::uint16_t id);
     };
 
     class RequestHandlerFactory {
@@ -58,36 +51,34 @@ class RequestManager {
         RequestHandlerFactory() {}
 
       public:
-        virtual RequestHandler* createRequestHandler(Connection *connection, uint16_t requestId) = 0;
+        virtual boost::shared_ptr<RequestHandler> createRequestHandler(Connection *connection, boost::uint16_t requestId) = 0;
         virtual ~RequestHandlerFactory() {}
     };
 
     template<class T> class SpecificRequestHandlerFactory : public RequestHandlerFactory {
       public:
-        virtual RequestHandler* createRequestHandler(Connection *connection, uint16_t requestId) {
-          return new T(connection, requestId);
+        virtual boost::shared_ptr<RequestHandler> createRequestHandler(Connection *connection, boost::uint16_t requestId) {
+          return boost::shared_ptr<RequestHandler>(new T(connection, requestId));
         }
     };
 
     static RequestManager requestManager;
 
-    std::map<Connection*,RequestMap*> requestMaps;
+    boost::shared_mutex mutex;
+
+    std::map<Connection*, boost::shared_ptr<RequestMap> > requestMaps;
     bool server;
-    uint16_t lastRequestId;
+    boost::uint16_t lastRequestId;
 
-    std::map<std::string,RequestHandlerFactory*> requestHandlerFactories;
+    std::map<std::string, boost::shared_ptr<RequestHandlerFactory> > requestHandlerFactories;
 
-    uint16_t getRequestId() {
+    boost::uint16_t _getRequestId() {
       return lastRequestId+=2;
     }
 
-    // Prevent shallow copy
-    RequestManager(const RequestManager &o);
-    RequestManager& operator=(const RequestManager &o);
-
-    void receiveHandler(Connection *connection, const XmlPacket &packet, uint16_t requestId);
+    void receiveHandler(Connection *connection, const XmlPacket &packet, boost::uint16_t requestId);
 
-    RequestMap* getUnusedRequestId(Connection *connection, uint16_t *requestId);
+    boost::shared_ptr<RequestMap> _getUnusedRequestId(Connection *connection, boost::uint16_t *requestId);
 
     bool send(Request *request);
 
@@ -98,8 +89,14 @@ class RequestManager {
       return &requestManager;
     }
 
-    bool isServer() const {return server;}
+    bool isServer() {
+      boost::shared_lock<boost::shared_mutex> lock(mutex);
+      return server;
+    }
+
     void setServer(bool newServer) {
+      boost::lock_guard<boost::shared_mutex> lock(mutex);
+
       server = newServer;
 
       if(server)
@@ -109,100 +106,126 @@ class RequestManager {
     }
 
     void registerConnection(Connection *connection);
-    void unregisterConnection(Connection *connection);
+
+    void unregisterConnection(Connection *connection) {
+      boost::lock_guard<boost::shared_mutex> lock(mutex);
+      requestMaps.erase(connection);
+    }
 
     template <class T> void registerPacketType(const std::string &type) {
-      requestHandlerFactories.insert(std::make_pair(type, new SpecificRequestHandlerFactory<T>()));
+      boost::lock_guard<boost::shared_mutex> lock(mutex);
+      requestHandlerFactories.insert(std::make_pair(type, boost::shared_ptr<SpecificRequestHandlerFactory<T> >(new SpecificRequestHandlerFactory<T>())));
     }
 
-    void unregisterPacketType(const std::string &type);
+    void unregisterPacketType(const std::string &type) {
+      boost::lock_guard<boost::shared_mutex> lock(mutex);
+      requestHandlerFactories.erase(type);
+    }
 
     template <class T>
     bool sendRequest(Connection *connection, Request::slot_type slot) {
-      RequestMap *requestMap;
-      uint16_t requestId;
+      boost::unique_lock<boost::shared_mutex> lock(mutex);
 
-      requestMap = getUnusedRequestId(connection, &requestId);
+      boost::shared_ptr<RequestMap> requestMap;
+      boost::uint16_t requestId;
+
+      requestMap = _getUnusedRequestId(connection, &requestId);
 
       if(!requestMap)
         return false;
 
-      Request *request = new T(connection, requestId, slot);
-      request->sendRequest();
+      boost::shared_ptr<Request> request(new T(connection, requestId, slot));
       requestMap->addRequest(requestId, request);
+      lock.unlock();
+
+      request->sendRequest();
 
       return true;
     }
 
     template <class T, typename T1>
     bool sendRequest(Connection *connection, Request::slot_type slot, T1 t1) {
-      RequestMap *requestMap;
-      uint16_t requestId;
+      boost::unique_lock<boost::shared_mutex> lock(mutex);
 
-      requestMap = getUnusedRequestId(connection, &requestId);
+      boost::shared_ptr<RequestMap> requestMap;
+      boost::uint16_t requestId;
+
+      requestMap = _getUnusedRequestId(connection, &requestId);
 
       if(!requestMap)
         return false;
 
-      Request *request = new T(connection, requestId, slot, t1);
-      request->sendRequest();
+      boost::shared_ptr<Request> request(new T(connection, requestId, slot, t1));
       requestMap->addRequest(requestId, request);
+      lock.unlock();
+
+      request->sendRequest();
 
       return true;
     }
 
     template <class T, typename T1, typename T2>
     bool sendRequest(Connection *connection, Request::slot_type slot, T1 t1, T2 t2) {
-      RequestMap *requestMap;
-      uint16_t requestId;
+      boost::unique_lock<boost::shared_mutex> lock(mutex);
 
-      requestMap = getUnusedRequestId(connection, &requestId);
+      boost::shared_ptr<RequestMap> requestMap;
+      boost::uint16_t requestId;
+
+      requestMap = _getUnusedRequestId(connection, &requestId);
 
       if(!requestMap)
         return false;
 
-      Request *request = new T(connection, requestId, slot, t1, t2);
-      request->sendRequest();
+      boost::shared_ptr<Request> request(new T(connection, requestId, slot, t1, t2));
       requestMap->addRequest(requestId, request);
+      lock.unlock();
+
+      request->sendRequest();
 
       return true;
     }
 
     template <class T, typename T1, typename T2, typename T3>
     bool sendRequest(Connection *connection, Request::slot_type slot, T1 t1, T2 t2, T3 t3) {
-      RequestMap *requestMap;
-      uint16_t requestId;
+      boost::unique_lock<boost::shared_mutex> lock(mutex);
 
-      requestMap = getUnusedRequestId(connection, &requestId);
+      boost::shared_ptr<RequestMap> requestMap;
+      boost::uint16_t requestId;
+
+      requestMap = _getUnusedRequestId(connection, &requestId);
 
       if(!requestMap)
         return false;
 
-      Request *request = new T(connection, requestId, slot, t1, t2, t3);
-      request->sendRequest();
+      boost::shared_ptr<T> request(new T(connection, requestId, slot, t1, t2, t3));
       requestMap->addRequest(requestId, request);
+      lock.unlock();
+
+      request->sendRequest();
 
       return true;
     }
 
     template <class T, typename T1, typename T2, typename T3, typename T4>
     bool sendRequest(Connection *connection, Request::slot_type slot, T1 t1, T2 t2, T3 t3, T4 t4) {
-      RequestMap *requestMap;
-      uint16_t requestId;
+      boost::unique_lock<boost::shared_mutex> lock(mutex);
 
-      requestMap = getUnusedRequestId(connection, &requestId);
+      boost::shared_ptr<RequestMap> requestMap;
+      boost::uint16_t requestId;
+
+      requestMap = _getUnusedRequestId(connection, &requestId);
 
       if(!requestMap)
         return false;
 
-      Request *request = new T(connection, requestId, slot, t1, t2, t3, t4);
-      request->sendRequest();
+      boost::shared_ptr<Request> request(new T(connection, requestId, slot, t1, t2, t3, t4));
       requestMap->addRequest(requestId, request);
+      lock.unlock();
+
+      request->sendRequest();
 
       return true;
     }
-
-    virtual ~RequestManager();
 };
 
 }