Merge pull request #2784 from gluk256/_bugfix_995

bugfix 995

libp2p/Common.h

@@ -221,7 +221,7 @@ class DeadlineOps
 	{
 	public:
 		DeadlineOp(ba::io_service& _io, unsigned _msInFuture, std::function<void(boost::system::error_code const&)> const& _f): m_timer(new ba::deadline_timer(_io)) { m_timer->expires_from_now(boost::posix_time::milliseconds(_msInFuture)); m_timer->async_wait(_f); }
-		~DeadlineOp() {}
+		~DeadlineOp() { if (m_timer) m_timer->cancel(); }
 		
 		DeadlineOp(DeadlineOp&& _s): m_timer(_s.m_timer.release()) {}
 		DeadlineOp& operator=(DeadlineOp&& _s)
@@ -247,6 +247,8 @@ public:
 	void schedule(unsigned _msInFuture, std::function<void(boost::system::error_code const&)> const& _f) { if (m_stopped) return; DEV_GUARDED(x_timers) m_timers.emplace_back(m_io, _msInFuture, _f); }	
 
 	void stop() { m_stopped = true; DEV_GUARDED(x_timers) m_timers.clear(); }
+
+	bool isStopped() const { return m_stopped; }
 	
 protected:
 	void reap();

libp2p/NodeTable.cpp

@@ -67,8 +67,8 @@ NodeTable::NodeTable(ba::io_service& _io, KeyPair const& _alias, NodeIPEndpoint
 	
 NodeTable::~NodeTable()
 {
-	m_timers.stop();
 	m_socketPointer->disconnect();
+	m_timers.stop();
 }
 
 void NodeTable::processEvents()
@@ -199,7 +199,17 @@ void NodeTable::doDiscover(NodeId _node, unsigned _round, shared_ptr<set<shared_
 	m_timers.schedule(c_reqTimeout.count() * 2, [this, _node, _round, _tried](boost::system::error_code const& _ec)
 	{
 		if (_ec)
-			clog(NodeTableWarn) << "Discovery timer canceled!";
+			clog(NodeTableMessageDetail) << "Discovery timer canceled: " << _ec.value() << _ec.message();
+
+		if (_ec.value() == boost::asio::error::operation_aborted || m_timers.isStopped())
+			return;
+
+		// error::operation_aborted means that the timer was probably aborted. 
+		// It usually happens when "this" object is deallocated, in which case 
+		// subsequent call to doDiscover() would cause a crash. We can not rely on 
+		// m_timers.isStopped(), because "this" pointer was captured by the lambda,
+		// and therefore, in case of deallocation m_timers object no longer exists.
+
 		doDiscover(_node, _round + 1, _tried);
 	});
 }

libp2p/NodeTable.h

@@ -264,7 +264,7 @@ private:
 	std::shared_ptr<NodeSocket> m_socket;							///< Shared pointer for our UDPSocket; ASIO requires shared_ptr.
 	NodeSocket* m_socketPointer;									///< Set to m_socket.get(). Socket is created in constructor and disconnected in destructor to ensure access to pointer is safe.
 
-	DeadlineOps m_timers;
+	DeadlineOps m_timers; ///< this should be the last member - it must be destroyed first
 };
 
 inline std::ostream& operator<<(std::ostream& _out, NodeTable const& _nodeTable)