Improve SHAMap missing node behavior:

* Prevent recursive invocation of missing node handler
* Do not throw from ledger constructor

src/ripple/app/ledger/Ledger.cpp

@@ -240,7 +240,15 @@ Ledger::Ledger (uint256 const& parentHash,
 
     txMap_->setImmutable ();
     stateMap_->setImmutable ();
-    setup(config);
+
+    if (! setup(config))
+        loaded = false;
+
+    if (! loaded)
+    {
+        updateHash ();
+        getApp().family().missing_node (info_.hash);
+    }
 }
 
 // Create a new ledger that's a snapshot of this one
@@ -682,31 +690,61 @@ Ledger::rawTxInsert (uint256 const& key,
     }
 }
 
-void
+bool
 Ledger::setup (Config const& config)
 {
+    bool ret = true;
+
     fees_.base = config.FEE_DEFAULT;
     fees_.units = config.TRANSACTION_FEE_BASE;
     fees_.reserve = config.FEE_ACCOUNT_RESERVE;
     fees_.increment = config.FEE_OWNER_RESERVE;
-    auto const sle = read(keylet::fees());
+
-    if (sle)
+    try
     {
-        // VFALCO NOTE Why getFieldIndex and not isFieldPresent?
+        auto const sle = read(keylet::fees());
 
-        if (sle->getFieldIndex (sfBaseFee) != -1)
+        if (sle)
-            fees_.base = sle->getFieldU64 (sfBaseFee);
+        {
+            // VFALCO NOTE Why getFieldIndex and not isFieldPresent?
 
-        if (sle->getFieldIndex (sfReferenceFeeUnits) != -1)
+            if (sle->getFieldIndex (sfBaseFee) != -1)
-            fees_.units = sle->getFieldU32 (sfReferenceFeeUnits);
+                fees_.base = sle->getFieldU64 (sfBaseFee);
 
-        if (sle->getFieldIndex (sfReserveBase) != -1)
+            if (sle->getFieldIndex (sfReferenceFeeUnits) != -1)
-            fees_.reserve = sle->getFieldU32 (sfReserveBase);
+                fees_.units = sle->getFieldU32 (sfReferenceFeeUnits);
 
-        if (sle->getFieldIndex (sfReserveIncrement) != -1)
+            if (sle->getFieldIndex (sfReserveBase) != -1)
-            fees_.increment = sle->getFieldU32 (sfReserveIncrement);
+                fees_.reserve = sle->getFieldU32 (sfReserveBase);
+
+            if (sle->getFieldIndex (sfReserveIncrement) != -1)
+                fees_.increment = sle->getFieldU32 (sfReserveIncrement);
+        }
     }
-    rules_ = Rules(*this);
+    catch (SHAMapMissingNode &)
+    {
+        ret = false;
+    }
+    catch (...)
+    {
+        throw;
+    }
+
+
+    try
+    {
+        rules_ = Rules(*this);
+    }
+    catch (SHAMapMissingNode &)
+    {
+        ret = false;
+    }
+    catch (...)
+    {
+        throw;
+    }
+
+    return ret;
 }
 
 std::shared_ptr<SLE>

src/ripple/app/ledger/Ledger.h

@@ -367,7 +367,9 @@ private:
     class sles_iter_impl;
     class txs_iter_impl;
 
-    void
+    bool saveValidatedLedger (bool current);
+
+    bool
     setup (Config const& config);
 
     std::shared_ptr<SLE>

src/ripple/app/ledger/LedgerConsensus.h

@@ -95,6 +95,7 @@ void applyTransactions (
   @return                       resultSuccess, resultFail or resultRetry
 */
 int applyTransaction (
+    Application& app,
     OpenView& view,
     std::shared_ptr<STTx const> const& txn,
     bool retryAssured,

src/ripple/app/ledger/impl/LedgerConsensusImp.cpp

@@ -1035,7 +1035,7 @@ void LedgerConsensusImp::accept (std::shared_ptr<SHAMap> set)
         {
             // Special case, we are replaying a ledger close
             for (auto& tx : replay->txns_)
-                applyTransaction (accum, tx.second, false, tapNO_CHECK_SIGN);
+                applyTransaction (app_, accum, tx.second, false, tapNO_CHECK_SIGN);
         }
         else
         {

src/ripple/app/main/Application.cpp

@@ -118,6 +118,10 @@ private:
     FullBelowCache fullbelow_;
     NodeStore::Database& db_;
 
+    // missing node handler
+    std::uint32_t maxSeq = 0;
+    std::mutex maxSeqLock;
+
 public:
     AppFamily (AppFamily const&) = delete;
     AppFamily& operator= (AppFamily const&) = delete;
@@ -173,18 +177,48 @@ public:
     void
     missing_node (std::uint32_t seq) override
     {
-        uint256 const hash = app_.getLedgerMaster ().getHashBySeq (seq);
+        WriteLog (lsERROR, Ledger) << "Missing node in " << seq;
-        if (hash.isZero())
+
-            app_.getInboundLedgers ().acquire (
+        // prevent recursive invocation
-                hash, seq, InboundLedger::fcGENERIC);
+        std::unique_lock <std::mutex> lock (maxSeqLock);
+
+        if (maxSeq == 0)
+        {
+            maxSeq = seq;
+
+            do
+            {
+                // Try to acquire the most recent missing ledger
+                seq = maxSeq;
+
+                lock.unlock();
+
+                // This can invoke the missing node handler
+                uint256 hash = app_.getLedgerMaster().getHashBySeq (seq);
+
+                if (hash.isNonZero())
+                    app_.getInboundLedgers().acquire (
+                        hash, seq, InboundLedger::fcGENERIC);
+
+                lock.lock();
+            }
+            while (maxSeq != seq);
+        }
+        else if (maxSeq < seq)
+        {
+            // We found a more recent ledger with a missing node
+            maxSeq = seq;
+        }
     }
 
-    virtual
     void
     missing_node (uint256 const& hash) override
     {
         if (hash.isNonZero())
         {
+            WriteLog (lsERROR, Ledger) << "Missing node in "
+                << to_string (hash);
+
             app_.getInboundLedgers ().acquire (
                 hash, 0, InboundLedger::fcGENERIC);
         }
@@ -1239,6 +1273,8 @@ bool ApplicationImp::loadOldLedger (
                          }
 
                          loadLedger->setClosed ();
+                         loadLedger->stateMap().flushDirty
+                             (hotACCOUNT_NODE, loadLedger->info().seq);
                          loadLedger->setAccepted (closeTime,
                              closeTimeResolution, ! closeTimeEstimated);
                      }

src/ripple/shamap/Family.h

@@ -62,7 +62,7 @@ public:
 
     virtual
     void
-    missing_node (uint256 const& hash) = 0;
+    missing_node (uint256 const& refHash) = 0;
 };
 
 } // ripple

src/ripple/shamap/tests/common.h

@@ -104,9 +104,9 @@ public:
     {
         throw std::runtime_error("missing node");
     }
-    
+
     void
-    missing_node (uint256 const& hash) override
+    missing_node (uint256 const& refHash) override
     {
         throw std::runtime_error("missing node");
     }