Fix deadlock in InboundLedgers and NetworkOPs

include/xrpl/basics/scope.h

@@ -21,6 +21,7 @@
 #define RIPPLE_BASICS_SCOPE_H_INCLUDED
 
 #include <exception>
+#include <mutex>
 #include <type_traits>
 #include <utility>
 
@@ -186,6 +187,70 @@ class scope_success
 template <class EF>
 scope_success(EF) -> scope_success<EF>;
 
+/**
+    Automatically unlocks and re-locks a unique_lock object.
+
+    This is the reverse of a std::unique_lock object - instead of locking the
+   mutex for the lifetime of this object, it unlocks it.
+
+    Make sure you don't try to unlock mutexes that aren't actually locked!
+
+    This is essentially a less-versatile boost::reverse_lock.
+
+    e.g. @code
+
+    std::mutex mut;
+
+    for (;;)
+    {
+        std::unique_lock myScopedLock{mut};
+        // mut is now locked
+
+        ... do some stuff with it locked ..
+
+        while (xyz)
+        {
+            ... do some stuff with it locked ..
+
+            ScopedUnlock unlocker{myScopedLock};
+
+            // mut is now unlocked for the remainder of this block,
+            // and re-locked at the end.
+
+            ...do some stuff with it unlocked ...
+        }  // mut gets locked here.
+
+    }  // mut gets unlocked here
+    @endcode
+*/
+
+template <class Mutex>
+class scope_unlock
+{
+    std::unique_lock<Mutex>* plock;
+
+public:
+    explicit scope_unlock(std::unique_lock<Mutex>& lock) noexcept(true)
+        : plock(&lock)
+    {
+        assert(plock->owns_lock());
+        plock->unlock();
+    }
+
+    // Immovable type
+    scope_unlock(scope_unlock const&) = delete;
+    scope_unlock&
+    operator=(scope_unlock const&) = delete;
+
+    ~scope_unlock() noexcept(true)
+    {
+        plock->lock();
+    }
+};
+
+template <class Mutex>
+scope_unlock(std::unique_lock<Mutex>&) -> scope_unlock<Mutex>;
+
 }  // namespace ripple
 
 #endif

src/xrpld/app/ledger/detail/InboundLedgers.cpp

@@ -25,9 +25,11 @@
 #include <xrpld/perflog/PerfLog.h>
 #include <xrpl/basics/DecayingSample.h>
 #include <xrpl/basics/Log.h>
+#include <xrpl/basics/scope.h>
 #include <xrpl/beast/container/aged_map.h>
 #include <xrpl/beast/core/LexicalCast.h>
 #include <xrpl/protocol/jss.h>
+
 #include <exception>
 #include <memory>
 #include <mutex>
@@ -139,7 +141,7 @@ class InboundLedgersImp : public InboundLedgers
             if (pendingAcquires_.contains(hash))
                 return;
             pendingAcquires_.insert(hash);
-            lock.unlock();
+            scope_unlock unlock(lock);
             acquire(hash, seq, reason);
         }
         catch (std::exception const& e)

src/xrpld/app/misc/NetworkOPs.cpp

@@ -52,6 +52,7 @@
 #include <xrpl/basics/UptimeClock.h>
 #include <xrpl/basics/mulDiv.h>
 #include <xrpl/basics/safe_cast.h>
+#include <xrpl/basics/scope.h>
 #include <xrpl/beast/rfc2616.h>
 #include <xrpl/beast/utility/rngfill.h>
 #include <xrpl/crypto/RFC1751.h>
@@ -2309,7 +2310,7 @@ NetworkOPsImp::recvValidation(
             bypassAccept = BypassAccept::yes;
         else
             pendingValidations_.insert(val->getLedgerHash());
-        lock.unlock();
+        scope_unlock unlock(lock);
         handleNewValidation(app_, val, source, bypassAccept, m_journal);
     }
     catch (std::exception const& e)
@@ -2326,10 +2327,9 @@ NetworkOPsImp::recvValidation(
     }
     if (bypassAccept == BypassAccept::no)
     {
-        lock.lock();
         pendingValidations_.erase(val->getLedgerHash());
-        lock.unlock();
     }
+    lock.unlock();
 
     pubValidation(val);