From 1d482eeecb3c568e748940c19b5d9967320aa091 Mon Sep 17 00:00:00 2001 From: Scott Schurr Date: Tue, 14 Feb 2017 14:02:59 -0800 Subject: [PATCH] Prevent DatabaseRotateImp crash on shutdown (RIPD-1392): The DatabaseImp holds threads that access DatabaseRotateImp. But the DatabaseRotateImp's destructor runs before the DatabaseImp destructor. The DatabaseRotateImp now assures that the DatabaseImp threads are stopped before the DatabaseRotateImp destructor completes. --- src/ripple/nodestore/impl/DatabaseImp.h | 35 +++++++++++++------ .../nodestore/impl/DatabaseRotatingImp.h | 6 ++++ 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/src/ripple/nodestore/impl/DatabaseImp.h b/src/ripple/nodestore/impl/DatabaseImp.h index 0fc6375407..ac4052dd8c 100644 --- a/src/ripple/nodestore/impl/DatabaseImp.h +++ b/src/ripple/nodestore/impl/DatabaseImp.h @@ -92,17 +92,15 @@ public: fdlimit_ = m_backend->fdlimit(); } - ~DatabaseImp () + ~DatabaseImp () override { - { - std::lock_guard lock (m_readLock); - m_readShut = true; - m_readCondVar.notify_all (); - m_readGenCondVar.notify_all (); - } - - for (auto& e : m_readThreads) - e.join(); + // NOTE! + // Any derived class should call the stopThreads() method in its + // destructor. Otherwise, occasionally, the derived class may + // crash during shutdown when its members are accessed by one of + // these threads after the derived class is destroyed but before + // this base class is destroyed. + stopThreads(); } std::string @@ -442,6 +440,23 @@ public: return fdlimit_; } +protected: + void stopThreads () + { + { + std::lock_guard lock (m_readLock); + if (m_readShut) // Only stop threads once. + return; + + m_readShut = true; + m_readCondVar.notify_all (); + m_readGenCondVar.notify_all (); + } + + for (auto& e : m_readThreads) + e.join(); + } + private: std::atomic m_storeCount; std::atomic m_fetchTotalCount; diff --git a/src/ripple/nodestore/impl/DatabaseRotatingImp.h b/src/ripple/nodestore/impl/DatabaseRotatingImp.h index a2645f3807..44532a1c50 100644 --- a/src/ripple/nodestore/impl/DatabaseRotatingImp.h +++ b/src/ripple/nodestore/impl/DatabaseRotatingImp.h @@ -63,6 +63,12 @@ public: , archiveBackend_ (archiveBackend) {} + ~DatabaseRotatingImp () override + { + // Stop threads before data members are destroyed. + DatabaseImp::stopThreads (); + } + std::shared_ptr const& getWritableBackend() const override { std::lock_guard lock (rotateMutex_);