revamped part of the port mapping code (UPnP and NAT-PMP). Added documentation for start_{lsd,natpmp,upnp} and stop_{lsd,natpmp,upnp}

docs/manual.rst

@@ -161,6 +161,15 @@ The ``session`` class has the following synopsis::
 			, int> const& node);
 		void add_dht_router(std::pair<std::string
 			, int> const& node);
+
+		void start_lsd();
+		void stop_lsd();
+
+		boost::intrusive_ptr<upnp> start_upnp();
+		void stop_upnp();
+
+		boost::intrusvice_ptr<natpmp> start_natpmp();
+		void stop_natpmp();
 	};
 
 Once it's created, the session object will spawn the main thread that will do all the work.
@@ -855,6 +864,47 @@ An example routing node that you could typically add is
 ``router.bittorrent.com``.
 
 
+start_lsd() stop_lsd()
+----------------------
+
+	::
+
+		void start_lsd();
+		void stop_lsd();
+
+Starts and stops Local Service Discovery. This service will broadcast
+the infohashes of all the non-private torrents on the local network to
+look for peers on the same swarm within multicast reach.
+
+It is turned off by default.
+
+start_upnp() stop_upnp()
+------------------------
+
+	::
+	
+		boost::intrusive_ptr<upnp> start_upnp();
+		void stop_upnp();
+
+Starts and stops the UPnP service. When started, the listen port and the DHT
+port are attempted to be forwarded on local UPnP router devices.
+
+It is off by default.
+
+start_natpmp() stop_natpmp()
+----------------------------
+
+	::
+		
+		boost::intrusvice_ptr<natpmp> start_natpmp();
+		void stop_natpmp();
+
+Starts and stops the NAT-PMP service. When started, the listen port and the DHT
+port are attempted to be forwarded on the router through NAT-PMP.
+
+It is off by default.
+
+
 entry
 =====
 

include/libtorrent/alert_types.hpp

@@ -426,20 +426,25 @@ namespace libtorrent
 
 	struct TORRENT_EXPORT portmap_error_alert: alert
 	{
-		portmap_error_alert(const std::string& msg)
+		portmap_error_alert(int i, const std::string& msg)
-			: alert(alert::warning, msg)
+			: mapping(i), alert(alert::warning, msg)
 		{}
 
+		int mapping;
+
 		virtual std::auto_ptr<alert> clone() const
 		{ return std::auto_ptr<alert>(new portmap_error_alert(*this)); }
 	};
 
 	struct TORRENT_EXPORT portmap_alert: alert
 	{
-		portmap_alert(const std::string& msg)
+		portmap_alert(int i, int port, const std::string& msg)
-			: alert(alert::info, msg)
+			: mapping(i), external_port(port), alert(alert::info, msg)
 		{}
 
+		int mapping;
+		int external_port;
+
 		virtual std::auto_ptr<alert> clone() const
 		{ return std::auto_ptr<alert>(new portmap_alert(*this)); }
 	};

include/libtorrent/aux_/session_impl.hpp

@@ -185,7 +185,8 @@ namespace libtorrent
 
 			// called when a port mapping is successful, or a router returns
 			// a failure to map a port
-			void on_port_mapping(int tcp_port, int udp_port, std::string const& errmsg);
+			void on_port_mapping(int mapping, int port, std::string const& errmsg
+				, int nat_transport);
 
 			bool is_aborted() const { return m_abort; }
 
@@ -319,8 +320,8 @@ namespace libtorrent
 			}
 #endif
 			void start_lsd();
-			void start_natpmp();
+			boost::intrusive_ptr<natpmp> start_natpmp();
-			void start_upnp();
+			boost::intrusive_ptr<upnp> start_upnp();
 
 			void stop_lsd();
 			void stop_natpmp();
@@ -537,6 +538,10 @@ namespace libtorrent
 			boost::intrusive_ptr<upnp> m_upnp;
 			boost::intrusive_ptr<lsd> m_lsd;
 
+			// 0 is natpmp 1 is upnp
+			int m_tcp_mapping[2];
+			int m_udp_mapping[2];
+
 			// the timer used to fire the second_tick
 			deadline_timer m_timer;
 

include/libtorrent/natpmp.hpp

@@ -37,6 +37,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include "libtorrent/intrusive_ptr_base.hpp"
 
 #include <boost/function.hpp>
+#include <boost/thread/mutex.hpp>
 
 #if defined(TORRENT_LOGGING) || defined(TORRENT_VERBOSE_LOGGING)
 #include <fstream>
@@ -45,8 +46,8 @@ POSSIBILITY OF SUCH DAMAGE.
 namespace libtorrent
 {
 
-// int: external tcp port
+// int: port mapping index
-// int: external udp port
+// int: external port
 // std::string: error message
 typedef boost::function<void(int, int, std::string const&)> portmap_callback_t;
 
@@ -59,36 +60,38 @@ public:
 
 	// maps the ports, if a port is set to 0
 	// it will not be mapped
-	void set_mappings(int tcp, int udp);
+	enum protocol_type { none = 0, udp = 1, tcp = 2 };
+	int add_mapping(protocol_type p, int external_port, int local_port);
+	void delete_mapping(int mapping_index);
 
 	void close();
 
 private:
 	
-	void update_mapping(int i, int port);
+	void update_mapping(int i);
 	void send_map_request(int i);
 	void resend_request(int i, asio::error_code const& e);
 	void on_reply(asio::error_code const& e
 		, std::size_t bytes_transferred);
 	void try_next_mapping(int i);
 	void update_expiration_timer();
-	void refresh_mapping(int i);
 	void mapping_expired(asio::error_code const& e, int i);
 
 	void disable(char const* message);
 
-	struct mapping
+	struct mapping_t
 	{
-		mapping()
+		enum action_t { action_none, action_add, action_delete };
-			: need_update(false)
+		mapping_t()
+			: action(action_none)
 			, local_port(0)
 			, external_port(0)
-			, protocol(1)
+			, protocol(none)
 		{}
 
 		// indicates that the mapping has changed
 		// and needs an update
-		bool need_update;
+		int action;
 
 		// the time the port mapping will expire
 		ptime expires;
@@ -102,14 +105,12 @@ private:
 		// should announce to others
 		int external_port;
 
-		// 1 = udp, 2 = tcp
 		int protocol;
 	};
 
 	portmap_callback_t m_callback;
 
-	// 0 is tcp and 1 is udp
+	std::vector<mapping_t> m_mappings;
-	mapping m_mappings[2];
 	
 	// the endpoint to the nat router
 	udp::endpoint m_nat_endpoint;
@@ -138,9 +139,17 @@ private:
 
 	// timer used to refresh mappings
 	deadline_timer m_refresh_timer;
+
+	// the mapping index that will expire next
+	int m_next_refresh;
 	
 	bool m_disabled;
 
+	bool m_abort;
+
+	typedef boost::mutex mutex_t;
+	mutex_t m_mutex;
+
 #if defined(TORRENT_LOGGING) || defined(TORRENT_VERBOSE_LOGGING)
 	std::ofstream m_log;
 #endif

include/libtorrent/session.hpp

@@ -284,8 +284,8 @@ namespace libtorrent
 		// starts/stops UPnP, NATPMP or LSD port mappers
 		// they are stopped by default
 		void start_lsd();
-		void start_natpmp();
+		boost::intrusive_ptr<natpmp> start_natpmp();
-		void start_upnp();
+		boost::intrusive_ptr<upnp> start_upnp();
 
 		void stop_lsd();
 		void stop_natpmp();

include/libtorrent/upnp.hpp

@@ -58,9 +58,10 @@ POSSIBILITY OF SUCH DAMAGE.
 namespace libtorrent
 {
 
-// int: external tcp port
+// int: port-mapping index
-// int: external udp port
+// int: external port
 // std::string: error message
+// an empty string as error means success
 typedef boost::function<void(int, int, std::string const&)> portmap_callback_t;
 
 class upnp : public intrusive_ptr_base<upnp>
@@ -71,29 +72,35 @@ public:
 		, portmap_callback_t const& cb, bool ignore_nonrouters);
 	~upnp();
 
-	// maps the ports, if a port is set to 0
+	enum protocol_type { none = 0, tcp = 1, udp = 2 };
-	// it will not be mapped
+	int add_mapping(protocol_type p, int external_port, int local_port);
-	void set_mappings(int tcp, int udp);
+	void delete_mapping(int index);
 
 	void discover_device();
 	void close();
 
-	std::string router_model() { return m_model; }
+	std::string router_model()
+	{
+		mutex_t::scoped_lock l(m_mutex);
+		return m_model;
+	}
 
 private:
 
+	void discover_device_impl();
 	static address_v4 upnp_multicast_address;
 	static udp::endpoint upnp_multicast_endpoint;
 
-	enum { num_mappings = 2 };
 	enum { default_lease_time = 3600 };
 	
-	void update_mapping(int i, int port);
 	void resend_request(asio::error_code const& e);
 	void on_reply(udp::endpoint const& from, char* buffer
 		, std::size_t bytes_transferred);
 
 	struct rootdevice;
+	void next(rootdevice& d, int i);
+	void update_map(rootdevice& d, int i);
+
 	
 	void on_upnp_xml(asio::error_code const& e
 		, libtorrent::http_parser const& p, rootdevice& d);
@@ -105,30 +112,43 @@ private:
 		, int mapping);
 	void on_expire(asio::error_code const& e);
 
-	void map_port(rootdevice& d, int i);
+	void disable(char const* msg);
-	void unmap_port(rootdevice& d, int i);
+	void return_error(int mapping, int code);
-	void disable();
-	void return_error(int code);
 
 	void delete_port_mapping(rootdevice& d, int i);
 	void create_port_mapping(http_connection& c, rootdevice& d, int i);
 	void post(upnp::rootdevice const& d, std::string const& soap
 		, std::string const& soap_action);
 
+	int num_mappings() const { return int(m_mappings.size()); }
+
+	struct global_mapping_t
+	{
+		global_mapping_t()
+			: protocol(none)
+			, external_port(0)
+			, local_port(0)
+		{}
+		int protocol;
+		int external_port;
+		int local_port;
+	};
+
 	struct mapping_t
 	{
+		enum action_t { action_none, action_add, action_delete };
 		mapping_t()
-			: need_update(false)
+			: action(action_none)
 			, local_port(0)
 			, external_port(0)
-			, protocol(1)
+			, protocol(none)
 			, failcount(0)
 		{}
 
 		// the time the port mapping will expire
 		ptime expires;
 		
-		bool need_update;
+		int action;
 
 		// the local port for this mapping. If this is set
 		// to 0, the mapping is not in use
@@ -139,7 +159,7 @@ private:
 		// should announce to others
 		int external_port;
 
-		// 1 = udp, 0 = tcp
+		// 2 = udp, 1 = tcp
 		int protocol;
 
 		// the number of times this mapping has failed
@@ -153,8 +173,6 @@ private:
 			, supports_specific_external(true)
 			, disabled(false)
 		{
-			mapping[0].protocol = 0;
-			mapping[1].protocol = 1;
 #ifndef NDEBUG
 			magic = 1337;
 #endif
@@ -167,7 +185,7 @@ private:
 			magic = 0;
 		}
 #endif
-		
+
 		// the interface url, through which the list of
 		// supported interfaces are fetched
 		std::string url;
@@ -177,7 +195,7 @@ private:
 		// either the WANIP namespace or the WANPPP namespace
 		char const* service_namespace;
 
-		mapping_t mapping[num_mappings];
+		std::vector<mapping_t> mapping;
 		
 		std::string hostname;
 		int port;
@@ -207,8 +225,7 @@ private:
 		{ return url < rhs.url; }
 	};
 	
-	int m_udp_local_port;
+	std::vector<global_mapping_t> m_mappings;
-	int m_tcp_local_port;
 
 	std::string const& m_user_agent;
 	
@@ -239,6 +256,9 @@ private:
 
 	connection_queue& m_cc;
 
+	typedef boost::mutex mutex_t;
+	mutex_t m_mutex;
+
 	std::string m_model;
 
 #ifdef TORRENT_UPNP_LOGGING

src/natpmp.cpp

@@ -43,8 +43,6 @@ POSSIBILITY OF SUCH DAMAGE.
 using boost::bind;
 using namespace libtorrent;
 
-enum { num_mappings = 2 };
-
 natpmp::natpmp(io_service& ios, address const& listen_interface, portmap_callback_t const& cb)
 	: m_callback(cb)
 	, m_currently_mapping(-1)
@@ -52,11 +50,9 @@ natpmp::natpmp(io_service& ios, address const& listen_interface, portmap_callbac
 	, m_socket(ios)
 	, m_send_timer(ios)
 	, m_refresh_timer(ios)
+	, m_next_refresh(-1)
 	, m_disabled(false)
 {
-	m_mappings[0].protocol = 2; // tcp
-	m_mappings[1].protocol = 1; // udp
-
 #if defined(TORRENT_LOGGING) || defined(TORRENT_VERBOSE_LOGGING)
 	m_log.open("natpmp.log", std::ios::in | std::ios::out | std::ios::trunc);
 #endif
@@ -65,6 +61,8 @@ natpmp::natpmp(io_service& ios, address const& listen_interface, portmap_callbac
 
 void natpmp::rebind(address const& listen_interface)
 {
+	mutex_t::scoped_lock l(m_mutex);
+
 	asio::error_code ec;
 	address gateway = get_default_gateway(m_socket.get_io_service(), listen_interface, ec);
 	if (ec)
@@ -101,42 +99,131 @@ void natpmp::rebind(address const& listen_interface)
 		return;
 	}
 
-	for (int i = 0; i < num_mappings; ++i)
+	for (std::vector<mapping_t>::iterator i = m_mappings.begin()
+		, end(m_mappings.end()); i != end; ++i)
 	{
-		if (m_mappings[i].local_port == 0)
+		if (i->protocol != none
+			|| i->action != mapping_t::action_none)
 			continue;
-		refresh_mapping(i);
+		i->action = mapping_t::action_add;
+		update_mapping(i - m_mappings.begin());
 	}
 }
 
 void natpmp::disable(char const* message)
 {
 	m_disabled = true;
-	std::stringstream msg;
+
-	msg << "NAT-PMP disabled: " << message;
+	for (std::vector<mapping_t>::iterator i = m_mappings.begin()
+		, end(m_mappings.end()); i != end; ++i)
+	{
+		if (i->protocol == none) continue;
+		i->protocol = none;
+		m_callback(i - m_mappings.begin(), 0, message);
+	}
+
 #if defined(TORRENT_LOGGING) || defined(TORRENT_VERBOSE_LOGGING)
-	m_log << msg.str() << std::endl;
+	m_log << time_now_string() << " NAT-PMP disabled: " << message << std::endl;
 #endif
-	m_callback(0, 0, msg.str());
+	close();
+}
+void natpmp::delete_mapping(int index)
+{
+	TORRENT_ASSERT(index < int(m_mappings.size()) && index >= 0);
+	if (index >= int(m_mappings.size()) || index < 0) return;
+	mapping_t& m = m_mappings[index];
+
+	if (m.protocol == none) return;
+
+	m.action = mapping_t::action_delete;
+	update_mapping(index);
 }
 
-void natpmp::set_mappings(int tcp, int udp)
+int natpmp::add_mapping(protocol_type p, int external_port, int local_port)
 {
-	if (m_disabled) return;
+	mutex_t::scoped_lock l(m_mutex);
-	update_mapping(0, tcp);
+
-	update_mapping(1, udp);
+	if (m_disabled) return -1;
+
+	std::vector<mapping_t>::iterator i = std::find_if(m_mappings.begin()
+		, m_mappings.end(), boost::bind(&mapping_t::protocol, _1) == int(none));
+	if (i == m_mappings.end())
+	{
+		m_mappings.push_back(mapping_t());
+		i = m_mappings.end() - 1;
+	}
+	i->protocol = p;
+	i->external_port = external_port;
+	i->local_port = local_port;
+	i->action = mapping_t::action_add;
+
+	int mapping_index = i - m_mappings.begin();
+
+	update_mapping(mapping_index);
+	return mapping_index;
 }
 
-void natpmp::update_mapping(int i, int port)
+void natpmp::try_next_mapping(int i)
 {
-	natpmp::mapping& m = m_mappings[i];
+#if defined(TORRENT_LOGGING) || defined(TORRENT_VERBOSE_LOGGING)
-	if (port <= 0) return;
+	m_log << time_now_string() << " try_next_mapping [ " << i << " ]" << std::endl;
-	if (m.local_port != port)
+#endif
-		m.need_update = true;
 
-	m.local_port = port;
+#if defined(TORRENT_LOGGING) || defined(TORRENT_VERBOSE_LOGGING)
-	// prefer the same external port as the local port
+	ptime now = time_now();
-	if (m.external_port == 0) m.external_port = port;
+	for (std::vector<mapping_t>::iterator i = m_mappings.begin()
+		, end(m_mappings.end()); i != end; ++i)
+	{
+		m_log << "     " << (i - m_mappings.begin()) << " [ "
+			"proto: " << (i->protocol == none ? "none" : i->protocol == tcp ? "tcp" : "udp")
+			<< " port: " << i->external_port
+			<< " local-port: " << i->local_port
+			<< " action: " << (i->action == mapping_t::action_none ? "none" : i->action == mapping_t::action_add ? "add" : "delete")
+			<< " ttl: " << total_seconds(i->expires - now)
+			<< " ]" << std::endl;
+	}
+#endif
+
+	if (i < int(m_mappings.size()) - 1)
+	{
+		update_mapping(i + 1);
+		return;
+	}
+
+	std::vector<mapping_t>::iterator m = std::find_if(
+		m_mappings.begin(), m_mappings.end()
+		, boost::bind(&mapping_t::action, _1) != int(mapping_t::action_none));
+
+	if (m == m_mappings.end())
+	{
+		if (m_abort)
+		{
+			asio::error_code ec;
+			m_send_timer.cancel(ec);
+			m_socket.close(ec);
+		}
+#if defined(TORRENT_LOGGING) || defined(TORRENT_VERBOSE_LOGGING)
+		m_log << "     done" << (m_abort?" shutting down":"") << std::endl;
+#endif
+		return;
+	}
+
+#if defined(TORRENT_LOGGING) || defined(TORRENT_VERBOSE_LOGGING)
+	m_log << "     updating " << (m - m_mappings.begin()) << std::endl;
+#endif
+
+	update_mapping(m - m_mappings.begin());
+}
+
+void natpmp::update_mapping(int i)
+{
+	natpmp::mapping_t& m = m_mappings[i];
+	if (m.action == mapping_t::action_none
+		|| m.protocol == none)
+	{
+		try_next_mapping(i);
+		return;
+	}
 
 	if (m_currently_mapping == -1)
 	{
@@ -156,7 +243,8 @@ void natpmp::send_map_request(int i)
 	TORRENT_ASSERT(m_currently_mapping == -1
 		|| m_currently_mapping == i);
 	m_currently_mapping = i;
-	mapping& m = m_mappings[i];
+	mapping_t& m = m_mappings[i];
+	TORRENT_ASSERT(m.action != mapping_t::action_none);
 	char buf[12];
 	char* out = buf;
 	write_uint8(0, out); // NAT-PMP version
@@ -164,14 +252,16 @@ void natpmp::send_map_request(int i)
 	write_uint16(0, out); // reserved
 	write_uint16(m.local_port, out); // private port
 	write_uint16(m.external_port, out); // requested public port
-	int ttl = m.external_port == 0 ? 0 : 3600;
+	int ttl = m.action == mapping_t::action_add ? 3600 : 0;
 	write_uint32(ttl, out); // port mapping lifetime
 
 #if defined(TORRENT_LOGGING) || defined(TORRENT_VERBOSE_LOGGING)
 	m_log << time_now_string()
-		<< " ==> port map request: " << (m.protocol == 1 ? "udp" : "tcp")
+		<< " ==> port map ["
+		<< " action: " << (m.action == mapping_t::action_add ? "add" : "delete") << " "
+		<< " proto: " << (m.protocol == udp ? "udp" : "tcp")
 		<< " local: " << m.local_port << " external: " << m.external_port
-		<< " ttl: " << ttl << std::endl;
+		<< " ttl: " << ttl << " ]" << std::endl;
 #endif
 
 	asio::error_code ec;
@@ -185,12 +275,16 @@ void natpmp::send_map_request(int i)
 void natpmp::resend_request(int i, asio::error_code const& e)
 {
 	if (e) return;
+
+	mutex_t::scoped_lock l(m_mutex);
 	if (m_currently_mapping != i) return;
 	if (m_retry_count >= 9)
 	{
-		m_mappings[i].need_update = false;
+		m_currently_mapping = -1;
+		m_mappings[i].action = mapping_t::action_none;
 		// try again in two hours
 		m_mappings[i].expires = time_now() + hours(2);
+		try_next_mapping(i);
 		return;
 	}
 	send_map_request(i);
@@ -209,12 +303,14 @@ void natpmp::on_reply(asio::error_code const& e
 		return;
 	}
 
+	mutex_t::scoped_lock l(m_mutex);
+
 	asio::error_code ec;
 	m_send_timer.cancel(ec);
 
 	TORRENT_ASSERT(m_currently_mapping >= 0);
 	int i = m_currently_mapping;
-	mapping& m = m_mappings[i];
+	mapping_t& m = m_mappings[i];
 
 	char* in = m_response_buffer;
 	int version = read_uint8(in);
@@ -229,9 +325,10 @@ void natpmp::on_reply(asio::error_code const& e
 
 #if defined(TORRENT_LOGGING) || defined(TORRENT_VERBOSE_LOGGING)
 	m_log << time_now_string()
-		<< " <== port map response: " << (cmd - 128 == 1 ? "udp" : "tcp")
+		<< " <== port map ["
+		<< " protocol: " << (cmd - 128 == 1 ? "udp" : "tcp")
 		<< " local: " << private_port << " external: " << public_port
-		<< " ttl: " << lifetime << std::endl;
+		<< " ttl: " << lifetime << " ]" << std::endl;
 #endif
 
 #if defined(TORRENT_LOGGING) || defined(TORRENT_VERBOSE_LOGGING)
@@ -259,7 +356,7 @@ void natpmp::on_reply(asio::error_code const& e
 	{
 		// this means the mapping was
 		// successfully closed
-		m.local_port = 0;
+		m.protocol = none;
 	}
 	else
 	{
@@ -282,21 +379,16 @@ void natpmp::on_reply(asio::error_code const& e
 			case 4: errmsg << "Out of resources"; break;
 			case 5: errmsg << "Unsupported opcode"; break;
 		}
-		m_mappings[i].expires = time_now() + hours(2);
+		m.expires = time_now() + hours(2);
-		m_callback(0, 0, errmsg.str());
+		m_callback(i, 0, errmsg.str());
 	}
-	else if (m.local_port != 0)
+	else if (m.action == mapping_t::action_add)
 	{
-		// don't report when we remove mappings
+		m_callback(i, m.external_port, "");
-		int tcp_port = 0;
-		int udp_port = 0;
-		if (m.protocol == 1) udp_port = m.external_port;
-		else tcp_port = public_port;
-		m_callback(tcp_port, udp_port, "");
 	}
 
 	m_currently_mapping = -1;
-	m_mappings[i].need_update = false;
+	m.action = mapping_t::action_none;
 	m_send_timer.cancel(ec);
 	update_expiration_timer();
 	try_next_mapping(i);
@@ -304,69 +396,95 @@ void natpmp::on_reply(asio::error_code const& e
 
 void natpmp::update_expiration_timer()
 {
+	if (m_abort) return;
+
 	ptime now = time_now();
+#if defined(TORRENT_LOGGING) || defined(TORRENT_VERBOSE_LOGGING)
+	m_log << time_now_string() << " update_expiration_timer " << std::endl;
+	for (std::vector<mapping_t>::iterator i = m_mappings.begin()
+		, end(m_mappings.end()); i != end; ++i)
+	{
+		m_log << "     " << (i - m_mappings.begin()) << " [ "
+			"proto: " << (i->protocol == none ? "none" : i->protocol == tcp ? "tcp" : "udp")
+			<< " port: " << i->external_port
+			<< " local-port: " << i->local_port
+			<< " action: " << (i->action == mapping_t::action_none ? "none" : i->action == mapping_t::action_add ? "add" : "delete")
+			<< " ttl: " << total_seconds(i->expires - now)
+			<< " ]" << std::endl;
+	}
+#endif
+
 	ptime min_expire = now + seconds(3600);
 	int min_index = -1;
-	for (int i = 0; i < num_mappings; ++i)
+	for (std::vector<mapping_t>::iterator i = m_mappings.begin()
-		if (m_mappings[i].expires < min_expire
+		, end(m_mappings.end()); i != end; ++i)
-			&& m_mappings[i].local_port != 0)
+	{
+		if (i->protocol == none
+			|| i->action != mapping_t::action_none) continue;
+		if (i->expires < min_expire)
 		{
-			min_expire = m_mappings[i].expires;
+			min_expire = i->expires;
-			min_index = i;
+			min_index = i - m_mappings.begin();
 		}
+	}
+
+	// this is already the mapping we're waiting for
+	if (m_next_refresh == min_index) return;
 
 	if (min_index >= 0)
 	{
+#if defined(TORRENT_LOGGING) || defined(TORRENT_VERBOSE_LOGGING)
+		m_log << time_now_string() << " next expiration ["
+			" i: " << min_index
+			<< " ttl: " << total_seconds(min_expire - time_now())
+			<< " ]" << std::endl;
+#endif
 		asio::error_code ec;
+		if (m_next_refresh >= 0) m_refresh_timer.cancel(ec);
 		m_refresh_timer.expires_from_now(min_expire - now, ec);
 		m_refresh_timer.async_wait(bind(&natpmp::mapping_expired, self(), _1, min_index));
+		m_next_refresh = min_index;
 	}
 }
 
 void natpmp::mapping_expired(asio::error_code const& e, int i)
 {
 	if (e) return;
+	mutex_t::scoped_lock l(m_mutex);
 #if defined(TORRENT_LOGGING) || defined(TORRENT_VERBOSE_LOGGING)
-	m_log << "*** mapping " << i << " expired, updating" << std::endl;
+	m_log << time_now_string() << " mapping expired [ i: " << i << " ]" << std::endl;
 #endif
-	refresh_mapping(i);
+	m_mappings[i].action = mapping_t::action_add;
-}
+	if (m_next_refresh == i) m_next_refresh = -1;
-
+	update_mapping(i);
-void natpmp::refresh_mapping(int i)
-{
-	m_mappings[i].need_update = true;
-	if (m_currently_mapping == -1)
-	{
-		// the socket is not currently in use
-		// send out a mapping request
-		m_retry_count = 0;
-		send_map_request(i);
-		m_socket.async_receive_from(asio::buffer(&m_response_buffer, 16)
-			, m_remote, bind(&natpmp::on_reply, self(), _1, _2));
-	}
-}
-
-void natpmp::try_next_mapping(int i)
-{
-	++i;
-	if (i >= num_mappings) i = 0;
-	if (m_mappings[i].need_update)
-		refresh_mapping(i);
 }
 
 void natpmp::close()
 {
+	mutex_t::scoped_lock l(m_mutex);
+	m_abort = true;
 	asio::error_code ec;
-	m_socket.close(ec);
+#if defined(TORRENT_LOGGING) || defined(TORRENT_VERBOSE_LOGGING)
+	m_log << time_now_string() << " close" << std::endl;
+#endif
 	if (m_disabled) return;
-	for (int i = 0; i < num_mappings; ++i)
+	ptime now = time_now();
+	for (std::vector<mapping_t>::iterator i = m_mappings.begin()
+		, end(m_mappings.end()); i != end; ++i)
 	{
-		if (m_mappings[i].local_port == 0)
+#if defined(TORRENT_LOGGING) || defined(TORRENT_VERBOSE_LOGGING)
-			continue;
+		m_log << "     " << (i - m_mappings.begin()) << " [ "
-		m_mappings[i].external_port = 0;
+			"proto: " << (i->protocol == none ? "none" : i->protocol == tcp ? "tcp" : "udp")
-		refresh_mapping(i);
+			<< " port: " << i->external_port
+			<< " local-port: " << i->local_port
+			<< " action: " << (i->action == mapping_t::action_none ? "none" : i->action == mapping_t::action_add ? "add" : "delete")
+			<< " ttl: " << total_seconds(i->expires - now)
+			<< " ]" << std::endl;
+#endif
+		if (i->protocol == none) continue;
+		i->action = mapping_t::action_delete;
 	}
 	m_refresh_timer.cancel(ec);
-	m_send_timer.cancel(ec);
+	update_mapping(0);
 }
 

src/session.cpp

@@ -479,14 +479,14 @@ namespace libtorrent
 		m_impl->start_lsd();
 	}
 	
-	void session::start_natpmp()
+	boost::intrusive_ptr<natpmp> session::start_natpmp()
 	{
-		m_impl->start_natpmp();
+		return m_impl->start_natpmp();
 	}
 	
-	void session::start_upnp()
+	boost::intrusive_ptr<upnp> session::start_upnp()
 	{
-		m_impl->start_upnp();
+		return m_impl->start_upnp();
 	}
 	
 	void session::stop_lsd()

src/session_impl.cpp

@@ -174,6 +174,10 @@ namespace aux {
 		, m_geoip_db(0)
 #endif
 	{
+		m_tcp_mapping[0] = -1;
+		m_tcp_mapping[1] = -1;
+		m_udp_mapping[0] = -1;
+		m_udp_mapping[1] = -1;
 #ifdef WIN32
 		// windows XP has a limit on the number of
 		// simultaneous half-open TCP connections
@@ -668,8 +672,18 @@ namespace aux {
 			tcp::endpoint local = m_listen_sockets.front().sock->local_endpoint(ec);
 			if (!ec)
 			{
-				if (m_natpmp.get()) m_natpmp->set_mappings(local.port(), 0);
+				if (m_natpmp.get())
-				if (m_upnp.get()) m_upnp->set_mappings(local.port(), 0);
+				{
+					if (m_tcp_mapping[0] != -1) m_natpmp->delete_mapping(m_tcp_mapping[0]);
+					m_tcp_mapping[0] = m_natpmp->add_mapping(natpmp::tcp
+						, local.port(), local.port());
+				}
+				if (m_upnp.get())
+				{
+					if (m_tcp_mapping[1] != -1) m_upnp->delete_mapping(m_tcp_mapping[1]);
+					m_tcp_mapping[1] = m_upnp->add_mapping(upnp::tcp
+						, local.port(), local.port());
+				}
 			}
 		}
 	}
@@ -1667,9 +1681,19 @@ namespace aux {
 			// the listen interface changed, rebind the dht listen socket as well
 			m_dht_socket.bind(m_dht_settings.service_port);
 			if (m_natpmp.get())
-				m_natpmp->set_mappings(0, m_dht_settings.service_port);
+			{
+				if (m_udp_mapping[0] != -1) m_natpmp->delete_mapping(m_udp_mapping[0]);
+				m_udp_mapping[0] = m_natpmp->add_mapping(natpmp::tcp
+					, m_dht_settings.service_port
+					, m_dht_settings.service_port);
+			}
 			if (m_upnp.get())
-				m_upnp->set_mappings(0, m_dht_settings.service_port);
+			{
+				if (m_udp_mapping[1] != -1) m_upnp->delete_mapping(m_udp_mapping[1]);
+				m_udp_mapping[1] = m_upnp->add_mapping(upnp::tcp
+					, m_dht_settings.service_port
+					, m_dht_settings.service_port);
+			}
 		}
 #endif
 
@@ -1714,43 +1738,40 @@ namespace aux {
 		t->get_policy().peer_from_tracker(peer, peer_id(0), peer_info::lsd, 0);
 	}
 
-	void session_impl::on_port_mapping(int tcp_port, int udp_port
+	void session_impl::on_port_mapping(int mapping, int port
-		, std::string const& errmsg)
+		, std::string const& errmsg, int map_transport)
 	{
 #ifndef TORRENT_DISABLE_DHT
-		if (udp_port != 0)
+		if (mapping == m_udp_mapping[map_transport] && port != 0)
 		{
-			m_external_udp_port = udp_port;
+			m_external_udp_port = port;
-			m_dht_settings.service_port = udp_port;
+			m_dht_settings.service_port = port;
 			if (m_alerts.should_post(alert::info))
-			{
+				m_alerts.post_alert(portmap_alert(mapping, port
-				std::stringstream msg;
+					, "successfully mapped UDP port"));
-				msg << "successfully mapped UDP port " << udp_port;
+			return;
-				m_alerts.post_alert(portmap_alert(msg.str()));
-			}
 		}
 #endif
 
-		if (tcp_port != 0)
+		if (mapping == m_tcp_mapping[map_transport] && port != 0)
 		{
 			if (!m_listen_sockets.empty())
-				m_listen_sockets.front().external_port = tcp_port;
+				m_listen_sockets.front().external_port = port;
 			if (m_alerts.should_post(alert::info))
-			{
+				m_alerts.post_alert(portmap_alert(mapping, port
-				std::stringstream msg;
+					, "successfully mapped TCP port"));
-				msg << "successfully mapped TCP port " << tcp_port;
+			return;
-				m_alerts.post_alert(portmap_alert(msg.str()));
-			}
 		}
 
 		if (!errmsg.empty())
 		{
 			if (m_alerts.should_post(alert::warning))
-			{
+				m_alerts.post_alert(portmap_error_alert(mapping, errmsg));
-				std::stringstream msg;
+		}
-				msg << "Error while mapping ports on NAT router: " << errmsg;
+		else
-				m_alerts.post_alert(portmap_error_alert(msg.str()));
+		{
-			}
+			if (m_alerts.should_post(alert::warning))
+				m_alerts.post_alert(portmap_alert(mapping, port, "successfully mapped port"));
 		}
 	}
 
@@ -1831,10 +1852,18 @@ namespace aux {
 			m_dht_settings.service_port = m_listen_interface.port();
 		}
 		m_external_udp_port = m_dht_settings.service_port;
-		if (m_natpmp.get())
+		if (m_natpmp.get() && m_udp_mapping[0] == -1)
-			m_natpmp->set_mappings(0, m_dht_settings.service_port);
+		{
-		if (m_upnp.get())
+			m_udp_mapping[0] = m_natpmp->add_mapping(natpmp::udp
-			m_upnp->set_mappings(0, m_dht_settings.service_port);
+				, m_dht_settings.service_port
+				, m_dht_settings.service_port);
+		}
+		if (m_upnp.get() && m_udp_mapping[1] == -1)
+		{
+			m_udp_mapping[1] = m_upnp->add_mapping(upnp::udp
+				, m_dht_settings.service_port
+				, m_dht_settings.service_port);
+		}
 		m_dht = new dht::dht_tracker(m_dht_socket, m_dht_settings, startup_state);
 		if (!m_dht_socket.is_open() || m_dht_socket.local_port() != m_dht_settings.service_port)
 		{
@@ -1867,9 +1896,19 @@ namespace aux {
 			m_dht_socket.bind(settings.service_port);
 
 			if (m_natpmp.get())
-				m_natpmp->set_mappings(0, m_dht_settings.service_port);
+			{
+				if (m_udp_mapping[0] != -1) m_upnp->delete_mapping(m_udp_mapping[0]);
+				m_udp_mapping[0] = m_natpmp->add_mapping(natpmp::udp
+					, m_dht_settings.service_port
+					, m_dht_settings.service_port);
+			}
 			if (m_upnp.get())
-				m_upnp->set_mappings(0, m_dht_settings.service_port);
+			{
+				if (m_udp_mapping[1] != -1) m_upnp->delete_mapping(m_udp_mapping[1]);
+				m_udp_mapping[1] = m_upnp->add_mapping(upnp::udp
+					, m_dht_settings.service_port
+					, m_dht_settings.service_port);
+			}
 			m_external_udp_port = settings.service_port;
 		}
 		m_dht_settings = settings;
@@ -2053,47 +2092,55 @@ namespace aux {
 			, bind(&session_impl::on_lsd_peer, this, _1, _2));
 	}
 	
-	void session_impl::start_natpmp()
+	boost::intrusive_ptr<natpmp> session_impl::start_natpmp()
 	{
 		mutex_t::scoped_lock l(m_mutex);
 
 		INVARIANT_CHECK;
 
-		if (m_natpmp) return;
+		if (m_natpmp) return m_natpmp;
 
 		m_natpmp = new natpmp(m_io_service
 			, m_listen_interface.address()
 			, bind(&session_impl::on_port_mapping
-				, this, _1, _2, _3));
+				, this, _1, _2, _3, 0));
 
-		m_natpmp->set_mappings(m_listen_interface.port(),
+		m_tcp_mapping[0] = m_natpmp->add_mapping(natpmp::tcp
+			, m_listen_interface.port(), m_listen_interface.port());
 #ifndef TORRENT_DISABLE_DHT
-			m_dht ? m_dht_settings.service_port : 
+		if (m_dht)
+			m_udp_mapping[0] = m_natpmp->add_mapping(natpmp::udp
+				, m_dht_settings.service_port 
+				, m_dht_settings.service_port);
 #endif
-			0);
+		return m_natpmp;
 	}
 
-	void session_impl::start_upnp()
+	boost::intrusive_ptr<upnp> session_impl::start_upnp()
 	{
 		mutex_t::scoped_lock l(m_mutex);
 
 		INVARIANT_CHECK;
 
-		if (m_upnp) return;
+		if (m_upnp) return m_upnp;
 
 		m_upnp = new upnp(m_io_service, m_half_open
 			, m_listen_interface.address()
 			, m_settings.user_agent
 			, bind(&session_impl::on_port_mapping
-				, this, _1, _2, _3)
+				, this, _1, _2, _3, 1)
 			, m_settings.upnp_ignore_nonrouters);
 
 		m_upnp->discover_device();
-		m_upnp->set_mappings(m_listen_interface.port(), 
+		m_tcp_mapping[1] = m_upnp->add_mapping(upnp::tcp
+			, m_listen_interface.port(), m_listen_interface.port());
 #ifndef TORRENT_DISABLE_DHT
-			m_dht ? m_dht_settings.service_port : 
+		if (m_dht)
+			m_udp_mapping[1] = m_upnp->add_mapping(upnp::udp
+				, m_dht_settings.service_port 
+				, m_dht_settings.service_port);
 #endif
-			0);
+		return m_upnp;
 	}
 
 	void session_impl::stop_lsd()
@@ -2116,7 +2163,11 @@ namespace aux {
 	{
 		mutex_t::scoped_lock l(m_mutex);
 		if (m_upnp.get())
+		{
 			m_upnp->close();
+			m_udp_mapping[1] = -1;
+			m_tcp_mapping[1] = -1;
+		}
 		m_upnp = 0;
 	}
 	

src/upnp.cpp

@@ -63,9 +63,7 @@ namespace libtorrent
 upnp::upnp(io_service& ios, connection_queue& cc
 	, address const& listen_interface, std::string const& user_agent
 	, portmap_callback_t const& cb, bool ignore_nonrouters)
-	: m_udp_local_port(0)
+	:  m_user_agent(user_agent)
-	, m_tcp_local_port(0)
-	, m_user_agent(user_agent)
 	, m_callback(cb)
 	, m_retry_count(0)
 	, m_io_service(ios)
@@ -89,6 +87,13 @@ upnp::~upnp()
 }
 
 void upnp::discover_device()
+{
+	mutex_t::scoped_lock l(m_mutex);
+
+	discover_device_impl();
+}
+
+void upnp::discover_device_impl()
 {
 	const char msearch[] = 
 		"M-SEARCH * HTTP/1.1\r\n"
@@ -112,7 +117,7 @@ void upnp::discover_device()
 			<< " ==> Broadcast FAILED: " << ec.message() << std::endl
 			<< "aborting" << std::endl;
 #endif
-		disable();
+		disable(ec.message().c_str());
 		return;
 	}
 
@@ -127,51 +132,99 @@ void upnp::discover_device()
 #endif
 }
 
-void upnp::set_mappings(int tcp, int udp)
+// returns a reference to a mapping or -1 on failure
+int upnp::add_mapping(upnp::protocol_type p, int external_port, int local_port)
 {
+	mutex_t::scoped_lock l(m_mutex);
+
 #ifdef TORRENT_UPNP_LOGGING
 	m_log << time_now_string()
-		<< " *** set mappings " << tcp << " " << udp;
+		<< " *** add mapping [ proto: " << (p == tcp?"tcp":"udp")
+		<< " ext_port: " << external_port
+		<< " local_port :" << local_port << " ]";
 	if (m_disabled) m_log << " DISABLED";
 	m_log << std::endl;
 #endif
+	if (m_disabled) return -1;
 
-	if (m_disabled) return;
+	std::vector<global_mapping_t>::iterator i = std::find_if(
-	if (udp != 0) m_udp_local_port = udp;
+		m_mappings.begin(), m_mappings.end()
-	if (tcp != 0) m_tcp_local_port = tcp;
+		, boost::bind(&global_mapping_t::protocol, _1) == int(none));
+
+	if (i == m_mappings.end())
+	{
+		m_mappings.push_back(global_mapping_t());
+		i = m_mappings.end() - 1;
+	}
+
+	i->protocol = p;
+	i->external_port = external_port;
+	i->local_port = local_port;
+
+	int mapping_index = i - m_mappings.begin();
 
 	for (std::set<rootdevice>::iterator i = m_devices.begin()
 		, end(m_devices.end()); i != end; ++i)
 	{
 		rootdevice& d = const_cast<rootdevice&>(*i);
 		TORRENT_ASSERT(d.magic == 1337);
-		if (d.mapping[0].local_port != m_tcp_local_port)
+
-		{
+		if (int(d.mapping.size()) <= mapping_index)
-			if (d.mapping[0].external_port == 0)
+			d.mapping.resize(mapping_index + 1);
-				d.mapping[0].external_port = m_tcp_local_port;
+		mapping_t& m = d.mapping[mapping_index];
-			d.mapping[0].local_port = m_tcp_local_port;
+
-			d.mapping[0].need_update = true;
+		m.action = mapping_t::action_add;
-		}
+		m.protocol = p;
-		if (d.mapping[1].local_port != m_udp_local_port)
+		m.external_port = external_port;
-		{
+		m.local_port = local_port;
-			if (d.mapping[1].external_port == 0)
+
-				d.mapping[1].external_port = m_udp_local_port;
+		if (d.service_namespace) update_map(d, mapping_index);
-			d.mapping[1].local_port = m_udp_local_port;
+	}
-			d.mapping[1].need_update = true;
+
-		}
+	return mapping_index;
-		if (d.service_namespace
+}
-			&& (d.mapping[0].need_update || d.mapping[1].need_update))
+
-			map_port(d, 0);
+void upnp::delete_mapping(int mapping)
+{
+	mutex_t::scoped_lock l(m_mutex);
+
+	if (mapping <= int(m_mappings.size())) return;
+
+	global_mapping_t& m = m_mappings[mapping];
+
+#ifdef TORRENT_UPNP_LOGGING
+	m_log << time_now_string()
+		<< " *** delete mapping [ proto: " << (m.protocol == tcp?"tcp":"udp")
+		<< " ext_port:" << m.external_port
+		<< " local_port:" << m.local_port << " ]";
+	m_log << std::endl;
+#endif
+
+	if (m.protocol == none) return;
+	
+	for (std::set<rootdevice>::iterator i = m_devices.begin()
+		, end(m_devices.end()); i != end; ++i)
+	{
+		rootdevice& d = const_cast<rootdevice&>(*i);
+		TORRENT_ASSERT(d.magic == 1337);
+
+		TORRENT_ASSERT(mapping < int(d.mapping.size()));
+		d.mapping[mapping].action = mapping_t::action_delete;
+
+		if (d.service_namespace) update_map(d, mapping);
 	}
 }
 
 void upnp::resend_request(asio::error_code const& e)
 {
 	if (e) return;
+
+	mutex_t::scoped_lock l(m_mutex);
+
 	if (m_retry_count < 9
 		&& (m_devices.empty() || m_retry_count < 4))
 	{
-		discover_device();
+		discover_device_impl();
 		return;
 	}
 
@@ -182,7 +235,7 @@ void upnp::resend_request(asio::error_code const& e)
 			<< " *** Got no response in 9 retries. Giving up, "
 			"disabling UPnP." << std::endl;
 #endif
-		disable();
+		disable("no UPnP router found");
 		return;
 	}
 	
@@ -223,6 +276,8 @@ void upnp::resend_request(asio::error_code const& e)
 void upnp::on_reply(udp::endpoint const& from, char* buffer
 	, std::size_t bytes_transferred)
 {
+	mutex_t::scoped_lock l(m_mutex);
+
 	using namespace libtorrent::detail;
 
 	// parse out the url for the device
@@ -379,25 +434,16 @@ void upnp::on_reply(udp::endpoint const& from, char* buffer
 			return;
 		}
 
-		if (m_tcp_local_port != 0)
+		TORRENT_ASSERT(d.mapping.empty());
+		for (std::vector<global_mapping_t>::iterator j = m_mappings.begin()
+			, end(m_mappings.end()); j != end; ++j)
 		{
-			d.mapping[0].need_update = true;
+			mapping_t m;
-			d.mapping[0].local_port = m_tcp_local_port;
+			m.action = mapping_t::action_add;
-			if (d.mapping[0].external_port == 0)
+			m.local_port = j->local_port;
-				d.mapping[0].external_port = d.mapping[0].local_port;
+			m.external_port = j->external_port;
-#ifdef TORRENT_UPNP_LOGGING
+			m.protocol = j->protocol;
-			m_log << time_now_string() << " *** Mapping 0 will be updated" << std::endl;
+			d.mapping.push_back(m);
-#endif
-		}
-		if (m_udp_local_port != 0)
-		{
-			d.mapping[1].need_update = true;
-			d.mapping[1].local_port = m_udp_local_port;
-			if (d.mapping[1].external_port == 0)
-				d.mapping[1].external_port = d.mapping[1].local_port;
-#ifdef TORRENT_UPNP_LOGGING
-			m_log << time_now_string() << " *** Mapping 1 will be updated" << std::endl;
-#endif
 		}
 		boost::tie(i, boost::tuples::ignore) = m_devices.insert(d);
 	}
@@ -474,6 +520,8 @@ void upnp::post(upnp::rootdevice const& d, std::string const& soap
 
 void upnp::create_port_mapping(http_connection& c, rootdevice& d, int i)
 {
+	mutex_t::scoped_lock l(m_mutex);
+
 	TORRENT_ASSERT(d.magic == 1337);
 
 	if (!d.upnp_connection)
@@ -509,29 +557,45 @@ void upnp::create_port_mapping(http_connection& c, rootdevice& d, int i)
 	post(d, soap.str(), soap_action);
 }
 
-void upnp::map_port(rootdevice& d, int i)
+void upnp::next(rootdevice& d, int i)
+{
+	if (i < num_mappings() - 1)
+	{
+		update_map(d, i + 1);
+	}
+	else
+	{
+		std::vector<mapping_t>::iterator i
+			= std::find_if(d.mapping.begin(), d.mapping.end()
+			, boost::bind(&mapping_t::action, _1) != int(mapping_t::action_none));
+		if (i == d.mapping.end()) return;
+
+		update_map(d, i - d.mapping.begin());
+	}
+}
+
+void upnp::update_map(rootdevice& d, int i)
 {
 	TORRENT_ASSERT(d.magic == 1337);
+	TORRENT_ASSERT(i < int(d.mapping.size()));
+	TORRENT_ASSERT(d.mapping.size() == m_mappings.size());
+
 	if (d.upnp_connection) return;
 
-	if (d.mapping[i].failcount > 5)
+	mapping_t& m = d.mapping[i];
-	{
+
-		// giving up
+	if (m.action == mapping_t::action_none
-		if (i < num_mappings - 1)
+		|| m.protocol == none)
-			map_port(d, i + 1);
-		return;
-	}
-	if (!d.mapping[i].need_update)
 	{
 #ifdef TORRENT_UPNP_LOGGING
-		m_log << time_now_string() << " *** mapping (" << i
+		if (m.protocol != none)
-			<< ") does not need update, skipping" << std::endl;
+			m_log << time_now_string() << " *** mapping (" << i
+				<< ") does not need update, skipping" << std::endl;
 #endif
-		if (i < num_mappings - 1)
+		next(d, i);
-			map_port(d, i + 1);
 		return;
 	}
-	d.mapping[i].need_update = false;
+
 	TORRENT_ASSERT(!d.upnp_connection);
 	TORRENT_ASSERT(d.service_namespace);
 
@@ -539,17 +603,40 @@ void upnp::map_port(rootdevice& d, int i)
 		m_log << time_now_string()
 			<< " ==> connecting to " << d.hostname << std::endl;
 #endif
-	d.upnp_connection.reset(new http_connection(m_io_service
+	if (m.action == mapping_t::action_add)
-		, m_cc, bind(&upnp::on_upnp_map_response, self(), _1, _2
+	{
-		, boost::ref(d), i), true
+		if (m.failcount > 5)
-		, bind(&upnp::create_port_mapping, self(), _1, boost::ref(d), i)));
+		{
+			// giving up
+			next(d, i);
+			return;
+		}
 
-	d.upnp_connection->start(d.hostname, boost::lexical_cast<std::string>(d.port)
+		d.upnp_connection.reset(new http_connection(m_io_service
-		, seconds(10), 1);
+			, m_cc, bind(&upnp::on_upnp_map_response, self(), _1, _2
+			, boost::ref(d), i), true
+			, bind(&upnp::create_port_mapping, self(), _1, boost::ref(d), i)));
+
+		d.upnp_connection->start(d.hostname, boost::lexical_cast<std::string>(d.port)
+			, seconds(10), 1);
+	}
+	else if (m.action == mapping_t::action_delete)
+	{
+		d.upnp_connection.reset(new http_connection(m_io_service
+			, m_cc, bind(&upnp::on_upnp_unmap_response, self(), _1, _2
+			, boost::ref(d), i), true
+			, bind(&upnp::delete_port_mapping, self(), boost::ref(d), i)));
+		d.upnp_connection->start(d.hostname, boost::lexical_cast<std::string>(d.port)
+			, seconds(10), 1);
+	}
+
+	m.action = mapping_t::action_none;
 }
 
 void upnp::delete_port_mapping(rootdevice& d, int i)
 {
+	mutex_t::scoped_lock l(m_mutex);
+
 	TORRENT_ASSERT(d.magic == 1337);
 
 	if (!d.upnp_connection)
@@ -579,31 +666,6 @@ void upnp::delete_port_mapping(rootdevice& d, int i)
 	post(d, soap.str(), soap_action);
 }
 
-// requires the mutex to be locked
-void upnp::unmap_port(rootdevice& d, int i)
-{
-	TORRENT_ASSERT(d.magic == 1337);
-	if (d.mapping[i].external_port == 0
-		|| d.disabled)
-	{
-		if (i < num_mappings - 1)
-		{
-			unmap_port(d, i + 1);
-		}
-		return;
-	}
-#ifdef TORRENT_UPNP_LOGGING
-		m_log << time_now_string()
-			<< " ==> connecting to " << d.hostname << std::endl;
-#endif
-	d.upnp_connection.reset(new http_connection(m_io_service
-		, m_cc, bind(&upnp::on_upnp_unmap_response, self(), _1, _2
-		, boost::ref(d), i), true
-		, bind(&upnp::delete_port_mapping, self(), boost::ref(d), i)));
-	d.upnp_connection->start(d.hostname, boost::lexical_cast<std::string>(d.port)
-		, seconds(10), 1);
-}
-
 namespace
 {
 	struct parse_state
@@ -673,6 +735,8 @@ namespace
 void upnp::on_upnp_xml(asio::error_code const& e
 	, libtorrent::http_parser const& p, rootdevice& d)
 {
+	mutex_t::scoped_lock l(m_mutex);
+
 	TORRENT_ASSERT(d.magic == 1337);
 	if (d.upnp_connection)
 	{
@@ -752,12 +816,22 @@ void upnp::on_upnp_xml(asio::error_code const& e
 
 	d.control_url = s.control_url;
 
-	map_port(d, 0);
+	update_map(d, 0);
 }
 
-void upnp::disable()
+void upnp::disable(char const* msg)
 {
 	m_disabled = true;
+
+	// kill all mappings
+	for (std::vector<global_mapping_t>::iterator i = m_mappings.begin()
+		, end(m_mappings.end()); i != end; ++i)
+	{
+		if (i->protocol == none) continue;
+		i->protocol = none;
+		m_callback(i - m_mappings.begin(), 0, msg);
+	}
+	
 	m_devices.clear();
 	asio::error_code ec;
 	m_broadcast_timer.cancel(ec);
@@ -820,6 +894,8 @@ namespace
 void upnp::on_upnp_map_response(asio::error_code const& e
 	, libtorrent::http_parser const& p, rootdevice& d, int mapping)
 {
+	mutex_t::scoped_lock l(m_mutex);
+
 	TORRENT_ASSERT(d.magic == 1337);
 	if (d.upnp_connection)
 	{
@@ -862,7 +938,7 @@ void upnp::on_upnp_map_response(asio::error_code const& e
 		m_log << time_now_string()
 			<< " <== error while adding portmap: incomplete http message" << std::endl;
 #endif
-		d.disabled = true;
+		next(d, mapping);
 		return;
 	}
 
@@ -881,42 +957,44 @@ void upnp::on_upnp_map_response(asio::error_code const& e
 	}
 #endif
 	
+	mapping_t& m = d.mapping[mapping];
+
 	if (s.error_code == 725)
 	{
 		// only permanent leases supported
 		d.lease_duration = 0;
-		d.mapping[mapping].need_update = true;
+		m.action = mapping_t::action_add;
-		++d.mapping[mapping].failcount;
+		++m.failcount;
-		map_port(d, mapping);
+		update_map(d, mapping);
 		return;
 	}
 	else if (s.error_code == 718 || s.error_code == 727)
 	{
-		if (d.mapping[mapping].external_port != 0)
+		if (m.external_port != 0)
 		{
 			// conflict in mapping, set port to wildcard
 			// and let the router decide
-			d.mapping[mapping].external_port = 0;
+			m.external_port = 0;
-			d.mapping[mapping].need_update = true;
+			m.action = mapping_t::action_add;
-			++d.mapping[mapping].failcount;
+			++m.failcount;
-			map_port(d, mapping);
+			update_map(d, mapping);
 			return;
 		}
-		return_error(s.error_code);
+		return_error(mapping, s.error_code);
 	}
 	else if (s.error_code == 716)
 	{
 		// The external port cannot be wildcarder
 		// pick a random port
-		d.mapping[mapping].external_port = 40000 + (rand() % 10000);
+		m.external_port = 40000 + (rand() % 10000);
-		d.mapping[mapping].need_update = true;
+		m.action = mapping_t::action_add;
-		++d.mapping[mapping].failcount;
+		++m.failcount;
-		map_port(d, mapping);
+		update_map(d, mapping);
 		return;
 	}
 	else if (s.error_code != -1)
 	{
-		return_error(s.error_code);
+		return_error(mapping, s.error_code);
 	}
 
 #ifdef TORRENT_UPNP_LOGGING
@@ -927,46 +1005,31 @@ void upnp::on_upnp_map_response(asio::error_code const& e
 
 	if (s.error_code == -1)
 	{
-		int tcp = 0;
+		m_callback(mapping, m.external_port, "");
-		int udp = 0;
-		
-		if (mapping == 0)
-			tcp = d.mapping[mapping].external_port;
-		else
-			udp = d.mapping[mapping].external_port;
-
-		m_callback(tcp, udp, "");
 		if (d.lease_duration > 0)
 		{
-			d.mapping[mapping].expires = time_now()
+			m.expires = time_now()
 				+ seconds(int(d.lease_duration * 0.75f));
 			ptime next_expire = m_refresh_timer.expires_at();
 			if (next_expire < time_now()
-				|| next_expire > d.mapping[mapping].expires)
+				|| next_expire > m.expires)
 			{
 				asio::error_code ec;
-				m_refresh_timer.expires_at(d.mapping[mapping].expires, ec);
+				m_refresh_timer.expires_at(m.expires, ec);
 				m_refresh_timer.async_wait(bind(&upnp::on_expire, self(), _1));
 			}
 		}
 		else
 		{
-			d.mapping[mapping].expires = max_time();
+			m.expires = max_time();
 		}
-		d.mapping[mapping].failcount = 0;
+		m.failcount = 0;
 	}
 
-	for (int i = 0; i < num_mappings; ++i)
+	next(d, mapping);
-	{
-		if (d.mapping[i].need_update)
-		{
-			map_port(d, i);
-			return;
-		}
-	}
 }
 
-void upnp::return_error(int code)
+void upnp::return_error(int mapping, int code)
 {
 	int num_errors = sizeof(error_codes) / sizeof(error_codes[0]);
 	error_code_t* end = error_codes + num_errors;
@@ -980,12 +1043,14 @@ void upnp::return_error(int code)
 		error_string += ": ";
 		error_string += e->msg;
 	}
-	m_callback(0, 0, error_string);
+	m_callback(mapping, 0, error_string);
 }
 
 void upnp::on_upnp_unmap_response(asio::error_code const& e
 	, libtorrent::http_parser const& p, rootdevice& d, int mapping)
 {
+	mutex_t::scoped_lock l(m_mutex);
+
 	TORRENT_ASSERT(d.magic == 1337);
 	if (d.upnp_connection)
 	{
@@ -999,39 +1064,33 @@ void upnp::on_upnp_unmap_response(asio::error_code const& e
 		m_log << time_now_string()
 			<< " <== error while deleting portmap: " << e.message() << std::endl;
 #endif
-	}
+	} else if (!p.header_finished())
-
-	if (!p.header_finished())
 	{
 #ifdef TORRENT_UPNP_LOGGING
 		m_log << time_now_string()
 			<< " <== error while deleting portmap: incomplete http message" << std::endl;
 #endif
-		return;
 	}
-
+	else if (p.status_code() != 200)
-	if (p.status_code() != 200)
 	{
 #ifdef TORRENT_UPNP_LOGGING
 		m_log << time_now_string()
 			<< " <== error while deleting portmap: " << p.message() << std::endl;
 #endif
-		d.disabled = true;
-		return;
 	}
+	else
+	{
 
 #ifdef TORRENT_UPNP_LOGGING
-	m_log << time_now_string()
+		m_log << time_now_string()
-		<< " <== unmap response: " << std::string(p.get_body().begin, p.get_body().end)
+			<< " <== unmap response: " << std::string(p.get_body().begin, p.get_body().end)
-		<< std::endl;
+			<< std::endl;
 #endif
-
-	// ignore errors and continue with the next mapping for this device
-	if (mapping < num_mappings - 1)
-	{
-		unmap_port(d, mapping + 1);
-		return;
 	}
+
+	d.mapping[mapping].protocol = none;
+
+	next(d, mapping);
 }
 
 void upnp::on_expire(asio::error_code const& e)
@@ -1041,12 +1100,14 @@ void upnp::on_expire(asio::error_code const& e)
 	ptime now = time_now();
 	ptime next_expire = max_time();
 
+	mutex_t::scoped_lock l(m_mutex);
+
 	for (std::set<rootdevice>::iterator i = m_devices.begin()
 		, end(m_devices.end()); i != end; ++i)
 	{
 		rootdevice& d = const_cast<rootdevice&>(*i);
 		TORRENT_ASSERT(d.magic == 1337);
-		for (int m = 0; m < num_mappings; ++m)
+		for (int m = 0; m < num_mappings(); ++m)
 		{
 			if (d.mapping[m].expires != max_time())
 				continue;
@@ -1054,7 +1115,7 @@ void upnp::on_expire(asio::error_code const& e)
 			if (d.mapping[m].expires < now)
 			{
 				d.mapping[m].expires = max_time();
-				map_port(d, m);
+				update_map(d, m);
 			}
 			else if (d.mapping[m].expires < next_expire)
 			{
@@ -1072,25 +1133,33 @@ void upnp::on_expire(asio::error_code const& e)
 
 void upnp::close()
 {
+	mutex_t::scoped_lock l(m_mutex);
+
 	asio::error_code ec;
 	m_refresh_timer.cancel(ec);
 	m_broadcast_timer.cancel(ec);
 	m_closing = true;
 	m_socket.close();
 
-	if (m_disabled)
-	{
-		m_devices.clear();
-		return;
-	}
-
 	for (std::set<rootdevice>::iterator i = m_devices.begin()
 		, end(m_devices.end()); i != end; ++i)
 	{
 		rootdevice& d = const_cast<rootdevice&>(*i);
 		TORRENT_ASSERT(d.magic == 1337);
 		if (d.control_url.empty()) continue;
-		unmap_port(d, 0);
+		for (std::vector<mapping_t>::iterator j = d.mapping.begin()
+			, end(d.mapping.end()); j != end; ++j)
+		{
+			if (j->protocol == none) continue;
+			if (j->action == mapping_t::action_add)
+			{
+				j->action = mapping_t::action_none;
+				continue;
+			}
+			j->action = mapping_t::action_delete;
+			m_mappings[j - d.mapping.begin()].protocol = none;
+		}
+		update_map(d, 0);
 	}
 }
 

test/Jamfile

@@ -3,6 +3,9 @@ use-project /torrent : .. ;
 exe test_upnp : test_upnp.cpp /torrent//torrent
 	: <link>static <threading>multi <logging>verbose <upnp-logging>on ;
 
+exe test_natpmp : test_natpmp.cpp /torrent//torrent
+	: <link>static <threading>multi <logging>verbose <upnp-logging>on ;
+
 lib test_common
 	:
 	main.cpp

test/test_natpmp.cpp

@@ -0,0 +1,56 @@
+#include "libtorrent/natpmp.hpp"
+#include "libtorrent/socket.hpp"
+#include "libtorrent/connection_queue.hpp"
+#include <boost/bind.hpp>
+#include <boost/ref.hpp>
+#include <boost/intrusive_ptr.hpp>
+
+using namespace libtorrent;
+
+void callback(int mapping, int port, std::string const& err)
+{
+	std::cerr << "mapping: " << mapping << ", port: " << port << ", error: \"" << err << "\"\n";
+}
+
+int main(int argc, char* argv[])
+{
+	io_service ios;
+	std::string user_agent = "test agent";
+
+	if (argc != 3)
+	{
+		std::cerr << "usage: " << argv[0] << " tcp-port udp-port" << std::endl;
+		return 1;
+	}
+
+	connection_queue cc(ios);
+	boost::intrusive_ptr<natpmp> natpmp_handler = new natpmp(ios, address_v4(), &callback);
+
+	deadline_timer timer(ios);
+
+	int tcp_map = natpmp_handler->add_mapping(natpmp::tcp, atoi(argv[1]), atoi(argv[1]));
+	int udp_map = natpmp_handler->add_mapping(natpmp::udp, atoi(argv[2]), atoi(argv[2]));
+
+	timer.expires_from_now(seconds(2));
+	timer.async_wait(boost::bind(&io_service::stop, boost::ref(ios)));
+	std::cerr << "mapping ports TCP: " << argv[1]
+		<< " UDP: " << argv[2] << std::endl;
+
+	ios.reset();
+	ios.run();
+	timer.expires_from_now(seconds(2));
+	timer.async_wait(boost::bind(&io_service::stop, boost::ref(ios)));
+	std::cerr << "removing mapping " << tcp_map << std::endl;
+	natpmp_handler->delete_mapping(tcp_map);
+
+	ios.reset();
+	ios.run();
+	std::cerr << "removing mappings" << std::endl;
+	natpmp_handler->close();
+
+	ios.reset();
+	ios.run();
+	std::cerr << "closing" << std::endl;
+}
+
+

test/test_upnp.cpp

@@ -7,9 +7,9 @@
 
 using namespace libtorrent;
 
-void callback(int tcp, int udp, std::string const& err)
+void callback(int mapping, int port, std::string const& err)
 {
-	std::cerr << "tcp: " << tcp << ", udp: " << udp << ", error: \"" << err << "\"\n";
+	std::cerr << "mapping: " << mapping << ", port: " << port << ", error: \"" << err << "\"\n";
 }
 
 int main(int argc, char* argv[])
@@ -36,8 +36,9 @@ int main(int argc, char* argv[])
 	ios.reset();
 	ios.run();
 
-	upnp_handler->set_mappings(atoi(argv[1]), atoi(argv[2]));
+	upnp_handler->add_mapping(upnp::tcp, atoi(argv[1]), atoi(argv[1]));
-	timer.expires_from_now(seconds(5));
+	upnp_handler->add_mapping(upnp::udp, atoi(argv[2]), atoi(argv[2]));
+	timer.expires_from_now(seconds(10));
 	timer.async_wait(boost::bind(&io_service::stop, boost::ref(ios)));
 	std::cerr << "mapping ports TCP: " << argv[1]
 		<< " UDP: " << argv[2] << std::endl;