From 04aa9867f5ceb9a0c4f89df0b96e258841226f7b Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Tue, 23 Jun 2015 13:35:07 +0200 Subject: [PATCH 1/3] Potential fix for EthashAux race condition Some EthashAux functions are protected by mutexes. These mutexes are normal members of the EthashAux class which itself is a singleton. It can happen that multiple threads will get inside the singleton creation `get()` functions when requesting a lock, but the lock will not be acquired due to the singleton not yet existing. This leads to multiple threads going in the same code and causing double free corruption. Making the mutexes static members of the singleton should hopefully address the issue --- libethcore/EthashAux.cpp | 17 ++++++++++------- libethcore/EthashAux.h | 6 +++--- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/libethcore/EthashAux.cpp b/libethcore/EthashAux.cpp index 51a606ff8..491fe2b0f 100644 --- a/libethcore/EthashAux.cpp +++ b/libethcore/EthashAux.cpp @@ -44,6 +44,9 @@ using namespace eth; const char* DAGChannel::name() { return EthGreen "DAG"; } EthashAux* dev::eth::EthashAux::s_this = nullptr; +SharedMutex dev::eth::EthashAux::x_lights; +Mutex dev::eth::EthashAux::x_epochs; +Mutex dev::eth::EthashAux::x_fulls; EthashAux::~EthashAux() { @@ -62,7 +65,7 @@ uint64_t EthashAux::dataSize(uint64_t _blockNumber) h256 EthashAux::seedHash(unsigned _number) { unsigned epoch = _number / ETHASH_EPOCH_LENGTH; - Guard l(get()->x_epochs); + Guard l(x_epochs); if (epoch >= get()->m_seedHashes.size()) { h256 ret; @@ -85,7 +88,7 @@ h256 EthashAux::seedHash(unsigned _number) uint64_t EthashAux::number(h256 const& _seedHash) { - Guard l(get()->x_epochs); + Guard l(x_epochs); unsigned epoch = 0; auto epochIter = get()->m_epochs.find(_seedHash); if (epochIter == get()->m_epochs.end()) @@ -112,7 +115,7 @@ void EthashAux::killCache(h256 const& _s) EthashAux::LightType EthashAux::light(h256 const& _seedHash) { - ReadGuard l(get()->x_lights); + ReadGuard l(x_lights); LightType ret = get()->m_lights[_seedHash]; return ret ? ret : (get()->m_lights[_seedHash] = make_shared(_seedHash)); } @@ -170,7 +173,7 @@ EthashAux::FullType EthashAux::full(h256 const& _seedHash, bool _createIfMissing FullType ret; auto l = light(_seedHash); - DEV_GUARDED(get()->x_fulls) + DEV_GUARDED(x_fulls) if ((ret = get()->m_fulls[_seedHash].lock())) { get()->m_lastUsedFull = ret; @@ -184,7 +187,7 @@ EthashAux::FullType EthashAux::full(h256 const& _seedHash, bool _createIfMissing ret = make_shared(l->light, dagCallbackShim); // cnote << "Done loading."; - DEV_GUARDED(get()->x_fulls) + DEV_GUARDED(x_fulls) get()->m_fulls[_seedHash] = get()->m_lastUsedFull = ret; } @@ -195,7 +198,7 @@ EthashAux::FullType EthashAux::full(h256 const& _seedHash, bool _createIfMissing unsigned EthashAux::computeFull(h256 const& _seedHash, bool _createIfMissing) { - Guard l(get()->x_fulls); + Guard l(x_fulls); uint64_t blockNumber; DEV_IF_THROWS(blockNumber = EthashAux::number(_seedHash)) @@ -248,7 +251,7 @@ Ethash::Result EthashAux::eval(BlockInfo const& _header, Nonce const& _nonce) Ethash::Result EthashAux::eval(h256 const& _seedHash, h256 const& _headerHash, Nonce const& _nonce) { - DEV_GUARDED(get()->x_fulls) + DEV_GUARDED(x_fulls) if (FullType dag = get()->m_fulls[_seedHash].lock()) return dag->compute(_headerHash, _nonce); DEV_IF_THROWS(return EthashAux::get()->light(_seedHash)->compute(_headerHash, _nonce)) diff --git a/libethcore/EthashAux.h b/libethcore/EthashAux.h index 47180bfd2..11485b733 100644 --- a/libethcore/EthashAux.h +++ b/libethcore/EthashAux.h @@ -91,10 +91,10 @@ private: static EthashAux* s_this; - SharedMutex x_lights; + static SharedMutex x_lights; std::unordered_map> m_lights; - Mutex x_fulls; + static Mutex x_fulls; std::condition_variable m_fullsChanged; std::unordered_map> m_fulls; FullType m_lastUsedFull; @@ -102,7 +102,7 @@ private: uint64_t m_generatingFullNumber = NotGenerating; unsigned m_fullProgress; - Mutex x_epochs; + static Mutex x_epochs; std::unordered_map m_epochs; h256s m_seedHashes; }; From a976df8c3be977bd99dda0b12655181d183ac436 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Tue, 23 Jun 2015 13:58:13 +0200 Subject: [PATCH 2/3] Reduce number of statics in EthashAux --- libethcore/EthashAux.cpp | 18 ++++++++---------- libethcore/EthashAux.h | 9 +++++---- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/libethcore/EthashAux.cpp b/libethcore/EthashAux.cpp index 491fe2b0f..4601e41e3 100644 --- a/libethcore/EthashAux.cpp +++ b/libethcore/EthashAux.cpp @@ -43,10 +43,8 @@ using namespace eth; const char* DAGChannel::name() { return EthGreen "DAG"; } +Mutex dev::eth::EthashAux::x_this; EthashAux* dev::eth::EthashAux::s_this = nullptr; -SharedMutex dev::eth::EthashAux::x_lights; -Mutex dev::eth::EthashAux::x_epochs; -Mutex dev::eth::EthashAux::x_fulls; EthashAux::~EthashAux() { @@ -65,7 +63,7 @@ uint64_t EthashAux::dataSize(uint64_t _blockNumber) h256 EthashAux::seedHash(unsigned _number) { unsigned epoch = _number / ETHASH_EPOCH_LENGTH; - Guard l(x_epochs); + Guard l(get()->x_epochs); if (epoch >= get()->m_seedHashes.size()) { h256 ret; @@ -88,7 +86,7 @@ h256 EthashAux::seedHash(unsigned _number) uint64_t EthashAux::number(h256 const& _seedHash) { - Guard l(x_epochs); + Guard l(get()->x_epochs); unsigned epoch = 0; auto epochIter = get()->m_epochs.find(_seedHash); if (epochIter == get()->m_epochs.end()) @@ -115,7 +113,7 @@ void EthashAux::killCache(h256 const& _s) EthashAux::LightType EthashAux::light(h256 const& _seedHash) { - ReadGuard l(x_lights); + ReadGuard l(get()->x_lights); LightType ret = get()->m_lights[_seedHash]; return ret ? ret : (get()->m_lights[_seedHash] = make_shared(_seedHash)); } @@ -173,7 +171,7 @@ EthashAux::FullType EthashAux::full(h256 const& _seedHash, bool _createIfMissing FullType ret; auto l = light(_seedHash); - DEV_GUARDED(x_fulls) + DEV_GUARDED(get()->x_fulls) if ((ret = get()->m_fulls[_seedHash].lock())) { get()->m_lastUsedFull = ret; @@ -187,7 +185,7 @@ EthashAux::FullType EthashAux::full(h256 const& _seedHash, bool _createIfMissing ret = make_shared(l->light, dagCallbackShim); // cnote << "Done loading."; - DEV_GUARDED(x_fulls) + DEV_GUARDED(get()->x_fulls) get()->m_fulls[_seedHash] = get()->m_lastUsedFull = ret; } @@ -198,7 +196,7 @@ EthashAux::FullType EthashAux::full(h256 const& _seedHash, bool _createIfMissing unsigned EthashAux::computeFull(h256 const& _seedHash, bool _createIfMissing) { - Guard l(x_fulls); + Guard l(get()->x_fulls); uint64_t blockNumber; DEV_IF_THROWS(blockNumber = EthashAux::number(_seedHash)) @@ -251,7 +249,7 @@ Ethash::Result EthashAux::eval(BlockInfo const& _header, Nonce const& _nonce) Ethash::Result EthashAux::eval(h256 const& _seedHash, h256 const& _headerHash, Nonce const& _nonce) { - DEV_GUARDED(x_fulls) + DEV_GUARDED(get()->x_fulls) if (FullType dag = get()->m_fulls[_seedHash].lock()) return dag->compute(_headerHash, _nonce); DEV_IF_THROWS(return EthashAux::get()->light(_seedHash)->compute(_headerHash, _nonce)) diff --git a/libethcore/EthashAux.h b/libethcore/EthashAux.h index 11485b733..654f898c3 100644 --- a/libethcore/EthashAux.h +++ b/libethcore/EthashAux.h @@ -38,7 +38,7 @@ class EthashAux public: ~EthashAux(); - static EthashAux* get() { if (!s_this) s_this = new EthashAux(); return s_this; } + static EthashAux* get() {Guard l(x_this);if (!s_this) s_this = new EthashAux(); return s_this; } struct LightAllocation { @@ -89,12 +89,13 @@ private: void killCache(h256 const& _s); + static Mutex x_this; static EthashAux* s_this; - static SharedMutex x_lights; + SharedMutex x_lights; std::unordered_map> m_lights; - static Mutex x_fulls; + Mutex x_fulls; std::condition_variable m_fullsChanged; std::unordered_map> m_fulls; FullType m_lastUsedFull; @@ -102,7 +103,7 @@ private: uint64_t m_generatingFullNumber = NotGenerating; unsigned m_fullProgress; - static Mutex x_epochs; + Mutex x_epochs; std::unordered_map m_epochs; h256s m_seedHashes; }; From b75514756a5a193466755762b352c6c6ffda28e5 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Tue, 23 Jun 2015 20:05:02 +0200 Subject: [PATCH 3/3] Use std::call_once instead of a mutex --- libethcore/EthashAux.cpp | 8 +++++++- libethcore/EthashAux.h | 3 +-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/libethcore/EthashAux.cpp b/libethcore/EthashAux.cpp index 4601e41e3..14a44a812 100644 --- a/libethcore/EthashAux.cpp +++ b/libethcore/EthashAux.cpp @@ -43,13 +43,19 @@ using namespace eth; const char* DAGChannel::name() { return EthGreen "DAG"; } -Mutex dev::eth::EthashAux::x_this; EthashAux* dev::eth::EthashAux::s_this = nullptr; EthashAux::~EthashAux() { } +EthashAux* EthashAux::get() +{ + static std::once_flag flag; + std::call_once(flag, []{s_this = new EthashAux();}); + return s_this; +} + uint64_t EthashAux::cacheSize(BlockInfo const& _header) { return ethash_get_cachesize((uint64_t)_header.number); diff --git a/libethcore/EthashAux.h b/libethcore/EthashAux.h index 654f898c3..d5bd60d44 100644 --- a/libethcore/EthashAux.h +++ b/libethcore/EthashAux.h @@ -38,7 +38,7 @@ class EthashAux public: ~EthashAux(); - static EthashAux* get() {Guard l(x_this);if (!s_this) s_this = new EthashAux(); return s_this; } + static EthashAux* get(); struct LightAllocation { @@ -89,7 +89,6 @@ private: void killCache(h256 const& _s); - static Mutex x_this; static EthashAux* s_this; SharedMutex x_lights;