From d4f4d9bf787f16850ab1cf67b3da8d652d0be6e2 Mon Sep 17 00:00:00 2001
From: JoelKatz <DavidJoelSchwartz@GMail.com>
Date: Thu, 3 Jan 2013 21:25:40 -0800
Subject: [PATCH] Redesign the way the acquire timer is (re)set so that we
 won't have bugs where we fail to arm it.

---
 src/cpp/ripple/LedgerAcquire.cpp   | 23 +++++++++++------------
 src/cpp/ripple/LedgerAcquire.h     |  3 ++-
 src/cpp/ripple/LedgerConsensus.cpp | 24 +++++++++---------------
 3 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/src/cpp/ripple/LedgerAcquire.cpp b/src/cpp/ripple/LedgerAcquire.cpp
index 3d56c9f19..29a2aad70 100644
--- a/src/cpp/ripple/LedgerAcquire.cpp
+++ b/src/cpp/ripple/LedgerAcquire.cpp
@@ -37,7 +37,7 @@ void PeerSet::badPeer(Peer::ref ptr)
 	mPeers.erase(ptr->getPeerId());
 }
 
-void PeerSet::resetTimer()
+void PeerSet::setTimer()
 {
 	mTimer.expires_from_now(boost::posix_time::milliseconds(mTimerInterval));
 	mTimer.async_wait(boost::bind(&PeerSet::TimerEntry, pmDowncast(), boost::asio::placeholders::error));
@@ -45,6 +45,9 @@ void PeerSet::resetTimer()
 
 void PeerSet::invokeOnTimer()
 {
+	if (isDone())
+		return;
+
 	if (!mProgress)
 	{
 		++mTimeouts;
@@ -56,6 +59,9 @@ void PeerSet::invokeOnTimer()
 		mProgress = false;
 		onTimer(true);
 	}
+
+	if (!isDone())
+		setTimer();
 }
 
 void PeerSet::TimerEntry(boost::weak_ptr<PeerSet> wptr, const boost::system::error_code& result)
@@ -120,19 +126,16 @@ void LedgerAcquire::onTimer(bool progress)
 	{
 		setFailed();
 		done();
+		return;
 	}
-	else if (!progress)
+
+	if (!progress)
 	{
 		if (!getPeerCount())
-		{
 			addPeers();
-			resetTimer();
-		}
 		else
 			trigger(Peer::pointer(), true);
 	}
-	else
-		resetTimer();
 }
 
 void LedgerAcquire::addPeers()
@@ -226,8 +229,6 @@ void LedgerAcquire::trigger(Peer::ref peer, bool timer)
 		tmGL.set_itype(ripple::liBASE);
 		cLog(lsTRACE) << "Sending base request to " << (peer ? "selected peer" : "all peers");
 		sendRequest(tmGL, peer);
-		if (timer)
-			resetTimer();
 		return;
 	}
 
@@ -319,8 +320,6 @@ void LedgerAcquire::trigger(Peer::ref peer, bool timer)
 		cLog(lsDEBUG) << "Done:" << (mComplete ? " complete" : "") << (mFailed ? " failed" : "");
 		done();
 	}
-	else if (timer)
-		resetTimer();
 }
 
 void PeerSet::sendRequest(const ripple::TMGetLedger& tmGL, Peer::ref peer)
@@ -512,7 +511,7 @@ LedgerAcquire::pointer LedgerAcquireMaster::findCreate(const uint256& hash)
 		return ptr;
 	ptr = boost::make_shared<LedgerAcquire>(hash);
 	ptr->addPeers();
-	ptr->resetTimer(); // Cannot call in constructor
+	ptr->setTimer(); // Cannot call in constructor
 	return ptr;
 }
 
diff --git a/src/cpp/ripple/LedgerAcquire.h b/src/cpp/ripple/LedgerAcquire.h
index 68cd27c48..68753afab 100644
--- a/src/cpp/ripple/LedgerAcquire.h
+++ b/src/cpp/ripple/LedgerAcquire.h
@@ -47,10 +47,11 @@ public:
 
 	void peerHas(Peer::ref);
 	void badPeer(Peer::ref);
-	void resetTimer();
+	void setTimer();
 
 	int takePeerSetFrom(const PeerSet& s);
 	int getPeerCount() const;
+	virtual bool isDone() const			{ return mComplete || mFailed; }
 
 protected:
 	virtual void newPeer(Peer::ref) = 0;
diff --git a/src/cpp/ripple/LedgerConsensus.cpp b/src/cpp/ripple/LedgerConsensus.cpp
index c386f62b3..0c174a806 100644
--- a/src/cpp/ripple/LedgerConsensus.cpp
+++ b/src/cpp/ripple/LedgerConsensus.cpp
@@ -67,7 +67,6 @@ void TransactionAcquire::onTimer(bool progress)
 			BOOST_FOREACH(Peer::ref peer, peerList)
 				peerHas(peer);
 		}
-		resetTimer();
 	}
 	else if (!progress)
 		trigger(Peer::pointer(), true);
@@ -111,20 +110,15 @@ void TransactionAcquire::trigger(Peer::ref peer, bool timer)
 			done();
 			return;
 		}
-		else
-		{
-			ripple::TMGetLedger tmGL;
-			tmGL.set_ledgerhash(mHash.begin(), mHash.size());
-			tmGL.set_itype(ripple::liTS_CANDIDATE);
-			if (getTimeouts() != 0)
-				tmGL.set_querytype(ripple::qtINDIRECT);
-			BOOST_FOREACH(SHAMapNode& it, nodeIDs)
-				*(tmGL.add_nodeids()) = it.getRawString();
-			sendRequest(tmGL, peer);
-		}
+		ripple::TMGetLedger tmGL;
+		tmGL.set_ledgerhash(mHash.begin(), mHash.size());
+		tmGL.set_itype(ripple::liTS_CANDIDATE);
+		if (getTimeouts() != 0)
+			tmGL.set_querytype(ripple::qtINDIRECT);
+		BOOST_FOREACH(SHAMapNode& it, nodeIDs)
+			*(tmGL.add_nodeids()) = it.getRawString();
+		sendRequest(tmGL, peer);
 	}
-	if (timer)
-		resetTimer();
 }
 
 SMAddNode TransactionAcquire::takeNodes(const std::list<SHAMapNode>& nodeIDs,
@@ -882,7 +876,7 @@ void LedgerConsensus::startAcquiring(const TransactionAcquire::pointer& acquire)
 			acquire->peerHas(peer);
 	}
 
-	acquire->resetTimer();
+	acquire->setTimer();
 }
 
 void LedgerConsensus::propose()