Merge pull request #117 from thomas-yuan/master

Fix bug caused by RP #96.

include/libtorrent/kademlia/observer.hpp

@@ -95,7 +95,7 @@ struct observer : boost::noncopyable
 	bool has_short_timeout() const { return (flags & flag_short_timeout) != 0; }
 
 	// this is called when no reply has been received within
-	// some timeout
+	// some timeout, or a reply with incorrect format.
 	virtual void timeout();
 
 	// if this is called the destructor should

src/kademlia/find_data.cpp

@@ -56,6 +56,7 @@ void find_data_observer::reply(msg const& m)
 		get_observer()->log(dht_logger::traversal, "[%p] missing response dict"
 			, static_cast<void*>(algorithm()));
 #endif
+		timeout();
 		return;
 	}
 
@@ -66,6 +67,7 @@ void find_data_observer::reply(msg const& m)
 		get_observer()->log(dht_logger::traversal, "[%p] invalid id in response"
 			, static_cast<void*>(algorithm()));
 #endif
+		timeout();
 		return;
 	}
 	bdecode_node token = r.dict_find_string("token");

src/kademlia/get_item.cpp

@@ -271,6 +271,7 @@ void get_item_observer::reply(msg const& m)
 		get_observer()->log(dht_logger::traversal, "[%p] missing response dict"
 			, static_cast<void*>(algorithm()));
 #endif
+		timeout();
 		return;
 	}
 
@@ -286,7 +287,10 @@ void get_item_observer::reply(msg const& m)
 	if (q)
 		seq = q.int_value();
 	else if (pk && sig)
+	{
+		timeout();
 		return;
+	}
 
 	bdecode_node v = r.dict_find("v");
 	if (v)

src/kademlia/get_peers.cpp

@@ -54,6 +54,7 @@ void get_peers_observer::reply(msg const& m)
 		get_observer()->log(dht_logger::traversal, "[%p] missing response dict"
 			, static_cast<void*>(algorithm()));
 #endif
+		timeout();
 		return;
 	}
 
@@ -312,6 +313,7 @@ void obfuscated_get_peers_observer::reply(msg const& m)
 		get_observer()->log(dht_logger::traversal, "[%p] missing response dict"
 			, static_cast<void*>(algorithm()));
 #endif
+		timeout();
 		return;
 	}
 
@@ -322,6 +324,7 @@ void obfuscated_get_peers_observer::reply(msg const& m)
 		get_observer()->log(dht_logger::traversal, "[%p] invalid id in response"
 			, static_cast<void*>(algorithm()));
 #endif
+		timeout();
 		return;
 	}
 

src/kademlia/rpc_manager.cpp

@@ -141,8 +141,6 @@ void observer::short_timeout()
 	m_algorithm->failed(observer_ptr(this), traversal_algorithm::short_timeout);
 }
 
-// this is called when no reply has been received within
-// some timeout
 void observer::timeout()
 {
 	if (flags & flag_done) return;
@@ -302,14 +300,23 @@ bool rpc_manager::incoming(msg const& m, node_id* id
 
 	if (m.message.dict_find_string_value("y") == "e") 
 	{
-		// it's an error.
+		// It's an error.
 #ifndef TORRENT_DISABLE_LOGGING
 		bdecode_node err_ent = m.message.dict_find("e");
 		TORRENT_ASSERT(err_ent);
 		m_log->log(dht_logger::rpc_manager, "reply with error from %s: %s"
 			, print_endpoint(m.addr).c_str(), err_ent.list_string_value_at(1).c_str());
 #endif
-		o->reply(m);
+		// Logically, we should call o->reply(m) since we get a reply.
+		// a reply could be "response" or "error", here the reply is an "error".
+		// if the reply is an "error", basically the observer could/will
+		// do nothing with it, especially when observer::reply() is intended to
+		// handle a "response", not an "error".
+		// A "response" should somehow call algorithm->finished(), and an error/timeout
+		// should call algorithm->failed(). From this point of view,
+		// we should call o->timeout() instead of o->reply(m) because o->reply()
+		// will call algorithm->finished().
+		o->timeout();
 		return false;
 	}