From c5e150ee69452dc119e31ab8d3474c37cdb34fc9 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Mon, 14 May 2007 07:31:01 +0000 Subject: [PATCH] upnp fixes --- Jamfile | 3 ++ include/libtorrent/upnp.hpp | 13 ++++- src/upnp.cpp | 97 +++++++++++++++++++++++++++++-------- 3 files changed, 90 insertions(+), 23 deletions(-) diff --git a/Jamfile b/Jamfile index 900502555..05163e142 100755 --- a/Jamfile +++ b/Jamfile @@ -40,6 +40,9 @@ feature.compose unicode : _UNICODE UNICODE ; feature statistics : off on : composite propagated symmetric link-incompatible ; feature.compose on : TORRENT_STATS ; +feature upnp-logging : off on : composite propagated link-incompatible ; +feature.compose on : TORRENT_UPNP_LOGGING ; + SOURCES = allocate_resources alert diff --git a/include/libtorrent/upnp.hpp b/include/libtorrent/upnp.hpp index 3a48dc720..aac11931f 100644 --- a/include/libtorrent/upnp.hpp +++ b/include/libtorrent/upnp.hpp @@ -102,6 +102,7 @@ private: , std::string const& soap_action); void map_port(rootdevice& d, int i); void unmap_port(rootdevice& d, int i); + void disable(); struct mapping_t { @@ -135,6 +136,7 @@ private: rootdevice(): service_namespace(0) , lease_duration(default_lease_time) , supports_specific_external(true) + , disabled(false) { mapping[0].protocol = 0; mapping[1].protocol = 1; @@ -159,10 +161,17 @@ private: // true if the device supports specifying a // specific external port, false if it doesn't bool supports_specific_external; + + bool disabled; - boost::shared_ptr upnp_connection; + mutable boost::shared_ptr upnp_connection; - void close() const { if (upnp_connection) upnp_connection->close(); } + void close() const + { + if (!upnp_connection) return; + upnp_connection->close(); + upnp_connection.reset(); + } bool operator<(rootdevice const& rhs) const { return url < rhs.url; } diff --git a/src/upnp.cpp b/src/upnp.cpp index 92813c3ee..45e0b65bf 100644 --- a/src/upnp.cpp +++ b/src/upnp.cpp @@ -144,7 +144,7 @@ void upnp::rebind(address const& listen_interface) try } catch (std::exception& e) { - m_disabled = true; + disable(); std::stringstream msg; msg << "UPnP portmapping disabled: " << e.what(); m_callback(0, 0, msg.str()); @@ -163,8 +163,20 @@ void upnp::discover_device() try m_socket.async_receive_from(asio::buffer(m_receive_buffer , sizeof(m_receive_buffer)), m_remote, m_strand.wrap(bind( &upnp::on_reply, this, _1, _2))); + + asio::error_code ec; +#ifdef TORRENT_DEBUG_UPNP + // simulate packet loss + if (m_retry_count & 1) +#endif m_socket.send_to(asio::buffer(msearch, sizeof(msearch) - 1) - , upnp_multicast_endpoint); + , upnp_multicast_endpoint, 0, ec); + + if (ec) + { + disable(); + return; + } ++m_retry_count; m_broadcast_timer.expires_from_now(milliseconds(250 * m_retry_count)); @@ -178,7 +190,7 @@ void upnp::discover_device() try } catch (std::exception&) { - m_disabled = true; + disable(); } void upnp::set_mappings(int tcp, int udp) @@ -211,7 +223,10 @@ void upnp::set_mappings(int tcp, int udp) } } -void upnp::resend_request(asio::error_code const& e) try +void upnp::resend_request(asio::error_code const& e) +#ifndef NDEBUG +try +#endif { if (e) return; if (m_retry_count < 9 @@ -228,32 +243,49 @@ void upnp::resend_request(asio::error_code const& e) try << " *** Got no response in 9 retries. Giving up, " "disabling UPnP." << std::endl; #endif - m_disabled = true; + disable(); return; } for (std::set::iterator i = m_devices.begin() , end(m_devices.end()); i != end; ++i) { - if (i->control_url.empty() && !i->upnp_connection) + if (i->control_url.empty() && !i->upnp_connection && !i->disabled) { // we don't have a WANIP or WANPPP url for this device, // ask for it rootdevice& d = const_cast(*i); - d.upnp_connection.reset(new http_connection(m_socket.io_service() - , m_cc, m_strand.wrap(bind(&upnp::on_upnp_xml, this, _1, _2 - , boost::ref(d))))); - d.upnp_connection->get(d.url); + try + { + d.upnp_connection.reset(new http_connection(m_socket.io_service() + , m_cc, m_strand.wrap(bind(&upnp::on_upnp_xml, this, _1, _2 + , boost::ref(d))))); + d.upnp_connection->get(d.url); + } + catch (std::exception& e) + { +#ifdef TORRENT_UPNP_LOGGING + m_log << time_now_string() + << " *** Connection failed to: " << d.url + << " " << e.what() << std::endl; +#endif + d.disabled = true; + } } } } +#ifndef NDEBUG catch (std::exception&) { - m_disabled = true; + assert(false); } +#endif void upnp::on_reply(asio::error_code const& e - , std::size_t bytes_transferred) try + , std::size_t bytes_transferred) +#ifndef NDEBUG +try +#endif { using namespace libtorrent::detail; if (e) return; @@ -379,23 +411,37 @@ void upnp::on_reply(asio::error_code const& e for (std::set::iterator i = m_devices.begin() , end(m_devices.end()); i != end; ++i) { - if (i->control_url.empty() && !i->upnp_connection) + if (i->control_url.empty() && !i->upnp_connection && !i->disabled) { // we don't have a WANIP or WANPPP url for this device, // ask for it rootdevice& d = const_cast(*i); - d.upnp_connection.reset(new http_connection(m_socket.io_service() - , m_cc, m_strand.wrap(bind(&upnp::on_upnp_xml, this, _1, _2 - , boost::ref(d))))); - d.upnp_connection->get(d.url); + try + { + d.upnp_connection.reset(new http_connection(m_socket.io_service() + , m_cc, m_strand.wrap(bind(&upnp::on_upnp_xml, this, _1, _2 + , boost::ref(d))))); + d.upnp_connection->get(d.url); + } + catch (std::exception& e) + { +#ifdef TORRENT_UPNP_LOGGING + m_log << time_now_string() + << " *** Connection failed to: " << d.url + << " " << e.what() << std::endl; +#endif + d.disabled = true; + } } } } } +#ifndef NDEBUG catch (std::exception&) { - m_disabled = true; + assert(false); } +#endif void upnp::post(rootdevice& d, std::stringstream const& soap , std::string const& soap_action) @@ -626,8 +672,17 @@ void upnp::on_upnp_xml(asio::error_code const& e map_port(d, 0); } catch (std::exception&) +{ + disable(); +} + +void upnp::disable() { m_disabled = true; + m_devices.clear(); + m_broadcast_timer.cancel(); + m_refresh_timer.cancel(); + m_socket.close(); } namespace @@ -821,7 +876,7 @@ void upnp::on_upnp_map_response(asio::error_code const& e } catch (std::exception&) { - m_disabled = true; + disable(); } void upnp::on_upnp_unmap_response(asio::error_code const& e @@ -869,7 +924,7 @@ void upnp::on_upnp_unmap_response(asio::error_code const& e } catch (std::exception&) { - m_disabled = true; + disable(); } void upnp::on_expire(asio::error_code const& e) try @@ -907,7 +962,7 @@ void upnp::on_expire(asio::error_code const& e) try } catch (std::exception&) { - m_disabled = true; + disable(); } void upnp::close()