diff --git a/include/libtorrent/kademlia/observer.hpp b/include/libtorrent/kademlia/observer.hpp index 67928a352..6ead9d845 100644 --- a/include/libtorrent/kademlia/observer.hpp +++ b/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 diff --git a/src/kademlia/find_data.cpp b/src/kademlia/find_data.cpp index 886a5680c..54aabba6c 100644 --- a/src/kademlia/find_data.cpp +++ b/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(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(algorithm())); #endif + timeout(); return; } bdecode_node token = r.dict_find_string("token"); diff --git a/src/kademlia/get_item.cpp b/src/kademlia/get_item.cpp index a794ed922..532adcc85 100644 --- a/src/kademlia/get_item.cpp +++ b/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(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) diff --git a/src/kademlia/get_peers.cpp b/src/kademlia/get_peers.cpp index 12d44828d..e49b0ce79 100644 --- a/src/kademlia/get_peers.cpp +++ b/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(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(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(algorithm())); #endif + timeout(); return; } diff --git a/src/kademlia/rpc_manager.cpp b/src/kademlia/rpc_manager.cpp index dcb17c5db..2b56f0fb5 100644 --- a/src/kademlia/rpc_manager.cpp +++ b/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; }