This is an automated email from the ASF dual-hosted git repository.
mmartell pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode-native.git
The following commit(s) were added to refs/heads/develop by this push:
new 2c70be3 GEODE-7316: Fix nc shutdown crash (#539)
2c70be3 is described below
commit 2c70be377941487a5ff45eccea04ef65a51dd61c
Author: Blake Bender <[email protected]>
AuthorDate: Mon Oct 21 22:20:34 2019 +0000
GEODE-7316: Fix nc shutdown crash (#539)
---
cppcache/include/geode/Cache.hpp | 2 +-
cppcache/src/CacheImpl.cpp | 56 ++++++++++++++++++++--------------
cppcache/src/CacheImpl.hpp | 9 +++---
cppcache/src/CachePerfStats.hpp | 3 ++
cppcache/src/ClientMetadataService.cpp | 10 +++---
5 files changed, 48 insertions(+), 32 deletions(-)
diff --git a/cppcache/include/geode/Cache.hpp b/cppcache/include/geode/Cache.hpp
index 3a0635c..f1f1d6b 100644
--- a/cppcache/include/geode/Cache.hpp
+++ b/cppcache/include/geode/Cache.hpp
@@ -254,7 +254,7 @@ class APACHE_GEODE_EXPORT Cache : public GeodeCache {
bool readPdxSerialized,
const std::shared_ptr<AuthInitialize>& authInitialize);
- std::unique_ptr<CacheImpl> m_cacheImpl;
+ std::shared_ptr<CacheImpl> m_cacheImpl;
protected:
static bool isPoolInMultiuserMode(std::shared_ptr<Region> regionPtr);
diff --git a/cppcache/src/CacheImpl.cpp b/cppcache/src/CacheImpl.cpp
index 168a23c..caeec99 100644
--- a/cppcache/src/CacheImpl.cpp
+++ b/cppcache/src/CacheImpl.cpp
@@ -19,6 +19,8 @@
#include <string>
+#include <ace/Guard_T.h>
+
#include <geode/CacheStatistics.hpp>
#include <geode/PersistenceManager.hpp>
#include <geode/PoolManager.hpp>
@@ -162,7 +164,7 @@ CacheImpl::RegionKind CacheImpl::getRegionKind(
}
void CacheImpl::removeRegion(const std::string& name) {
- TryReadGuard guardCacheDestroy(m_destroyCacheMutex, m_destroyPending);
+ ACE_Guard<ACE_Recursive_Thread_Mutex> lock(m_destroyCacheMutex, 1);
if (!m_destroyPending) {
m_regions.erase(name);
}
@@ -240,15 +242,18 @@ void CacheImpl::close(bool keepalive) {
TcrMessage::setKeepAlive(keepalive);
// bug #247 fix for durable clients missing events when recycled
sendNotificationCloseMsgs();
+
{
- TryWriteGuard guardCacheDestroy(m_destroyCacheMutex, m_destroyPending);
+ ACE_Guard<ACE_Recursive_Thread_Mutex> lock(m_destroyCacheMutex, 1);
if (m_destroyPending) {
return;
}
m_destroyPending = true;
}
- if (m_closed || (!m_initialized)) return;
+ if (m_closed || (!m_initialized)) {
+ return;
+ }
// Close the distribution manager used for queries.
if (m_remoteQueryServicePtr != nullptr) {
@@ -324,14 +329,21 @@ void CacheImpl::close(bool keepalive) {
LOGFINE("Cache closed.");
}
-bool CacheImpl::isCacheDestroyPending() const { return m_destroyPending; }
+bool CacheImpl::doIfDestroyNotPending(std::function<void()> f) {
+ ACE_Guard<ACE_Recursive_Thread_Mutex> lock(m_destroyCacheMutex, 1);
+ if (lock.locked() && !m_destroyPending) {
+ f();
+ }
+
+ return !m_destroyPending;
+}
void CacheImpl::validateRegionAttributes(
const std::string& name, const RegionAttributes& regionAttributes) const {
auto&& kind = getRegionKind(regionAttributes);
if (regionAttributes.m_clientNotificationEnabled && kind == CPP_REGION) {
throw UnsupportedOperationException(
- "Cache::createRegion: \"" + name +
+ "CacheImpl::createRegion: \"" + name +
"\" Client notification can be enabled only for native client region");
}
}
@@ -370,7 +382,7 @@ void CacheImpl::createRegion(std::string name,
auto&& lock = m_regions.make_lock();
if (m_regions.find(name) != m_regions.end()) {
- throw RegionExistsException("Cache::createRegion: \"" + name +
+ throw RegionExistsException("CacheImpl::createRegion: \"" + name +
"\" region exists in local cache");
}
@@ -385,17 +397,18 @@ void CacheImpl::createRegion(std::string name,
} catch (const Exception&) {
throw;
} catch (std::exception& ex) {
- throw UnknownException("Cache::createRegion: Failed to create Region \""
+
- name +
- "\" due to unknown exception: " + ex.what());
+ throw UnknownException(
+ "CacheImpl::createRegion: Failed to create Region \"" + name +
+ "\" due to unknown exception: " + ex.what());
} catch (...) {
- throw UnknownException("Cache::createRegion: Failed to create Region \""
+
- name + "\" due to unknown exception.");
+ throw UnknownException(
+ "CacheImpl::createRegion: Failed to create Region \"" + name +
+ "\" due to unknown exception.");
}
if (rpImpl == nullptr) {
throw RegionCreationFailedException(
- "Cache::createRegion: Failed to create Region \"" + name + "\"");
+ "CacheImpl::createRegion: Failed to create Region \"" + name + "\"");
}
regionPtr = rpImpl;
@@ -449,11 +462,10 @@ std::shared_ptr<Region> CacheImpl::findRegion(const
std::string& name) {
}
std::shared_ptr<Region> CacheImpl::getRegion(const std::string& path) {
- LOGDEBUG("Cache::getRegion " + path);
+ LOGDEBUG("CacheImpl::getRegion " + path);
this->throwIfClosed();
-
- TryReadGuard guardCacheDestroy(m_destroyCacheMutex, m_destroyPending);
+ ACE_Guard<ACE_Recursive_Thread_Mutex> lock(m_destroyCacheMutex, 1);
if (m_destroyPending) {
return nullptr;
@@ -461,7 +473,7 @@ std::shared_ptr<Region> CacheImpl::getRegion(const
std::string& path) {
static const std::string slash("/");
if (path == slash || path.length() < 1) {
- LOGERROR("Cache::getRegion: path [" + path + "] is not valid.");
+ LOGERROR("CacheImpl::getRegion: path [" + path + "] is not valid.");
throw IllegalArgumentException("Cache::getRegion: path is empty or a /");
}
@@ -597,14 +609,12 @@ void CacheImpl::readyForEvents() {
bool isDurable = m_tcrConnectionManager->isDurable();
if (!isDurable && autoReadyForEvents) {
- LOGERROR(
- "Only durable clients or clients with the "
- "auto-ready-for-events property set to false should call "
- "readyForEvents()");
- throw IllegalStateException(
+ std::string msg =
"Only durable clients or clients with the "
"auto-ready-for-events property set to false should call "
- "readyForEvents()");
+ "readyForEvents()";
+ LOGERROR("CacheImpl::%s(%p): %s", __FUNCTION__, this, msg.c_str());
+ throw IllegalStateException(msg.c_str());
}
// Send the CLIENT_READY message to the server
@@ -670,7 +680,7 @@ bool CacheImpl::getEndpointStatus(const std::string&
endpoint) {
}
void CacheImpl::processMarker() {
- TryReadGuard guardCacheDestroy(m_destroyCacheMutex, m_destroyPending);
+ ACE_Guard<ACE_Recursive_Thread_Mutex> destroy_lock(m_destroyCacheMutex, 1);
if (m_destroyPending) {
return;
}
diff --git a/cppcache/src/CacheImpl.hpp b/cppcache/src/CacheImpl.hpp
index bd718b6..b5f4441 100644
--- a/cppcache/src/CacheImpl.hpp
+++ b/cppcache/src/CacheImpl.hpp
@@ -24,7 +24,7 @@
#include <memory>
#include <mutex>
-#include <ace/RW_Thread_Mutex.h>
+#include <ace/Recursive_Thread_Mutex.h>
#include <geode/Cache.hpp>
#include <geode/PoolManager.hpp>
@@ -272,8 +272,6 @@ class APACHE_GEODE_EXPORT CacheImpl : private NonCopyable,
return m_readPdxSerialized;
}
- bool isCacheDestroyPending() const;
-
static std::map<std::string, RegionAttributes> getRegionShortcut();
std::shared_ptr<PdxTypeRegistry> getPdxTypeRegistry() const;
@@ -322,6 +320,8 @@ class APACHE_GEODE_EXPORT CacheImpl : private NonCopyable,
std::shared_ptr<Properties> userSecurityProperties,
const std::string& poolName);
+ bool doIfDestroyNotPending(std::function<void()>);
+
private:
std::atomic<bool> m_networkhop;
std::atomic<int> m_blacklistBucketTimeout;
@@ -374,7 +374,7 @@ class APACHE_GEODE_EXPORT CacheImpl : private NonCopyable,
std::unique_ptr<EvictionController> m_evictionController;
TcrConnectionManager* m_tcrConnectionManager;
std::shared_ptr<RemoteQueryService> m_remoteQueryServicePtr;
- ACE_RW_Thread_Mutex m_destroyCacheMutex;
+ ACE_Recursive_Thread_Mutex m_destroyCacheMutex;
volatile bool m_destroyPending;
volatile bool m_initDone;
std::mutex m_initDoneLock;
@@ -397,6 +397,7 @@ class APACHE_GEODE_EXPORT CacheImpl : private NonCopyable,
friend class CacheFactory;
friend class Cache;
friend class DistributedSystemImpl;
+ friend class PdxInstanceFactory;
};
} // namespace client
} // namespace geode
diff --git a/cppcache/src/CachePerfStats.hpp b/cppcache/src/CachePerfStats.hpp
index 9c7ad7e..3bd633b 100644
--- a/cppcache/src/CachePerfStats.hpp
+++ b/cppcache/src/CachePerfStats.hpp
@@ -201,6 +201,9 @@ class APACHE_GEODE_EXPORT CachePerfStats {
m_cachePerfStats->setLong(m_pdxDeserializedBytesId, 0);
}
+ CachePerfStats(const CachePerfStats& other) = default;
+ CachePerfStats(CachePerfStats&& other) = default;
+
virtual ~CachePerfStats() { m_cachePerfStats = nullptr; }
void close() {
diff --git a/cppcache/src/ClientMetadataService.cpp
b/cppcache/src/ClientMetadataService.cpp
index bf78afa..191b9c6 100644
--- a/cppcache/src/ClientMetadataService.cpp
+++ b/cppcache/src/ClientMetadataService.cpp
@@ -81,10 +81,12 @@ void ClientMetadataService::svc() {
m_regionQueue.pop_front();
queue::coalesce(m_regionQueue, regionFullPath);
- if (!m_cache->isCacheDestroyPending()) {
- lock.unlock();
- getClientPRMetadata(regionFullPath.c_str());
- } else {
+ if (!m_cache->doIfDestroyNotPending([&]() {
+ lock.unlock();
+ getClientPRMetadata(regionFullPath.c_str());
+ })) {
+ LOGINFO("ClientMetadataService::%s(%p): destroy is pending, bail out",
+ __FUNCTION__, this);
break;
}
}
