From 018a030d9707e5c0c07f11ce37eb2b3f6c37680e Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sun, 29 Sep 2013 12:29:30 -0700 Subject: [PATCH] Clean this up and make it handle exceptions a bit more sanely. --- src/ripple_app/ledger/LedgerMaster.cpp | 144 +++++++++++++------------ src/ripple_app/ledger/LedgerMaster.h | 4 +- 2 files changed, 81 insertions(+), 67 deletions(-) diff --git a/src/ripple_app/ledger/LedgerMaster.cpp b/src/ripple_app/ledger/LedgerMaster.cpp index 1b2b7fcb7..aeeab9e6b 100644 --- a/src/ripple_app/ledger/LedgerMaster.cpp +++ b/src/ripple_app/ledger/LedgerMaster.cpp @@ -626,7 +626,6 @@ void LedgerMaster::checkAccept (Ledger::ref ledger) tryAdvance (); } -// Try to publish ledgers, acquire missing ledgers void LedgerMaster::advanceThread() { ScopedLockType sl (mLock, __FILE__, __LINE__); @@ -634,12 +633,28 @@ void LedgerMaster::advanceThread() WriteLog (lsTRACE, LedgerMaster) << "advanceThread<"; + try + { + doAdvance(); + } + catch (...) + { + WriteLog (lsFATAL, LedgerMaster) << "doAdvance throws an exception"; + } + + mAdvanceThread = false; + WriteLog (lsTRACE, LedgerMaster) << "advanceThread>"; +} + +// Try to publish ledgers, acquire missing ledgers +void LedgerMaster::doAdvance () +{ do { mAdvanceWork = false; // If there's work to do, we'll make progress bool progress = false; - std::list pubLedgers = findNewLedgersToPublish (sl); + std::list pubLedgers = findNewLedgersToPublish (); if (pubLedgers.empty()) { if (!getConfig().RUN_STANDALONE && !getApp().getFeeTrack().isLoadedLocal() && @@ -657,73 +672,73 @@ void LedgerMaster::advanceThread() ((mFillInProgress == 0) || (missing > mFillInProgress))) { WriteLog (lsTRACE, LedgerMaster) << "advanceThread should acquire"; - sl.unlock(); - Ledger::pointer nextLedger = mLedgerHistory.getLedgerBySeq(missing + 1); - if (nextLedger) { - assert (nextLedger->getLedgerSeq() == (missing + 1)); - Ledger::pointer ledger = getLedgerByHash(nextLedger->getParentHash()); - if (!ledger) + ScopedUnlockType sl(mLock, __FILE__, __LINE__); + Ledger::pointer nextLedger = mLedgerHistory.getLedgerBySeq(missing + 1); + if (nextLedger) { - if (!getApp().getInboundLedgers().isFailure(nextLedger->getParentHash())) + assert (nextLedger->getLedgerSeq() == (missing + 1)); + Ledger::pointer ledger = getLedgerByHash(nextLedger->getParentHash()); + if (!ledger) { - InboundLedger::pointer acq = - getApp().getInboundLedgers().findCreate(nextLedger->getParentHash(), - nextLedger->getLedgerSeq() - 1, false); - if (acq->isComplete() && !acq->isFailed()) - ledger = acq->getLedger(); - else if ((missing > 40000) && getApp().getOPs().shouldFetchPack(missing)) + if (!getApp().getInboundLedgers().isFailure(nextLedger->getParentHash())) { - WriteLog (lsTRACE, LedgerMaster) << "tryAdvance want fetch pack " << missing; - getFetchPack(nextLedger); + InboundLedger::pointer acq = + getApp().getInboundLedgers().findCreate(nextLedger->getParentHash(), + nextLedger->getLedgerSeq() - 1, false); + if (acq->isComplete() && !acq->isFailed()) + ledger = acq->getLedger(); + else if ((missing > 40000) && getApp().getOPs().shouldFetchPack(missing)) + { + WriteLog (lsTRACE, LedgerMaster) << "tryAdvance want fetch pack " << missing; + getFetchPack(nextLedger); + } + else + WriteLog (lsTRACE, LedgerMaster) << "tryAdvance no fetch pack for " << missing; } else - WriteLog (lsTRACE, LedgerMaster) << "tryAdvance no fetch pack for " << missing; + WriteLog (lsDEBUG, LedgerMaster) << "tryAdvance found failed acquire"; + } + if (ledger) + { + assert(ledger->getLedgerSeq() == missing); + WriteLog (lsTRACE, LedgerMaster) << "tryAdvance acquired " << ledger->getLedgerSeq(); + setFullLedger(ledger, false, false); + if ((mFillInProgress == 0) && (Ledger::getHashByIndex(ledger->getLedgerSeq() - 1) == ledger->getParentHash())) + { // Previous ledger is in DB + ScopedLockType sl(mLock, __FILE__, __LINE__); + mFillInProgress = ledger->getLedgerSeq(); + getApp().getJobQueue().addJob(jtADVANCE, "tryFill", BIND_TYPE (&LedgerMaster::tryFill, this, P_1, ledger)); + } + progress = true; } else - WriteLog (lsDEBUG, LedgerMaster) << "tryAdvance found failed acquire"; - } - if (ledger) - { - assert(ledger->getLedgerSeq() == missing); - WriteLog (lsTRACE, LedgerMaster) << "tryAdvance acquired " << ledger->getLedgerSeq(); - setFullLedger(ledger, false, false); - if ((mFillInProgress == 0) && (Ledger::getHashByIndex(ledger->getLedgerSeq() - 1) == ledger->getParentHash())) - { // Previous ledger is in DB - sl.lock(__FILE__, __LINE__); - mFillInProgress = ledger->getLedgerSeq(); - getApp().getJobQueue().addJob(jtADVANCE, "tryFill", BIND_TYPE (&LedgerMaster::tryFill, this, P_1, ledger)); - sl.unlock(); + { + try + { + for (int i = 0; i < getConfig().getSize(siLedgerFetch); ++i) + { + uint32 seq = missing - i; + uint256 hash = nextLedger->getLedgerHash(seq); + if (hash.isNonZero()) + getApp().getInboundLedgers().findCreate(hash, seq, false); + } + } + catch (...) + { + WriteLog (lsWARNING, LedgerMaster) << "Threw while prefecthing"; + } } - progress = true; } else { - try - { - for (int i = 0; i < getConfig().getSize(siLedgerFetch); ++i) - { - uint32 seq = missing - i; - uint256 hash = nextLedger->getLedgerHash(seq); - if (hash.isNonZero()) - getApp().getInboundLedgers().findCreate(hash, seq, false); - } - } - catch (...) - { - WriteLog (lsWARNING, LedgerMaster) << "Threw while prefecthing"; - } + WriteLog (lsFATAL, LedgerMaster) << "Unable to find ledger following prevMissing " << missing; + WriteLog (lsFATAL, LedgerMaster) << "Pub:" << mPubLedger->getLedgerSeq() << " Val:" << mValidLedger->getLedgerSeq(); + WriteLog (lsFATAL, LedgerMaster) << "Ledgers: " << getApp().getLedgerMaster().getCompleteLedgers(); + clearLedger (missing + 1); + progress = true; } } - else - { - WriteLog (lsFATAL, LedgerMaster) << "Unable to find ledger following prevMissing " << missing; - WriteLog (lsFATAL, LedgerMaster) << "Pub:" << mPubLedger->getLedgerSeq() << " Val:" << mValidLedger->getLedgerSeq(); - WriteLog (lsFATAL, LedgerMaster) << "Ledgers: " << getApp().getLedgerMaster().getCompleteLedgers(); - clearLedger (missing + 1); - progress = true; - } - sl.lock(__FILE__, __LINE__); if (mValidLedger->getLedgerSeq() != mPubLedger->getLedgerSeq()) { WriteLog (lsDEBUG, LedgerMaster) << "tryAdvance found last valid changed"; @@ -739,13 +754,14 @@ void LedgerMaster::advanceThread() WriteLog (lsTRACE, LedgerMaster) << "tryAdvance found " << pubLedgers.size() << " ledgers to publish"; BOOST_FOREACH(Ledger::ref ledger, pubLedgers) { - sl.unlock(); - WriteLog(lsDEBUG, LedgerMaster) << "tryAdvance publishing seq " << ledger->getLedgerSeq(); + { + ScopedUnlockType sul (mLock, __FILE__, __LINE__); + WriteLog(lsDEBUG, LedgerMaster) << "tryAdvance publishing seq " << ledger->getLedgerSeq(); - setFullLedger(ledger, true, true); - getApp().getOPs().pubLedger(ledger); + setFullLedger(ledger, true, true); + getApp().getOPs().pubLedger(ledger); + } - sl.lock(__FILE__, __LINE__); setPubLedger(ledger); progress = true; } @@ -762,12 +778,9 @@ void LedgerMaster::advanceThread() if (progress) mAdvanceWork = true; } while (mAdvanceWork); - - mAdvanceThread = false; - WriteLog (lsTRACE, LedgerMaster) << "advanceThread>"; } -std::list LedgerMaster::findNewLedgersToPublish(ScopedLockType& sl) +std::list LedgerMaster::findNewLedgersToPublish () { std::list ret; @@ -792,7 +805,7 @@ std::list LedgerMaster::findNewLedgersToPublish(ScopedLockType& uint32 valSeq = mValidLedger->getLedgerSeq(); Ledger::pointer valLedger = mValidLedger; - sl.unlock(); + ScopedUnlockType sul(mLock, __FILE__, __LINE__); try { for (uint32 seq = pubSeq; seq <= valSeq; ++seq) @@ -857,7 +870,6 @@ std::list LedgerMaster::findNewLedgersToPublish(ScopedLockType& { WriteLog (lsERROR, LedgerMaster) << "findNewLedgersToPublish catches an exception"; } - sl.lock(__FILE__, __LINE__); } WriteLog (lsTRACE, LedgerMaster) << "findNewLedgersToPublish> " << ret.size(); diff --git a/src/ripple_app/ledger/LedgerMaster.h b/src/ripple_app/ledger/LedgerMaster.h index e4d27ad16..13e197f2f 100644 --- a/src/ripple_app/ledger/LedgerMaster.h +++ b/src/ripple_app/ledger/LedgerMaster.h @@ -38,6 +38,7 @@ public: public: typedef RippleRecursiveMutex LockType; typedef LockType::ScopedLockType ScopedLockType; + typedef LockType::ScopedUnlockType ScopedUnlockType; explicit LedgerMaster (Stoppable& parent) : Stoppable ("LedgerMaster", parent) @@ -217,7 +218,7 @@ public: static bool shouldAcquire (uint32 currentLedgerID, uint32 ledgerHistory, uint32 targetLedger); private: - std::list findNewLedgersToPublish(ScopedLockType& sl); + std::list findNewLedgersToPublish (); void applyFutureTransactions (uint32 ledgerIndex); bool isValidTransaction (Transaction::ref trans); @@ -226,6 +227,7 @@ private: void getFetchPack (Ledger::ref have); void tryFill (Job&, Ledger::pointer); void advanceThread (); + void doAdvance (); void updatePaths (Job&); void setValidLedger(Ledger::ref);