From 94628fa78d7651bdb01be799c4e1f4e742c97307 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Sun, 28 May 2006 19:03:54 +0000 Subject: [PATCH] fixed race condition in peer_connection, fixed assert in torrent destructor, updated tests --- Jamfile | 6 +- Makefile.am | 16 +- docs/manual.html | 248 ++++++++++++------------- docs/manual.rst | 6 +- examples/client_test.cpp | 20 +- include/Makefile.am | 2 +- include/libtorrent/peer_connection.hpp | 3 +- include/libtorrent/policy.hpp | 5 + include/libtorrent/torrent.hpp | 9 +- src/http_tracker_connection.cpp | 2 +- src/peer_connection.cpp | 29 ++- src/policy.cpp | 2 +- src/session.cpp | 226 +++++++++++++--------- src/torrent.cpp | 70 +++++-- src/torrent_handle.cpp | 34 +++- test/Jamfile | 1 + test/Makefile.am | 4 +- test/setup_transfer.cpp | 65 +++++++ test/setup_transfer.hpp | 14 ++ test/test_metadata_extension.cpp | 81 ++------ 20 files changed, 521 insertions(+), 322 deletions(-) create mode 100644 test/setup_transfer.cpp create mode 100644 test/setup_transfer.hpp diff --git a/Jamfile b/Jamfile index c0b29690e..b00ccf882 100755 --- a/Jamfile +++ b/Jamfile @@ -30,9 +30,9 @@ project torrent BOOST_ALL_NO_LIB _FILE_OFFSET_BITS=64 BOOST_THREAD_USE_LIB - /boost/thread//boost_thread/static - /boost/filesystem//boost_filesystem/static - /boost/date_time//boost_date_time/static + /boost/thread//boost_thread #/static + /boost/filesystem//boost_filesystem #/static + /boost/date_time//boost_date_time #/static multi msvc:_WIN32_WINNT=0x0500 # WIN32 makes sure the win32 socket api is used diff --git a/Makefile.am b/Makefile.am index 0c28de75b..6c65160fc 100644 --- a/Makefile.am +++ b/Makefile.am @@ -4,7 +4,21 @@ docs/extension_protocol.html docs/udp_tracker_protocol.rst \ docs/udp_tracker_protocol.html docs/client_test.rst docs/client_test.html \ docs/unicode_support.png docs/client_test.png docs/style.css Jamfile project-root.jam \ m4/ac_cxx_namespaces.m4 m4/acx_pthread.m4 m4/ax_boost_date-time.m4 \ -m4/ax_boost_filesystem.m4 m4/ax_boost_thread.m4 src/file_win.cpp libtorrent.pc \ +m4/ax_boost_filesystem.m4 m4/ax_boost_thread.m4 src/file_win.cpp libtorrent.pc + +pkginclude_HEADER = \ +debian/changelog \ +debian/compat \ +debian/control \ +debian/copyright \ +debian/files \ +debian/libtorrent0-dev.dirs \ +debian/libtorrent0-dev.docs \ +debian/libtorrent0-dev.install \ +debian/libtorrent0.dirs \ +debian/libtorrent0.docs \ +debian/libtorrent0.install \ +debian/rules \ asio/aclocal.m4 \ asio/autogen.sh \ asio/boostify.pl \ diff --git a/docs/manual.html b/docs/manual.html index 038c07fae..62cf5a517 100755 --- a/docs/manual.html +++ b/docs/manual.html @@ -142,148 +142,148 @@ div.warning, div.note, div.important {

Table of contents

@@ -356,7 +356,7 @@ epoll on linux and kqueue on MacOS X and BSD.

  • Windows 2000 vc7.1
  • Linux x86 GCC 3.3, GCC 3.4.2
    • -
  • MacOS X (darwin), (Apple's) GCC 4.0
    • +
  • MacOS X (darwin), (Apple's) GCC 3.3, (Apple's) GCC 4.0
  • SunOS 5.8 GCC 3.1
  • Cygwin GCC 3.3.3
@@ -366,7 +366,6 @@ epoll on linux and kqueue on MacOS X and BSD.

  • GCC 2.95.4
  • msvc6
    • -
  • (Apple's) GCC 3.3 (compiler crashes with the latest version of asio)

libtorrent is released under the BSD-license.

@@ -514,6 +513,7 @@ For more build configuration flags see link=static otherwise you may get unresolved external symbols for some boost.program-options symbols.

+

For more information, see the Boost build v2 documentation.

@@ -793,7 +793,7 @@ class session: public boost::noncopyable

Once it's created, the session object will spawn the main thread that will do all the work. The main thread will be idle as long it doesn't have any torrents to participate in.

-

session()

+

session()

 session(fingerprint const& print = libtorrent::fingerprint("LT", 0, 1, 0, 0));
@@ -812,7 +812,7 @@ will automatically try to listen on a port on the given interface. For more info
 the parameters, see listen_on() function.
-

~session()

+

~session()

The destructor of session will notify all trackers that our torrents have been shut down. If some trackers are down, they will time out. All this before the destructor of session returns. So, it's advised that any kind of interface (such as windows) are closed before @@ -1252,7 +1252,7 @@ public: };

-

torrent_info()

+

torrent_info()

 torrent_info();
@@ -1760,7 +1760,7 @@ sha1_hash info_hash() const;
 
info_hash() returns the info-hash for the torrent.
-

set_max_uploads() set_max_connections()

+

set_max_uploads() set_max_connections()

 void set_max_uploads(int max_uploads) const;
@@ -1809,7 +1809,7 @@ std::vector<char> const& metadata() const;
 it will produce the same hash as the info-hash.
-

status()

+

status()

 torrent_status status() const;
@@ -2325,7 +2325,7 @@ public:
-

ip_filter()

+

ip_filter()

 ip_filter()
diff --git a/docs/manual.rst b/docs/manual.rst
index eba3c12b7..df78e54f6 100755
--- a/docs/manual.rst
+++ b/docs/manual.rst
@@ -86,7 +86,7 @@ libtorrent has been successfully compiled and tested on:
 
 	* Windows 2000 vc7.1
 	* Linux x86 GCC 3.3, GCC 3.4.2
-	* MacOS X (darwin), (Apple's) GCC 4.0
+	* MacOS X (darwin), (Apple's) GCC 3.3, (Apple's) GCC 4.0
 	* SunOS 5.8 GCC 3.1
 	* Cygwin GCC 3.3.3
 
@@ -94,7 +94,6 @@ Fails on:
 
 	* GCC 2.95.4
 	* msvc6
-	* (Apple's) GCC 3.3 (compiler crashes with the latest version of asio)
 
 libtorrent is released under the BSD-license_.
 
@@ -275,6 +274,9 @@ When building the example client on windows, you need to build with
 ``link=static`` otherwise you may get unresolved external symbols for some
 boost.program-options symbols.
 
+For more information, see the `Boost build v2 documentation`__.
+
+__ http://www.boost.org/tools/build/v2/index.html
 
 building with autotools
 -----------------------
diff --git a/examples/client_test.cpp b/examples/client_test.cpp
index 9af8cec26..a16a1e73c 100755
--- a/examples/client_test.cpp
+++ b/examples/client_test.cpp
@@ -689,14 +689,17 @@ int main(int ac, char* av[])
 			{
 				if (torrent_finished_alert* p = dynamic_cast(a.get()))
 				{
-					// limit the bandwidth for all seeding torrents
 					p->handle.set_max_connections(60);
-					//p->handle.set_max_uploads(5);
-					//p->handle.set_upload_limit(10000);
 
-					// all finished downloades are
-					// moved into this directory
-					//p->handle.move_storage("finished");
+					// write resume data for the finished torrent
+					torrent_handle h = p->handle;
+					entry data = h.write_resume_data();
+					std::stringstream s;
+					s << h.get_torrent_info().name() << ".fastresume";
+					boost::filesystem::ofstream out(h.save_path() / s.str(), std::ios_base::binary);
+					out.unsetf(std::ios_base::skipws);
+					bencode(std::ostream_iterator(out), data);
+
 					events.push_back(now + ": "
 						+ p->handle.get_torrent_info().name() + ": " + a->msg());
 				}
@@ -794,13 +797,8 @@ int main(int ac, char* av[])
 					out << "tracker: " << s.current_tracker << "\n";
 				}
 
-				out << "___________________________________\n";
-
 				if (print_peers && !peers.empty())
-				{
 					print_peer_info(out, peers);
-					out << "___________________________________\n";
-				}
 
 				if (print_downloads && s.state != torrent_status::seeding)
 				{
diff --git a/include/Makefile.am b/include/Makefile.am
index c3a05efc8..8535745e5 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -1,4 +1,4 @@
-pkginclude_HEADERS = libtorrent/alert.hpp \
+nobase_pkginclude_HEADERS = libtorrent/alert.hpp \
 libtorrent/alert_types.hpp \
 libtorrent/allocate_resources.hpp \
 libtorrent/bencode.hpp \
diff --git a/include/libtorrent/peer_connection.hpp b/include/libtorrent/peer_connection.hpp
index e2df10238..3e58a475f 100755
--- a/include/libtorrent/peer_connection.hpp
+++ b/include/libtorrent/peer_connection.hpp
@@ -52,6 +52,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include 
 #include 
 #include 
+#include 
 
 #ifdef _MSC_VER
 #pragma warning(pop)
@@ -540,7 +541,7 @@ namespace libtorrent
 		bool m_reading;
 		int m_last_read_size;		
 		// reference counter for intrusive_ptr
-		mutable int m_refs;
+		mutable boost::detail::atomic_count m_refs;
 
 #ifndef NDEBUG
 	public:
diff --git a/include/libtorrent/policy.hpp b/include/libtorrent/policy.hpp
index a6cd0f00f..975180c61 100755
--- a/include/libtorrent/policy.hpp
+++ b/include/libtorrent/policy.hpp
@@ -178,6 +178,10 @@ namespace libtorrent
 		{
 			return m_num_unchoked;
 		}
+		
+		typedef std::vector::iterator iterator;
+		iterator begin_peer() { return m_peers.begin(); }
+		iterator end_peer() { return m_peers.end(); }
 
 	private:
 
@@ -209,6 +213,7 @@ namespace libtorrent
 
 				ptime not_tried_yet(boost::gregorian::date(1970,boost::gregorian::Jan,1));
 
+				// this timeout has to be customizable!
 				return p.connection == 0
 					&& p.connected != not_tried_yet
 					&& second_clock::universal_time() - p.connected > minutes(30);
diff --git a/include/libtorrent/torrent.hpp b/include/libtorrent/torrent.hpp
index 40f24eb9b..f0fe154b1 100755
--- a/include/libtorrent/torrent.hpp
+++ b/include/libtorrent/torrent.hpp
@@ -360,9 +360,14 @@ namespace libtorrent
 			assert(m_picker.get());
 			return *m_picker;
 		}
-		policy& get_policy() { return *m_policy; }
+		policy& get_policy()
+		{
+			assert(m_policy);
+			return *m_policy;
+		}
 		piece_manager& filesystem();
-		torrent_info const& torrent_file() const { return m_torrent_file; }
+		torrent_info const& torrent_file() const
+		{ return m_torrent_file; }
 
 		std::vector const& trackers() const
 		{ return m_trackers; }
diff --git a/src/http_tracker_connection.cpp b/src/http_tracker_connection.cpp
index 4fcd45133..d3dd5ae5f 100755
--- a/src/http_tracker_connection.cpp
+++ b/src/http_tracker_connection.cpp
@@ -375,7 +375,7 @@ namespace libtorrent
 		}
 #endif
 
-		tcp::resolver::query q(*connect_to_host, "http");
+		tcp::resolver::query q(*connect_to_host, "https");
 		m_name_lookup.async_resolve(q
 			, boost::bind(&http_tracker_connection::name_lookup, self(), _1, _2));
 		set_timeout(m_settings.tracker_completion_timeout
diff --git a/src/peer_connection.cpp b/src/peer_connection.cpp
index f362f5d3b..30a855bcc 100755
--- a/src/peer_connection.cpp
+++ b/src/peer_connection.cpp
@@ -65,8 +65,7 @@ namespace libtorrent
 	{
 		assert(c->m_refs > 0);
 		assert(c != 0);
-		--c->m_refs;
-		if (c->m_refs == 0)
+		if (--c->m_refs == 0)
 			delete c;
 	}
 
@@ -467,7 +466,11 @@ namespace libtorrent
 
 		boost::shared_ptr t = m_torrent.lock();
 
-		if (t && t->is_aborted()) { m_torrent.reset(); t.reset(); }
+		if (t && t->is_aborted())
+		{
+			m_torrent.reset();
+			t.reset();
+		}
 
 		if (!t)
 		{
@@ -1669,7 +1672,8 @@ namespace libtorrent
 		assert(packet_size > 0);
 		m_recv_pos = 0;
 		m_packet_size = packet_size;
-		m_recv_buffer.resize(m_packet_size);
+		if (int(m_recv_buffer.size()) < m_packet_size)
+			m_recv_buffer.resize(m_packet_size);
 	}
 	
 	void peer_connection::send_buffer(char const* begin, char const* end)
@@ -1830,6 +1834,11 @@ namespace libtorrent
 			return;
 		}
 
+		// the connection cannot time out while connecting
+		// so we don't need to check m_disconnecting
+		assert(m_disconnecting == false);
+		m_last_receive = second_clock::universal_time();
+
 		// this means the connection just succeeded
 
 #if defined(TORRENT_VERBOSE_LOGGING) || defined(TORRENT_LOGGING)
@@ -1914,7 +1923,17 @@ namespace libtorrent
 	void peer_connection::check_invariant() const
 	{
 		boost::shared_ptr t = m_torrent.lock();
-		if (!t) return;
+		if (!t)
+		{
+			typedef detail::session_impl::torrent_map torrent_map;
+			torrent_map& m = m_ses.m_torrents;
+			for (torrent_map::iterator i = m.begin(), end(m.end()); i != end; ++i)
+			{
+				torrent& t = *i->second;
+				assert(t.connection_for(m_remote) != this);
+			}
+			return;
+		}
 
 		if (!m_in_constructor && t->connection_for(remote()) != this)
 		{
diff --git a/src/policy.cpp b/src/policy.cpp
index 6bc65416d..714a36f93 100755
--- a/src/policy.cpp
+++ b/src/policy.cpp
@@ -829,7 +829,7 @@ namespace libtorrent
 
 		if (i == m_peers.end())
 		{
-			// this is probably a http seed
+			// this is probably an http seed
 			if (web_peer_connection const* p = dynamic_cast(&c))
 			{
 				m_torrent->remove_url_seed(p->url());
diff --git a/src/session.cpp b/src/session.cpp
index 22c2491c2..fa8521070 100755
--- a/src/session.cpp
+++ b/src/session.cpp
@@ -117,21 +117,39 @@ namespace libtorrent { namespace detail
 					if (m_torrents.empty() && !m_abort && !processing)
 						m_cond.wait(l);
 
-					if (m_abort) return;
+					if (m_abort)
+					{
+						// no lock is needed here, because the main thread
+						// has already been shut down by now
+						processing.reset();
+						t.reset();
+						std::for_each(m_torrents.begin(), m_torrents.end()
+							, boost::bind(&torrent::abort
+							, boost::bind(&shared_ptr::get
+							, boost::bind(&piece_checker_data::torrent_ptr, _1))));
+						m_torrents.clear();
+						std::for_each(m_processing.begin(), m_processing.end()
+							, boost::bind(&torrent::abort
+							, boost::bind(&shared_ptr::get
+							, boost::bind(&piece_checker_data::torrent_ptr, _1))));
+						m_processing.clear();
+						return;
+					}
 
 					if (!m_torrents.empty())
 					{
 						t = m_torrents.front();
 						if (t->abort)
 						{
-							if (processing->torrent_ptr->num_peers())
-							{
-								m_ses.m_torrents.insert(std::make_pair(
-									t->info_hash, t->torrent_ptr));
-								t->torrent_ptr->abort();
-							}
-
+							// make sure the locking order is
+							// consistent to avoid dead locks
+							// we need to lock the session because closing
+							// torrents assume to have access to it
+							l.unlock();
+							session_impl::mutex_t::scoped_lock l2(m_ses.m_mutex);
+							l.lock();
 
+							t->torrent_ptr->abort();
 							m_torrents.pop_front();
 							continue;
 						}
@@ -145,12 +163,11 @@ namespace libtorrent { namespace detail
 
 					if (!error_msg.empty() && m_ses.m_alerts.should_post(alert::warning))
 					{
-						session_impl::mutex_t::scoped_lock l2(m_ses.m_mutex);
+						session_impl::mutex_t::scoped_lock l(m_ses.m_mutex);
 						m_ses.m_alerts.post_alert(fastresume_rejected_alert(
 							t->torrent_ptr->get_handle()
 							, error_msg));
 #if defined(TORRENT_VERBOSE_LOGGING) || defined(TORRENT_LOGGING)
-						session_impl::mutex_t::scoped_lock l(m_ses.m_mutex);
 						(*m_ses.m_logger) << "fastresume data for "
 							<< t->torrent_ptr->torrent_file().name() << " rejected: "
 							<< error_msg << "\n";
@@ -172,21 +189,31 @@ namespace libtorrent { namespace detail
 
 						t->torrent_ptr->files_checked(t->unfinished_pieces);
 						m_torrents.pop_front();
-						m_ses.m_torrents.insert(std::make_pair(t->info_hash, t->torrent_ptr));
-						if (t->torrent_ptr->is_seed() && m_ses.m_alerts.should_post(alert::info))
-						{
-							m_ses.m_alerts.post_alert(torrent_finished_alert(
-								t->torrent_ptr->get_handle()
-								, "torrent is complete"));
-						}
 
-						peer_id id;
-						std::fill(id.begin(), id.end(), 0);
-						for (std::vector::const_iterator i = t->peers.begin();
-							i != t->peers.end(); ++i)
+						// we cannot add the torrent if the session is aborted.
+						if (!m_ses.m_abort)
 						{
-							t->torrent_ptr->get_policy().peer_from_tracker(*i, id);
+							m_ses.m_torrents.insert(std::make_pair(t->info_hash, t->torrent_ptr));
+							if (t->torrent_ptr->is_seed() && m_ses.m_alerts.should_post(alert::info))
+							{
+								m_ses.m_alerts.post_alert(torrent_finished_alert(
+									t->torrent_ptr->get_handle()
+									, "torrent is complete"));
+							}
+
+							peer_id id;
+							std::fill(id.begin(), id.end(), 0);
+							for (std::vector::const_iterator i = t->peers.begin();
+								i != t->peers.end(); ++i)
+							{
+								t->torrent_ptr->get_policy().peer_from_tracker(*i, id);
+							}
 						}
+						else
+						{
+							t->torrent_ptr->abort();
+						}
+						t.reset();
 						continue;
 					}
 
@@ -202,6 +229,7 @@ namespace libtorrent { namespace detail
 						{
 							processing = t;
 							processing->processing = true;
+							t.reset();
 						}
 					}
 				}
@@ -219,12 +247,7 @@ namespace libtorrent { namespace detail
 							t->torrent_ptr->get_handle()
 							, e.what()));
 				}
-				if (t->torrent_ptr->num_peers())
-				{
-					m_ses.m_torrents.insert(std::make_pair(
-						t->info_hash, t->torrent_ptr));
-					t->torrent_ptr->abort();
-				}
+				t->torrent_ptr->abort();
 
 				assert(!m_torrents.empty());
 				m_torrents.pop_front();
@@ -258,12 +281,7 @@ namespace libtorrent { namespace detail
 						assert(!m_processing.empty());
 						assert(m_processing.front() == processing);
 
-						if (processing->torrent_ptr->num_peers())
-						{
-							m_ses.m_torrents.insert(std::make_pair(
-								processing->info_hash, processing->torrent_ptr));
-							processing->torrent_ptr->abort();
-						}
+						processing->torrent_ptr->abort();
 
 						processing.reset();
 						m_processing.pop_front();
@@ -284,23 +302,33 @@ namespace libtorrent { namespace detail
 					assert(!m_processing.empty());
 					assert(m_processing.front() == processing);
 
-					processing->torrent_ptr->files_checked(processing->unfinished_pieces);
-					m_ses.m_torrents.insert(std::make_pair(
-						processing->info_hash, processing->torrent_ptr));
-					if (processing->torrent_ptr->is_seed()
-						&& m_ses.m_alerts.should_post(alert::info))
+					// TODO: factor out the adding of torrents to the session
+					// and to the checker thread to avoid duplicating the
+					// check for abortion.
+					if (!m_ses.m_abort)
 					{
-						m_ses.m_alerts.post_alert(torrent_finished_alert(
-							processing->torrent_ptr->get_handle()
-							, "torrent is complete"));
-					}
+						processing->torrent_ptr->files_checked(processing->unfinished_pieces);
+						m_ses.m_torrents.insert(std::make_pair(
+							processing->info_hash, processing->torrent_ptr));
+						if (processing->torrent_ptr->is_seed()
+							&& m_ses.m_alerts.should_post(alert::info))
+						{
+							m_ses.m_alerts.post_alert(torrent_finished_alert(
+								processing->torrent_ptr->get_handle()
+								, "torrent is complete"));
+						}
 
-					peer_id id;
-					std::fill(id.begin(), id.end(), 0);
-					for (std::vector::const_iterator i = processing->peers.begin();
+						peer_id id;
+						std::fill(id.begin(), id.end(), 0);
+						for (std::vector::const_iterator i = processing->peers.begin();
 							i != processing->peers.end(); ++i)
+						{
+							processing->torrent_ptr->get_policy().peer_from_tracker(*i, id);
+						}
+					}
+					else
 					{
-						processing->torrent_ptr->get_policy().peer_from_tracker(*i, id);
+						processing->torrent_ptr->abort();
 					}
 					processing.reset();
 					m_processing.pop_front();
@@ -311,7 +339,7 @@ namespace libtorrent { namespace detail
 					}
 				}
 			}
-			catch(const std::exception& e)
+			catch(std::exception const& e)
 			{
 				// This will happen if the storage fails to initialize
 				session_impl::mutex_t::scoped_lock l(m_ses.m_mutex);
@@ -326,12 +354,7 @@ namespace libtorrent { namespace detail
 				}
 				assert(!m_processing.empty());
 
-				if (processing->torrent_ptr->num_peers())
-				{
-					m_ses.m_torrents.insert(std::make_pair(
-						processing->info_hash, processing->torrent_ptr));
-					processing->torrent_ptr->abort();
-				}
+				processing->torrent_ptr->abort();
 
 				processing.reset();
 				m_processing.pop_front();
@@ -604,6 +627,8 @@ namespace libtorrent { namespace detail
 		mutex_t::scoped_lock l(m_mutex);
 		assert(listen_socket.lock() == m_listen_socket);
 
+		if (m_abort) return;
+
 		async_accept();
 		if (e)
 		{
@@ -715,6 +740,7 @@ namespace libtorrent { namespace detail
 		if (p->is_connecting())
 		{
 			assert(p->is_local());
+			assert(m_connections.find(p->get_socket()) == m_connections.end());
 			// Since this peer is still connecting, will not be
 			// in the list of completed connections.
 			connection_map::iterator i = m_half_open.find(p->get_socket());
@@ -725,6 +751,7 @@ namespace libtorrent { namespace detail
 				connection_queue::iterator j = std::find(
 					m_connection_queue.begin(), m_connection_queue.end(), p);
 					
+				assert(j != m_connection_queue.end());
 				if (j != m_connection_queue.end())
 					m_connection_queue.erase(j);
 			}
@@ -736,7 +763,11 @@ namespace libtorrent { namespace detail
 		}
 		else
 		{
+			assert(m_half_open.find(p->get_socket()) == m_half_open.end());
+			assert(std::find(m_connection_queue.begin()
+				, m_connection_queue.end(), p) == m_connection_queue.end());
 			connection_map::iterator i = m_connections.find(p->get_socket());
+//			assert (i != m_connections.end());
 			if (i != m_connections.end())
 				m_connections.erase(i);
 		}
@@ -744,6 +775,8 @@ namespace libtorrent { namespace detail
 
 	void session_impl::second_tick(asio::error const& e) try
 	{
+		session_impl::mutex_t::scoped_lock l(m_mutex);
+
 		if (e)
 		{
 #if defined(TORRENT_LOGGING)
@@ -753,9 +786,7 @@ namespace libtorrent { namespace detail
 			m_selector.interrupt();
 			return;
 		}
-		
-		session_impl::mutex_t::scoped_lock l(m_mutex);
-		
+
 		if (m_abort) return;
 		float tick_interval = (microsec_clock::universal_time()
 			- m_last_tick).total_milliseconds() / 1000.f;
@@ -777,26 +808,27 @@ namespace libtorrent { namespace detail
 			++i;
 			// if this socket has timed out
 			// close it.
-			if (j->second->has_timed_out())
+			peer_connection& c = *j->second;
+			if (c.has_timed_out())
 			{
 				if (m_alerts.should_post(alert::debug))
 				{
 					m_alerts.post_alert(
 						peer_error_alert(
-							j->second->remote()
-							, j->second->pid()
+							c.remote()
+							, c.pid()
 							, "connection timed out"));
 				}
 #if defined(TORRENT_VERBOSE_LOGGING)
-				(*j->second->m_logger) << "*** CONNECTION TIMED OUT\n";
+				(*c.m_logger) << "*** CONNECTION TIMED OUT\n";
 #endif
 
-				j->second->set_failed();
-				j->second->disconnect();
+				c.set_failed();
+				c.disconnect();
 				continue;
 			}
 
-			j->second->keep_alive();
+			c.keep_alive();
 		}
 
 		// check each torrent for tracker updates
@@ -923,40 +955,55 @@ namespace libtorrent { namespace detail
 		}
 		catch (std::exception& e)
 		{
-			std::cerr << e.what() << "\n";
 			#ifndef NDEBUG
+			std::cerr << e.what() << "\n";
 			std::string err = e.what();
 			#endif
 			assert(false);
 		}
 
 		{
-		session_impl::mutex_t::scoped_lock l(m_mutex);
+			session_impl::mutex_t::scoped_lock l(m_mutex);
 
-		m_tracker_manager.abort_all_requests();
-		for (std::map >::iterator i =
-			m_torrents.begin(); i != m_torrents.end(); ++i)
-		{
-			i->second->abort();
-			if (!i->second->is_paused() || i->second->should_request())
+			m_tracker_manager.abort_all_requests();
+			for (std::map >::iterator i =
+				m_torrents.begin(); i != m_torrents.end(); ++i)
 			{
-				tracker_request req = i->second->generate_tracker_request();
-				req.listen_port = m_listen_interface.port();
-				req.key = m_key;
-				std::string login = i->second->tracker_login();
-				m_tracker_manager.queue_request(m_selector, req, login);
+				i->second->abort();
+				if (!i->second->is_paused() || i->second->should_request())
+				{
+					tracker_request req = i->second->generate_tracker_request();
+					req.listen_port = m_listen_interface.port();
+					req.key = m_key;
+					std::string login = i->second->tracker_login();
+					m_tracker_manager.queue_request(m_selector, req, login);
+				}
 			}
-		}
-		m_timer.expires_from_now(boost::posix_time::seconds(
-			m_settings.stop_tracker_timeout));
-		m_timer.async_wait(bind(&demuxer::interrupt, &m_selector));
+			m_timer.expires_from_now(boost::posix_time::seconds(
+				m_settings.stop_tracker_timeout));
+			m_timer.async_wait(bind(&demuxer::interrupt, &m_selector));
 		}
 
 		m_selector.reset();
 		m_selector.run();
 
-		m_torrents.clear();
+		session_impl::mutex_t::scoped_lock l(m_mutex);
+		assert(m_abort);
+		m_abort = true;
+
 		m_connections.clear();
+		m_half_open.clear();
+		m_connection_queue.clear();
+
+#ifndef NDEBUG
+		for (torrent_map::iterator i = m_torrents.begin();
+			i != m_torrents.end(); ++i)
+		{
+			assert(i->second->num_peers() == 0);
+		}
+#endif
+
+		m_torrents.clear();
 
 		assert(m_torrents.empty());
 		assert(m_connections.empty());
@@ -1263,6 +1310,9 @@ namespace libtorrent
 		if (!m_impl.find_torrent(info_hash).expired())
 			throw duplicate_torrent();
 
+		// you cannot add new torrents to a session that is closing down
+		assert(!m_impl.m_abort);
+
 		// create the torrent and the data associated with
 		// the checker thread and store it before starting
 		// the thread
@@ -1407,6 +1457,14 @@ namespace libtorrent
 			m_impl.m_abort = true;
 			m_impl.m_selector.interrupt();
 		}
+		m_thread.join();
+
+		// it's important that the main thread is closed completely before
+		// the checker thread is terminated. Because all the connections
+		// have to be closed and removed from the torrents before they
+		// can be destructed. (because the weak pointers in the
+		// peer_connections will be invalidated when the torrents are
+		// destructed and then the invariant will be broken).
 
 		{
 			mutex::scoped_lock l(m_checker_impl.m_mutex);
@@ -1421,8 +1479,10 @@ namespace libtorrent
 			m_checker_impl.m_cond.notify_one();
 		}
 
-		m_thread.join();
 		m_checker_thread.join();
+
+		assert(m_impl.m_torrents.empty());
+		assert(m_impl.m_connections.empty());
 	}
 
 	void session::set_max_uploads(int limit)
diff --git a/src/torrent.cpp b/src/torrent.cpp
index 03d18e87f..22f9f1f09 100755
--- a/src/torrent.cpp
+++ b/src/torrent.cpp
@@ -372,6 +372,17 @@ namespace libtorrent
 
 	torrent::~torrent()
 	{
+		// The invariant can't be maintained here, since the torrent
+		// is being destructed, all weak references to it have been
+		// reset, which means that all its peers already have an
+		// invalidated torrent pointer (so it cannot be verified to be correct)
+		
+		// i.e. the invariant can only be maintained if all connections have
+		// been closed by the time the torrent is destructed. And they are
+		// supposed to be closed. So we can still do the invariant check.
+
+		assert(m_connections.empty());
+		
 		INVARIANT_CHECK;
 
 		if (m_ses.m_abort)
@@ -523,6 +534,8 @@ namespace libtorrent
 	// been filtered as not wanted we have downloaded
 	tuple torrent::bytes_done() const
 	{
+		INVARIANT_CHECK;
+
 		if (!valid_metadata()) return tuple(0,0);
 
 		assert(m_picker.get());
@@ -796,6 +809,8 @@ namespace libtorrent
 
 	void torrent::filtered_pieces(std::vector& bitmask) const
 	{
+		INVARIANT_CHECK;
+
 		// this call is only valid on torrents with metadata
 		assert(m_picker.get());
 		m_picker->filtered_pieces(bitmask);
@@ -803,6 +818,8 @@ namespace libtorrent
 
 	void torrent::filter_files(std::vector const& bitmask)
 	{
+		INVARIANT_CHECK;
+
 		// this call is only valid on torrents with metadata
 		if (!valid_metadata()) return;
 
@@ -950,7 +967,7 @@ namespace libtorrent
 		detail::session_impl::mutex_t::scoped_lock l(m_ses.m_mutex);
 
 		INVARIANT_CHECK;
-
+		
 		std::set::iterator i = m_resolving_web_seeds.find(url);
 		if (i != m_resolving_web_seeds.end()) m_resolving_web_seeds.erase(i);
 
@@ -970,6 +987,8 @@ namespace libtorrent
 			return;
 		}
 
+		if (m_ses.m_abort) return;
+
 		tcp::endpoint a(host->endpoint().address(), port);
 
 		boost::shared_ptr s(new stream_socket(m_ses.m_selector));
@@ -1073,7 +1092,11 @@ namespace libtorrent
 			throw protocol_error("peer is not properly constructed");
 		}
 
-		
+		if (m_ses.m_abort)
+		{
+			throw protocol_error("session is closing");
+		}
+
 		peer_iterator i = m_connections.insert(
 			std::make_pair(p->remote(), p)).first;
 
@@ -1158,6 +1181,8 @@ namespace libtorrent
 	// called when torrent is complete (all pieces downloaded)
 	void torrent::completed()
 	{
+		INVARIANT_CHECK;
+
 /*
 		if (alerts().should_post(alert::info))
 		{
@@ -1177,6 +1202,8 @@ namespace libtorrent
 	// the begining) and return the new index to the tracker.
 	int torrent::prioritize_tracker(int index)
 	{
+		INVARIANT_CHECK;
+
 		assert(index >= 0);
 		if (index >= (int)m_trackers.size()) return (int)m_trackers.size()-1;
 
@@ -1294,6 +1321,8 @@ namespace libtorrent
 
 	bool torrent::move_storage(boost::filesystem::path const& save_path)
 	{
+		INVARIANT_CHECK;
+
 		bool ret = true;
 		if (m_storage.get())
 		{
@@ -1309,6 +1338,8 @@ namespace libtorrent
 
 	piece_manager& torrent::filesystem()
 	{
+		INVARIANT_CHECK;
+
 		assert(m_storage.get());
 		return *m_storage;
 	}
@@ -1316,11 +1347,15 @@ namespace libtorrent
 
 	torrent_handle torrent::get_handle() const
 	{
+		INVARIANT_CHECK;
+
 		return torrent_handle(&m_ses, 0, m_torrent_file.info_hash());
 	}
 
 	session_settings const& torrent::settings() const
 	{
+		INVARIANT_CHECK;
+
 		return m_ses.m_settings;
 	}
 
@@ -1331,7 +1366,12 @@ namespace libtorrent
 //		size_type done = boost::get<0>(bytes_done());
 //		assert(download >= done - m_initial_done);
 		for (const_peer_iterator i = begin(); i != end(); ++i)
-			assert(i->second->associated_torrent().lock().get() == this);
+		{
+			peer_connection const& p = *i->second;
+			torrent* associated_torrent = p.associated_torrent().lock().get();
+			if (associated_torrent != this)
+				assert(false);
+		}
 
 // This check is very expensive.
 //		assert(m_num_pieces
@@ -1857,6 +1897,8 @@ namespace libtorrent
 
 	std::pair torrent::metadata_request()
 	{
+		INVARIANT_CHECK;
+
 		// count the number of peers that supports the
 		// extension and that has metadata
 		int peers = 0;
@@ -1909,9 +1951,11 @@ namespace libtorrent
 
 	void torrent::cancel_metadata_request(std::pair req)
 	{
+		INVARIANT_CHECK;
+
 		for (int i = req.first; i < req.first + req.second; ++i)
 		{
-            assert(m_requested_metadata[i] > 0);
+			assert(m_requested_metadata[i] > 0);
 			if (m_requested_metadata[i] > 0)
 				--m_requested_metadata[i];
 		}
@@ -1921,9 +1965,12 @@ namespace libtorrent
 		tracker_request const&)
 	{
 		session_impl::mutex_t::scoped_lock l(m_ses.m_mutex);
+		INVARIANT_CHECK;
+
 #if defined(TORRENT_VERBOSE_LOGGING) || defined(TORRENT_LOGGING)
 		debug_log("*** tracker timed out");
 #endif
+
 		if (m_ses.m_alerts.should_post(alert::warning))
 		{
 			std::stringstream s;
@@ -1942,6 +1989,8 @@ namespace libtorrent
 	void torrent::tracker_request_error(tracker_request const&
 		, int response_code, const std::string& str)
 	{
+		INVARIANT_CHECK;
+
 		session_impl::mutex_t::scoped_lock l(m_ses.m_mutex);
 #if defined(TORRENT_VERBOSE_LOGGING) || defined(TORRENT_LOGGING)
 		debug_log(std::string("*** tracker error: ") + str);
@@ -1956,7 +2005,6 @@ namespace libtorrent
 				, m_failed_trackers + 1, response_code, s.str()));
 		}
 
-
 		try_next_tracker();
 	}
 
@@ -1968,11 +2016,11 @@ namespace libtorrent
 	}
 #endif
 
-}
-
-void torrent::metadata_progress(int total_size, int received)
-{
-	m_metadata_progress += received;
-	m_metadata_size = total_size;
+	void torrent::metadata_progress(int total_size, int received)
+	{
+		m_metadata_progress += received;
+		m_metadata_size = total_size;
+	}
+
 }
 
diff --git a/src/torrent_handle.cpp b/src/torrent_handle.cpp
index 815dc73e0..623540022 100755
--- a/src/torrent_handle.cpp
+++ b/src/torrent_handle.cpp
@@ -459,18 +459,23 @@ namespace libtorrent
 
 		ret["peers"] = entry::list_type();
 		entry::list_type& peer_list = ret["peers"].list();
+		
+		policy& pol = t->get_policy();
 
-		for (torrent::const_peer_iterator i = t->begin();
-			i != t->end(); ++i)
+		for (policy::iterator i = pol.begin_peer()
+			, end(pol.end_peer()); i != end; ++i)
 		{
 			// we cannot save remote connection
 			// since we don't know their listen port
-			// TODO: iterate the peers in the policy
-			// instead, since peers may be remote
-			// but still connectable
-			if (!i->second->is_local()) continue;
+			// unless they gave us their listen port
+			// through the extension handshake
+			// so, if the peer is not connectable (i.e. we
+			// don't know its listen port) or if it has
+			// been banned, don't save it.
+			if (i->type == policy::peer::not_connectable
+				|| i->banned) continue;
 
-			tcp::endpoint ip = i->second->remote();
+			tcp::endpoint ip = i->ip;
 			entry peer(entry::dictionary_t);
 			peer["ip"] = ip.address().to_string();
 			peer["port"] = ip.port();
@@ -520,9 +525,18 @@ namespace libtorrent
 		session_impl::mutex_t::scoped_lock l(m_ses->m_mutex);
 		boost::shared_ptr t = m_ses->find_torrent(m_info_hash).lock();
 		
-		// TODO: if the torrent is being checked, put this peer in a queue and
-		// connect it once the checking is done
-		if (!t) throw_invalid_handle();
+		if (!t)
+		{
+			// the torrent is being checked. Add the peer to its
+			// peer list. The entries in there will be connected
+			// once the checking is complete.
+			mutex::scoped_lock l2(m_chk->m_mutex);
+
+			detail::piece_checker_data* d = m_chk->find_torrent(m_info_hash);
+			if (d == 0) throw_invalid_handle();
+			d->peers.push_back(adr);
+			return;
+		}
 
 		peer_id id;
 		std::fill(id.begin(), id.end(), 0);
diff --git a/test/Jamfile b/test/Jamfile
index e6ecbf5db..d5a28f407 100644
--- a/test/Jamfile
+++ b/test/Jamfile
@@ -5,6 +5,7 @@ project
 	requirements multi
 	/torrent
 	main.cpp
+	setup_transfer.cpp
    ;
 
 test-suite libtorrent : 
diff --git a/test/Makefile.am b/test/Makefile.am
index dd9d1ac4d..885abad04 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -19,10 +19,10 @@ test_storage_LDADD = $(top_builddir)/src/libtorrent.la
 test_buffer_SOURCES = main.cpp test_buffer.cpp
 test_buffer_LDADD = $(top_builddir)/src/libtorrent.la
 
-test_metadata_extension_SOURCES = main.cpp test_metadata_extension.cpp
+test_metadata_extension_SOURCES = main.cpp setup_transfer.cpp test_metadata_extension.cpp
 test_metadata_extension_LDADD = $(top_builddir)/src/libtorrent.la
 
-noinst_HEADERS = test.hpp
+noinst_HEADERS = test.hpp setup_transfer.hpp
 
 AM_CXXFLAGS=-ftemplate-depth-50 -I$(top_srcdir)/include -I$(top_srcdir)/asio/include @DEBUGFLAGS@ @PTHREAD_CFLAGS@
 AM_LDFLAGS= -L./ -l@BOOST_DATE_TIME_LIB@ -l@BOOST_FILESYSTEM_LIB@ -l@BOOST_THREAD_LIB@ @PTHREAD_LIBS@ 
diff --git a/test/setup_transfer.cpp b/test/setup_transfer.cpp
new file mode 100644
index 000000000..24a2419cf
--- /dev/null
+++ b/test/setup_transfer.cpp
@@ -0,0 +1,65 @@
+#include "libtorrent/session.hpp"
+#include "libtorrent/hasher.hpp"
+#include 
+#include 
+
+#include "test.hpp"
+
+void sleep(int msec)
+{
+	boost::xtime xt;
+	boost::xtime_get(&xt, boost::TIME_UTC);
+	xt.nsec += msec * 1000000;
+	boost::thread::sleep(xt);
+}
+
+using namespace libtorrent;
+
+boost::tuple setup_transfer(
+	session& ses1, session& ses2, bool clear_files)
+{
+	using namespace boost::filesystem;
+
+	char const* tracker_url = "http://non-existent-name.com/announce";
+	
+	torrent_info t;
+	t.add_file(path("temporary"), 42);
+	t.set_piece_size(256 * 1024);
+	t.add_tracker(tracker_url);
+
+	std::vector piece(42);
+	std::fill(piece.begin(), piece.end(), 0xfe);
+	
+	// calculate the hash for all pieces
+	int num = t.num_pieces();
+	for (int i = 0; i < num; ++i)
+	{
+		t.set_hash(i, hasher(&piece[0], piece.size()).final());
+	}
+	
+	create_directory("./tmp1");
+	std::ofstream file("./tmp1/temporary");
+	file.write(&piece[0], piece.size());
+	file.close();
+	if (clear_files) remove_all("./tmp2/temporary");
+	
+	t.create_torrent();
+
+
+	ses1.set_severity_level(alert::debug);
+	ses2.set_severity_level(alert::debug);
+	
+	// they should not use the same save dir, because the
+	// file pool will complain if two torrents are trying to
+	// use the same files
+	torrent_handle tor1 = ses1.add_torrent(t, "./tmp1");
+	torrent_handle tor2 = ses2.add_torrent(tracker_url
+		, t.info_hash(), "./tmp2");
+
+	std::cerr << "connecting peer\n";
+	tor1.connect_peer(tcp::endpoint(address::from_string("127.0.0.1")
+		, ses2.listen_port()));
+
+	return boost::make_tuple(tor1, tor2);
+}
+
diff --git a/test/setup_transfer.hpp b/test/setup_transfer.hpp
new file mode 100644
index 000000000..dcd8a5670
--- /dev/null
+++ b/test/setup_transfer.hpp
@@ -0,0 +1,14 @@
+#ifndef SETUP_TRANSFER_HPP
+#define SETUP_TRANSFER_HPP
+
+#include "libtorrent/session.hpp"
+#include 
+
+
+void sleep(int msec);
+boost::tuple
+	setup_transfer(libtorrent::session& ses1, libtorrent::session& ses2
+	, bool clear_files);
+
+#endif
+
diff --git a/test/test_metadata_extension.cpp b/test/test_metadata_extension.cpp
index e17949bd8..49a4414af 100644
--- a/test/test_metadata_extension.cpp
+++ b/test/test_metadata_extension.cpp
@@ -1,54 +1,27 @@
 #include "libtorrent/session.hpp"
 #include "libtorrent/hasher.hpp"
 #include 
+#include 
 
 #include "test.hpp"
+#include "setup_transfer.hpp"
 
-void sleep(int msec)
-{
-	boost::xtime xt;
-	boost::xtime_get(&xt, boost::TIME_UTC);
-	xt.nsec += msec * 1000000;
-	boost::thread::sleep(xt);
-}
-
-void test_transfer(char const* tracker_url, libtorrent::torrent_info const& t)
+void test_transfer(bool clear_files = true, bool disconnect = false)
 {
 	using namespace libtorrent;
 
 	session ses1;
-	ses1.set_severity_level(alert::debug);
 	session ses2(fingerprint("LT", 0, 1, 0, 0), std::make_pair(49000, 50000));
-	ses2.set_severity_level(alert::debug);
-	
-	// they should not use the same save dir, because the
-	// file pool will complain if two torrents are trying to
-	// use the same files
-	torrent_handle tor1 = ses1.add_torrent(t, "./tmp1");
-	torrent_handle tor2 = ses2.add_torrent(tracker_url
-		, t.info_hash(), "./tmp2");
+	torrent_handle tor1;
+	torrent_handle tor2;
 
-	std::cerr << "waiting for file check to complete\n";
-	
-	// wait for 5 seconds or until the torrent is in a state
-	// were it can accept connections
-	for (int i = 0; i < 50; ++i)
-	{
-		torrent_status st = tor1.status();
-		if (st.state != torrent_status::queued_for_checking
-			&&st.state != torrent_status::checking_files)
-			break;
-		sleep(100);
-	}
-
-	std::cerr << "connecting peer\n";
-	tor1.connect_peer(tcp::endpoint(address::from_string("127.0.0.1"), ses2.listen_port()));
+	boost::tie(tor1, tor2) = setup_transfer(ses1, ses2, clear_files);	
 
 	for (int i = 0; i < 50; ++i)
 	{
 		// make sure this function can be called on
 		// torrents without metadata
-		tor2.status();
+		if (!disconnect) tor2.status();
 		std::auto_ptr a;
 		a = ses1.pop_alert();
 		if (a.get())
@@ -58,12 +31,15 @@ void test_transfer(char const* tracker_url, libtorrent::torrent_info const& t)
 		if (a.get())
 			std::cerr << "ses2: " << a->msg() << "\n";
 
-		if (tor2.has_metadata()) break;
+		if (disconnect && tor2.is_valid()) ses2.remove_torrent(tor2);
+		if (!disconnect && tor2.has_metadata()) break;
 		sleep(100);
 	}
 
+	if (disconnect) return;
+
 	TEST_CHECK(tor2.has_metadata());
-	std::cerr << "metadata received. waiting for transfer to complete\n";
+	std::cerr << "waiting for transfer to complete\n";
 
 	for (int i = 0; i < 50; ++i)
 	{
@@ -73,44 +49,21 @@ void test_transfer(char const* tracker_url, libtorrent::torrent_info const& t)
 	}
 
 	TEST_CHECK(tor2.is_seed());
-	std::cerr << "done\n";
+	if (tor2.is_seed()) std::cerr << "done\n";
 }
 
 int test_main()
 {
 	using namespace libtorrent;
-	using namespace boost::filesystem;
-
-	char const* tracker_url = "http://non-existant-name.com/announce";
 	
-	torrent_info t;
-	t.add_file(path("temporary"), 42);
-	t.set_piece_size(256 * 1024);
-	t.add_tracker(tracker_url);
-
-	std::vector piece(42);
-	std::fill(piece.begin(), piece.end(), 0xfe);
-	
-	// calculate the hash for all pieces
-	int num = t.num_pieces();
-	for (int i = 0; i < num; ++i)
-	{
-		t.set_hash(i, hasher(&piece[0], piece.size()).final());
-	}
-	
-	create_directory("./tmp1");
-	std::ofstream file("./tmp1/temporary");
-	file.write(&piece[0], piece.size());
-	file.close();
-	remove_all("./tmp2/temporary");
-	
-	t.create_torrent();
+	// test to disconnect one client prematurely
+	test_transfer(true, true);
 	
 	// test where one has data and one doesn't
-	test_transfer(tracker_url, t);
+	test_transfer(true);
 
 	// test where both have data (to trigger the file check)
-	test_transfer(tracker_url, t);
+	test_transfer(false);
 
 	return 0;
 }