correct termination of UPnP

UPnP.cpp

@@ -21,49 +21,54 @@ namespace i2p
 {
 namespace transport
 {
-    UPnP::UPnP () : m_Thread (nullptr)
+    UPnP::UPnP () : m_IsRunning(false), m_Thread (nullptr), m_Timer (m_Service)
     {
     }
 
     void UPnP::Stop ()
     {
-        LogPrint(eLogInfo, "UPnP: stopping");
+		if (m_IsRunning)
-        if (m_Thread)
+		{
-        {   
+		    LogPrint(eLogInfo, "UPnP: stopping");
-            m_Thread->join (); 
+			m_IsRunning = false;	
-            delete m_Thread;
+			m_Timer.cancel ();
-            m_Thread = nullptr;
+			m_Service.stop ();
-        }
+		    if (m_Thread)
+		    {   
+		        m_Thread->join (); 
+		        m_Thread.reset (nullptr);
+		    }
+			CloseMapping ();
+		    Close ();
+		}
     }
 
     void UPnP::Start()
     {
+		m_IsRunning = true;
         LogPrint(eLogInfo, "UPnP: starting");
-        m_Thread = new std::thread (std::bind (&UPnP::Run, this));
+		m_Service.post (std::bind (&UPnP::Discover, this));
+        m_Thread.reset (new std::thread (std::bind (&UPnP::Run, this)));
     }
     
     UPnP::~UPnP ()
     {
+		Stop ();
     } 
 
     void UPnP::Run ()
     {
-        auto a = context.GetRouterInfo().GetAddresses();
+		while (m_IsRunning)
-        for (auto address : a)
+		{
-        {
+			try
-            if (!address->host.is_v6 ())
+			{	
-            {
+				m_Service.run ();
-                Discover ();
+			}
-                if (address->transportStyle == data::RouterInfo::eTransportSSU )
+			catch (std::exception& ex)
-                {
+			{
-                    TryPortMapping (I2P_UPNP_UDP, address->port);
+				LogPrint (eLogError, "UPnP: runtime exception: ", ex.what ());
-                }
+			}	
-                else if (address->transportStyle == data::RouterInfo::eTransportNTCP )
+		}	
-                {
-                    TryPortMapping (I2P_UPNP_TCP, address->port);
-                }
-            }
-        }
     } 
         
     void UPnP::Discover ()
@@ -87,73 +92,74 @@ namespace transport
             }
             else
             {
-                if (m_externalIPAddress[0])
+                if (!m_externalIPAddress[0])
-                {
-                    LogPrint (eLogDebug, "UPnP: ExternalIPAddress is ", m_externalIPAddress);
-                    i2p::context.UpdateAddress (boost::asio::ip::address::from_string (m_externalIPAddress));
-                    return;
-                }
-                else
                 {
                     LogPrint (eLogError, "UPnP: GetExternalIPAddress() failed.");
                     return;
                 }
             }
         }
+		else
+		{
+			 LogPrint (eLogError, "UPnP: GetValidIGD() failed.");
+             return;
+		}
+
+		// UPnP discovered	
+		LogPrint (eLogDebug, "UPnP: ExternalIPAddress is ", m_externalIPAddress);
+        i2p::context.UpdateAddress (boost::asio::ip::address::from_string (m_externalIPAddress));
+		// port mapping
+		PortMapping ();
     }
 
-    void UPnP::TryPortMapping (int type, int port)
+	void UPnP::PortMapping ()
-    {
+	{
-        std::string strType, strPort (std::to_string (port));
+		auto a = context.GetRouterInfo().GetAddresses();
-        switch (type)
+        for (auto address : a)
         {
-            case I2P_UPNP_TCP:
+            if (!address->host.is_v6 ())
-                strType = "TCP";
+            	TryPortMapping (address);
-                break;
+        }
-            case I2P_UPNP_UDP:
+		m_Timer.expires_from_now (boost::posix_time::minutes(20));	// every 20 minutes
-            default:
+		m_Timer.async_wait ([this](const boost::system::error_code& ecode)
-                strType = "UDP";
+			{ 
+				if (ecode != boost::asio::error::operation_aborted)	
+					PortMapping ();
+			});
+
+	}	
+
+	void UPnP::CloseMapping ()
+	{
+		auto a = context.GetRouterInfo().GetAddresses();
+        for (auto address : a)
+        {
+            if (!address->host.is_v6 ())
+            	CloseMapping (address);
         }
+	}	
+
+    void UPnP::TryPortMapping (std::shared_ptr<i2p::data::RouterInfo::Address> address)
+    {
+        std::string strType (GetProto (address)), strPort (std::to_string (address->port));
         int r;
         std::string strDesc = "I2Pd";
-        try {
+        r = UPNP_AddPortMapping (m_upnpUrls.controlURL, m_upnpData.first.servicetype, strPort.c_str (), strPort.c_str (), m_NetworkAddr, strDesc.c_str (), strType.c_str (), 0, "0");
-            for (;;) {
+        if (r!=UPNPCOMMAND_SUCCESS)
-                r = UPNP_AddPortMapping (m_upnpUrls.controlURL, m_upnpData.first.servicetype, strPort.c_str (), strPort.c_str (), m_NetworkAddr, strDesc.c_str (), strType.c_str (), 0, "0");
+        {
-                if (r!=UPNPCOMMAND_SUCCESS)
+            LogPrint (eLogError, "UPnP: AddPortMapping (", m_NetworkAddr, ":", strPort, ") failed with code ", r);
-                {
+            return;
-                    LogPrint (eLogError, "UPnP: AddPortMapping (", m_NetworkAddr, ":", strPort, ") failed with code ", r);
-                    return;
-                }
-                else
-                {
-                    LogPrint (eLogDebug, "UPnP: Port Mapping successful. (", m_NetworkAddr ,":", strPort, " type ", strType, " -> ", m_externalIPAddress ,":", strPort ,")");
-                    return;
-                }
-                std::this_thread::sleep_for(std::chrono::minutes(20)); // c++11
-                //boost::this_thread::sleep_for(); // pre c++11
-                //sleep(20*60); // non-portable
-            }
         }
-        catch (boost::thread_interrupted)
+        else
         {
-            CloseMapping(type, port);
+            LogPrint (eLogDebug, "UPnP: Port Mapping successful. (", m_NetworkAddr ,":", strPort, " type ", strType, " -> ", m_externalIPAddress ,":", strPort ,")");
-            Close();
+            return;
-            throw;
         }
     }
 
-    void UPnP::CloseMapping (int type, int port)
+    void UPnP::CloseMapping (std::shared_ptr<i2p::data::RouterInfo::Address> address)
     {
-        std::string strType, strPort (std::to_string (port));
+        std::string strType (GetProto (address)), strPort (std::to_string (address->port));
-        switch (type)
-        {
-            case I2P_UPNP_TCP:
-                strType = "TCP";
-                break;
-            case I2P_UPNP_UDP:
-            default:
-                strType = "UDP";
-        }
         int r = 0;
         r = UPNP_DeletePortMapping (m_upnpUrls.controlURL, m_upnpData.first.servicetype, strPort.c_str (), strType.c_str (), 0);
         LogPrint (eLogError, "UPnP: DeletePortMapping() returned : ", r);
@@ -165,6 +171,19 @@ namespace transport
         m_Devlist = 0;
         FreeUPNPUrls (&m_upnpUrls);
     }
+
+	std::string UPnP::GetProto (std::shared_ptr<i2p::data::RouterInfo::Address> address)
+	{
+		switch (address->transportStyle)
+        {
+            case i2p::data::RouterInfo::eTransportNTCP:
+                return "TCP";
+            break;
+            case i2p::data::RouterInfo::eTransportSSU:
+            default:
+                return "UDP";
+        }
+	}
 }
 }
 #else /* USE_UPNP */

UPnP.h

@@ -4,6 +4,7 @@
 #ifdef USE_UPNP
 #include <string>
 #include <thread>
+#include <memory>
 
 #include <miniupnpc/miniwget.h>
 #include <miniupnpc/miniupnpc.h>
@@ -14,9 +15,6 @@
 
 #include "util.h"
 
-#define I2P_UPNP_TCP 1
-#define I2P_UPNP_UDP 2
-
 namespace i2p
 {
 namespace transport
@@ -32,13 +30,23 @@ namespace transport
         void Start ();
         void Stop ();
 
-		void Discover ();
-		void TryPortMapping (int type, int port);
-		void CloseMapping (int type, int port);
 	private:
+
+		void Discover ();
+		void PortMapping ();
+		void TryPortMapping (std::shared_ptr<i2p::data::RouterInfo::Address> address);
+		void CloseMapping ();
+		void CloseMapping (std::shared_ptr<i2p::data::RouterInfo::Address> address);
+
 		void Run ();
+		std::string GetProto (std::shared_ptr<i2p::data::RouterInfo::Address> address);
 
-        std::thread * m_Thread;
+	private:
+	
+		bool m_IsRunning;
+        std::unique_ptr<std::thread> m_Thread;
+		boost::asio::io_service m_Service;
+		boost::asio::deadline_timer m_Timer;
         struct UPNPUrls m_upnpUrls;
         struct IGDdatas m_upnpData;