From 0b7b61ee7b364bfbbe3e3bd739ae183528adde46 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 9 Nov 2012 09:46:38 -0800 Subject: [PATCH 1/6] Remove a FIXME since the issue was already fixed. --- src/cpp/ripple/Peer.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/cpp/ripple/Peer.cpp b/src/cpp/ripple/Peer.cpp index d4167994ba..a5f34665f6 100644 --- a/src/cpp/ripple/Peer.cpp +++ b/src/cpp/ripple/Peer.cpp @@ -893,8 +893,6 @@ void Peer::recvPropose(const boost::shared_ptr& packet) void Peer::recvHaveTxSet(ripple::TMHaveTransactionSet& packet) { - // FIXME: We should have some limit on the number of HaveTxSet messages a peer can send us - // per consensus pass, to keep a peer from running up our memory without limit uint256 hashes; if (packet.hash().size() != (256 / 8)) { From c32711c4720a3e7558c83afa7a4ae6aeb1847f28 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 9 Nov 2012 14:12:37 -0800 Subject: [PATCH 2/6] Mark two FIXMEs. --- src/cpp/ripple/TransactionAction.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cpp/ripple/TransactionAction.cpp b/src/cpp/ripple/TransactionAction.cpp index 8b30023167..f7d26eeee5 100644 --- a/src/cpp/ripple/TransactionAction.cpp +++ b/src/cpp/ripple/TransactionAction.cpp @@ -34,6 +34,7 @@ TER TransactionEngine::setAuthorized(const SerializedTransaction& txn, bool bMus std::vector vucSignature = txn.getFieldVL(sfSignature); RippleAddress naAccountPublic = RippleAddress::createAccountPublic(vucPubKey); + // FIXME: This should be moved to the transaction's signature check and cached if (!naAccountPublic.accountPublicVerify(Serializer::getSHA512Half(vucCipher), vucSignature)) { Log(lsWARNING) << "createGenerator: bad signature unauthorized generator claim"; @@ -614,6 +615,7 @@ TER TransactionEngine::doWalletAdd(const SerializedTransaction& txn) const RippleAddress naMasterPubKey = RippleAddress::createAccountPublic(vucPubKey); const uint160 uDstAccountID = naMasterPubKey.getAccountID(); + // FIXME: This should be moved to the transaction's signature check logic and cached if (!naMasterPubKey.accountPublicVerify(Serializer::getSHA512Half(uAuthKeyID.begin(), uAuthKeyID.size()), vucSignature)) { std::cerr << "WalletAdd: unauthorized: bad signature " << std::endl; From 546c0f288f4ed886d5e517194c76fbec484f2133 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 9 Nov 2012 14:12:48 -0800 Subject: [PATCH 3/6] Add a job type for local transactions. --- src/cpp/ripple/JobQueue.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cpp/ripple/JobQueue.h b/src/cpp/ripple/JobQueue.h index ddcfa10172..26a72dca23 100644 --- a/src/cpp/ripple/JobQueue.h +++ b/src/cpp/ripple/JobQueue.h @@ -21,6 +21,7 @@ enum JobType jtTRANSACTION, jtPROPOSAL_ut, jtVALIDATION_t, + jtTRANSACTION_l, jtPROPOSAL_t, jtADMIN, jtDEATH, // job of death, used internally From 82d26ea7563712ac0b7f1c626dd878e33e35c4b6 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 9 Nov 2012 14:13:03 -0800 Subject: [PATCH 4/6] Add a scoped unlock class. --- src/cpp/ripple/ScopedLock.h | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/cpp/ripple/ScopedLock.h b/src/cpp/ripple/ScopedLock.h index 1fd1ee6082..5f85b83ed3 100644 --- a/src/cpp/ripple/ScopedLock.h +++ b/src/cpp/ripple/ScopedLock.h @@ -16,16 +16,26 @@ protected: public: ScopedLock(boost::recursive_mutex& mutex) : - mHolder(boost::make_shared(boost::ref(mutex))) - { ; } - void lock() const - { - mHolder->lock(); - } - void unlock() const - { - mHolder->unlock(); - } + mHolder(boost::make_shared(boost::ref(mutex))) { ; } + + void lock() const { mHolder->lock(); } + void unlock() const { mHolder->unlock(); } +}; + +// A class that unlocks on construction and locks on destruction + +class ScopedUnlock +{ +protected: + boost::recursive_mutex& mMutex; + +public: + ScopedUnlock(boost::recursive_mutex& mutex) : mMutex(mutex) { mMutex.unlock(); } + ~ScopedUnlock() { mMutex.lock(); } + +private: + ScopedUnlock(const ScopedUnlock&); // no implementation + ScopedUnlock& operator=(const ScopedUnlock&); // no implementation }; #endif From 40527cca2b53656e14f3f10a5ac4033a43d1bcce Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 9 Nov 2012 14:14:47 -0800 Subject: [PATCH 5/6] Start adding support for concurrent I/O. --- src/cpp/ripple/Application.h | 3 +++ src/cpp/ripple/LedgerConsensus.cpp | 1 + src/cpp/ripple/NetworkOPs.cpp | 5 ++++- src/cpp/ripple/Peer.cpp | 4 ++++ src/cpp/ripple/RPCHandler.cpp | 4 +++- 5 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/cpp/ripple/Application.h b/src/cpp/ripple/Application.h index a100190966..3e68a8b14a 100644 --- a/src/cpp/ripple/Application.h +++ b/src/cpp/ripple/Application.h @@ -43,6 +43,8 @@ class Application boost::asio::io_service mIOService, mAuxService; boost::asio::io_service::work mIOWork, mAuxWork; + boost::recursive_mutex mMasterLock; + Wallet mWallet; UniqueNodeList mUNL; LedgerMaster mMasterLedger; @@ -99,6 +101,7 @@ public: JobQueue& getJobQueue() { return mJobQueue; } SuppressionTable& getSuppression() { return mSuppressions; } RPCHandler& getRPCHandler() { return mRPCHandler; } + boost::recursive_mutex& getMasterLock() { return mMasterLock; } bool isNew(const uint256& s) { return mSuppressions.addSuppression(s); } diff --git a/src/cpp/ripple/LedgerConsensus.cpp b/src/cpp/ripple/LedgerConsensus.cpp index 0fc07a4aad..9f9e099454 100644 --- a/src/cpp/ripple/LedgerConsensus.cpp +++ b/src/cpp/ripple/LedgerConsensus.cpp @@ -1141,6 +1141,7 @@ uint32 LedgerConsensus::roundCloseTime(uint32 closeTime) void LedgerConsensus::accept(SHAMap::ref set) { + boost::recursive_mutex::scoped_lock masterLock(theApp->getMasterLock()); assert(set->getHash() == mOurPosition->getCurrentHash()); uint32 closeTime = roundCloseTime(mOurPosition->getCloseTime()); diff --git a/src/cpp/ripple/NetworkOPs.cpp b/src/cpp/ripple/NetworkOPs.cpp index 44259523f6..882551cc81 100644 --- a/src/cpp/ripple/NetworkOPs.cpp +++ b/src/cpp/ripple/NetworkOPs.cpp @@ -161,7 +161,6 @@ Transaction::pointer NetworkOPs::submitTransactionSync(const Transaction::pointe Transaction::pointer NetworkOPs::processTransaction(Transaction::pointer trans, stCallback callback) { - Transaction::pointer dbtx = theApp->getMasterTransaction().fetch(trans->getID(), true); int newFlags = theApp->getSuppression().getFlags(trans->getID()); if ((newFlags & SF_BAD) != 0) @@ -182,6 +181,8 @@ Transaction::pointer NetworkOPs::processTransaction(Transaction::pointer trans, theApp->isNewFlag(trans->getID(), SF_SIGGOOD); } + boost::recursive_mutex::scoped_lock sl(theApp->getMasterLock()); + Transaction::pointer dbtx = theApp->getMasterTransaction().fetch(trans->getID(), true); TER r = mLedgerMaster->doTransaction(*trans->getSTransaction(), tapOPEN_LEDGER | tapNO_CHECK_SIGN); trans->setResult(r); @@ -762,6 +763,8 @@ uint256 NetworkOPs::getConsensusLCL() void NetworkOPs::processTrustedProposal(LedgerProposal::pointer proposal, boost::shared_ptr set, RippleAddress nodePublic, uint256 checkLedger, bool sigGood) { + boost::recursive_mutex::scoped_lock sl(theApp->getMasterLock()); + bool relay = true; if (!haveConsensusObject()) diff --git a/src/cpp/ripple/Peer.cpp b/src/cpp/ripple/Peer.cpp index a5f34665f6..f364df838b 100644 --- a/src/cpp/ripple/Peer.cpp +++ b/src/cpp/ripple/Peer.cpp @@ -41,6 +41,8 @@ void Peer::handle_write(const boost::system::error_code& error, size_t bytes_tra // std::cerr << "Peer::handle_write bytes: "<< bytes_transferred << std::endl; #endif + boost::recursive_mutex::scoped_lock sl(theApp->getMasterLock()); + mSendingPacket.reset(); if (mDetaching) @@ -357,6 +359,7 @@ void Peer::handle_read_body(const boost::system::error_code& error) else { cLog(lsINFO) << "Peer: Body: Error: " << ADDRESS(this) << ": " << error.category().name() << ": " << error.message() << ": " << error; + boost::recursive_mutex::scoped_lock sl(theApp->getMasterLock()); detach("hrb"); } } @@ -371,6 +374,7 @@ void Peer::processReadBuffer() // std::cerr << "Peer::processReadBuffer: " << mIpPort.first << " " << mIpPort.second << std::endl; // If connected and get a mtHELLO or if not connected and get a non-mtHELLO, wrong message was sent. + boost::recursive_mutex::scoped_lock sl(theApp->getMasterLock()); if (mHelloed == (type == ripple::mtHELLO)) { cLog(lsWARNING) << "Wrong message type: " << type; diff --git a/src/cpp/ripple/RPCHandler.cpp b/src/cpp/ripple/RPCHandler.cpp index bd70776c40..fbd24d555f 100644 --- a/src/cpp/ripple/RPCHandler.cpp +++ b/src/cpp/ripple/RPCHandler.cpp @@ -1384,7 +1384,9 @@ Json::Value RPCHandler::doCommand(const std::string& command, Json::Value& param return rpcError(rpcNO_NETWORK); } // XXX Should verify we have a current ledger. - else if ((commandsA[i].iOptions & optCurrent) && false) + + boost::recursive_mutex::scoped_lock sl(theApp->getMasterLock()); + if ((commandsA[i].iOptions & optCurrent) && false) { return rpcError(rpcNO_CURRENT); } From b2ef5de5f7e6b1f1d946ef10d454c1edfc8b4482 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Fri, 9 Nov 2012 14:15:48 -0800 Subject: [PATCH 6/6] Cosmetic change. --- src/cpp/ripple/LedgerAcquire.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/ripple/LedgerAcquire.cpp b/src/cpp/ripple/LedgerAcquire.cpp index b4e5b3a30b..0d979f61d8 100644 --- a/src/cpp/ripple/LedgerAcquire.cpp +++ b/src/cpp/ripple/LedgerAcquire.cpp @@ -541,7 +541,7 @@ bool LedgerAcquireMaster::gotLedgerData(ripple::TMLedgerData& packet, Peer::ref return false; } memcpy(hash.begin(), packet.ledgerhash().data(), 32); - cLog(lsTRACE) << "Got data ( " << packet.nodes().size() << ") for acquiring ledger: " << hash; + cLog(lsTRACE) << "Got data (" << packet.nodes().size() << ") for acquiring ledger: " << hash; LedgerAcquire::pointer ledger = find(hash); if (!ledger)