From 801d3637b925488ab9a0f59cb871801bf909645d Mon Sep 17 00:00:00 2001
From: Arvid Norberg <arvid@libtorrent.org>
Date: Sat, 14 May 2011 22:25:49 +0000
Subject: [PATCH] attempt to fix crash in udp_socket when using a broken socks5
 proxy

---
 include/libtorrent/connection_queue.hpp |  6 ++++
 src/connection_queue.cpp                |  2 ++
 src/udp_socket.cpp                      | 43 +++++++++++++++++++++++++
 3 files changed, 51 insertions(+)

diff --git a/include/libtorrent/connection_queue.hpp b/include/libtorrent/connection_queue.hpp
index 7192e59f8..e17e7a841 100644
--- a/include/libtorrent/connection_queue.hpp
+++ b/include/libtorrent/connection_queue.hpp
@@ -84,8 +84,14 @@ private:
 	{
 		entry(): connecting(false), ticket(0), expires(max_time()), priority(0) {}
 		// called when the connection is initiated
+		// this is when the timeout countdown starts
 		boost::function<void(int)> on_connect;
 		// called if done hasn't been called within the timeout
+		// or if the connection queue aborts. This means there
+		// are 3 different interleaves of these function calls:
+		// 1. on_connect
+		// 2. on_connect, on_timeout
+		// 3. on_timeout
 		boost::function<void()> on_timeout;
 		bool connecting;
 		int ticket;
diff --git a/src/connection_queue.cpp b/src/connection_queue.cpp
index 06048e7a6..26c37bb34 100644
--- a/src/connection_queue.cpp
+++ b/src/connection_queue.cpp
@@ -292,6 +292,8 @@ namespace libtorrent
 		for (std::list<entry>::iterator i = timed_out.begin()
 			, end(timed_out.end()); i != end; ++i)
 		{
+			TORRENT_ASSERT(i->connecting);
+			TORRENT_ASSERT(i->ticket != -1);
 			TORRENT_TRY {
 				i->on_timeout();
 			} TORRENT_CATCH(std::exception&) {}
diff --git a/src/udp_socket.cpp b/src/udp_socket.cpp
index de4ee85b7..557e3de29 100644
--- a/src/udp_socket.cpp
+++ b/src/udp_socket.cpp
@@ -552,6 +552,17 @@ void udp_socket::close()
 	{
 		m_cc.done(m_connection_ticket);
 		m_connection_ticket = -1;
+
+		// we just called done, which means on_timeout
+		// won't be called. Decrement the outstanding
+		// ops counter for that
+		TORRENT_ASSERT(m_outstanding_ops > 0);
+		--m_outstanding_ops;
+		if (m_abort)
+		{
+			maybe_clear_callback();
+			return;
+		}
 	}
 
 	maybe_clear_callback();
@@ -741,6 +752,12 @@ void udp_socket::on_name_lookup(error_code const& e, tcp::resolver::iterator i)
 	m_proxy_addr.address(i->endpoint().address());
 	m_proxy_addr.port(i->endpoint().port());
 	// on_connect may be called from within this thread
+	// the semantics for on_connect and on_timeout is 
+	// a bit complicated. See comments in connection_queue.hpp
+	// for more details. This semantic determines how and
+	// when m_outstanding_ops may be decremented
+	// To simplyfy this, it's probably a good idea to
+	// merge on_connect and on_timeout to a single function
 	++m_outstanding_ops;
 	m_cc.enqueue(boost::bind(&udp_socket::on_connect, this, _1)
 		, boost::bind(&udp_socket::on_timeout, this), seconds(10));
@@ -748,6 +765,13 @@ void udp_socket::on_name_lookup(error_code const& e, tcp::resolver::iterator i)
 
 void udp_socket::on_timeout()
 {
+	TORRENT_ASSERT(m_outstanding_ops > 0);
+	--m_outstanding_ops;
+	if (m_abort)
+	{
+		maybe_clear_callback();
+		return;
+	}
 	CHECK_MAGIC;
 	TORRENT_ASSERT(is_single_thread());
 
@@ -775,6 +799,13 @@ void udp_socket::on_connect(int ticket)
 	add_outstanding_async("udp_socket::on_connected");
 #endif
 	m_connection_ticket = ticket;
+	// at this point on_timeout may be called before on_connected
+	// so increment the outstanding ops
+	// it may also not be called in case we call
+	// connection_queue::done first, so be sure to
+	// decrement if that happens
+	++m_outstanding_ops;
+
 	error_code ec;
 	m_socks5_sock.open(m_proxy_addr.address().is_v4()?tcp::v4():tcp::v6(), ec);
 	++m_outstanding_ops;
@@ -802,6 +833,18 @@ void udp_socket::on_connected(error_code const& e)
 	TORRENT_ASSERT(is_single_thread());
 	m_cc.done(m_connection_ticket);
 	m_connection_ticket = -1;
+
+	// we just called done, which means on_timeout
+	// won't be called. Decrement the outstanding
+	// ops counter for that
+	TORRENT_ASSERT(m_outstanding_ops > 0);
+	--m_outstanding_ops;
+	if (m_abort)
+	{
+		maybe_clear_callback();
+		return;
+	}
+
 	if (e)
 	{
 		TORRENT_TRY {