This is an automated email from the ASF dual-hosted git repository.
bbender 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 f9ba7e2 GEODE-9171: Fix leaked threads in .NET clients (#786)
f9ba7e2 is described below
commit f9ba7e2218b9e83aac220e8b152946814b35f73c
Author: Michael Martell <[email protected]>
AuthorDate: Wed Apr 21 20:31:04 2021 -0700
GEODE-9171: Fix leaked threads in .NET clients (#786)
* GEMNC-503: Fix thread leak in managed cache
- Call delete on native shared ptr does the trick
- Also shut down thread pool at cache close
* Add new GarbageCollect tests
Co-authored-by: Blake Bender <[email protected]>
Co-authored-by: Matthew Reddington <[email protected]>
---
clicache/integration-test2/CMakeLists.txt | 1 +
clicache/integration-test2/GarbageCollectCache.cs | 95 +++++++++++++++++++++++
clicache/src/Cache.cpp | 17 +++-
clicache/src/Cache.hpp | 2 +
clicache/src/native_shared_ptr.hpp | 5 +-
cppcache/include/geode/Cache.hpp | 4 +-
cppcache/src/CacheImpl.cpp | 2 +
cppcache/src/ThreadPool.cpp | 1 -
8 files changed, 122 insertions(+), 5 deletions(-)
diff --git a/clicache/integration-test2/CMakeLists.txt
b/clicache/integration-test2/CMakeLists.txt
index a5b0de3..91cb035 100644
--- a/clicache/integration-test2/CMakeLists.txt
+++ b/clicache/integration-test2/CMakeLists.txt
@@ -38,6 +38,7 @@ add_library( Apache.Geode.IntegrationTests2 SHARED
CacheXmlTests.cs
CqOperationTest.cs
FunctionExecutionTest.cs
+ GarbageCollectCache.cs
QueryTest.cs
RegionTest.cs
RegionSSLTest.cs
diff --git a/clicache/integration-test2/GarbageCollectCache.cs
b/clicache/integration-test2/GarbageCollectCache.cs
new file mode 100644
index 0000000..90c671c
--- /dev/null
+++ b/clicache/integration-test2/GarbageCollectCache.cs
@@ -0,0 +1,95 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.IO;
+using System.Linq;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace Apache.Geode.Client.IntegrationTests
+{
+ [Trait("Category", "Integration")]
+ public class GarbageCollectCache : TestBase
+ {
+ public GarbageCollectCache(ITestOutputHelper testOutputHelper) :
base(testOutputHelper)
+ {
+ }
+
+ [Fact]
+ public void VerifyNoLeakedThreads()
+ {
+ using (var cluster = new Cluster(output,
CreateTestCaseDirectoryName(), 1, 1))
+ {
+ Assert.True(cluster.Start());
+ Assert.Equal(0, cluster.Gfsh
+ .create()
+ .region()
+ .withName("testRegion")
+ .withType("PARTITION")
+ .execute());
+
+ for (int i=0; i<25; i++)
+ {
+ var ncThreadsBefore =
Process.GetCurrentProcess().Threads.Count;
+
+ using (var cache = new CacheFactory()
+ .Set("log-level", "none")
+ .Create())
+ {
+
+
cluster.ApplyLocators(cache.GetPoolFactory()).Create("default");
+
+ var regionFactory =
cache.CreateRegionFactory(RegionShortcut.PROXY)
+ .SetPoolName("default");
+
+ var region = regionFactory.Create<string,
string>("testRegion");
+
+ const string key = "hello";
+ const string expectedResult = "dave";
+
+ region.Put(key, expectedResult);
+ var actualResult = region.Get(key);
+ Assert.Equal(expectedResult, actualResult);
+
+ cache.Close();
+ }
+
+ var ncThreadsAfter =
Process.GetCurrentProcess().Threads.Count;
+ var ratio = ncThreadsBefore / (double)ncThreadsAfter;
+
+ // Because the number of threads in the process depends on
+ // the number of cores, debug vs release, whether the GC
thread
+ // is running, etc., a robust test is to check whether the
ratio
+ // of before and after threads is close to one. Also
skipping the
+ // first couple of iterations avoids threads related to
test
+ // environment startup.
+ if (i > 5)
+ {
+ //Assert.True(.8 < ratio && ratio < 1.3);
+ string error = "ncThreadsBefore = " +
ncThreadsBefore.ToString() +
+ ", ncThreadsAfter = " + ncThreadsAfter.ToString();
+ Assert.False(!(.8 < ratio && ratio < 1.3), error);
+ }
+ }
+ }
+ }
+ }
+}
diff --git a/clicache/src/Cache.cpp b/clicache/src/Cache.cpp
index 622ff85..c69e58e 100644
--- a/clicache/src/Cache.cpp
+++ b/clicache/src/Cache.cpp
@@ -18,7 +18,6 @@
#include "begin_native.hpp"
#include <CacheRegionHelper.hpp>
-#include <CacheImpl.hpp>
#include "end_native.hpp"
#include "Cache.hpp"
@@ -57,6 +56,22 @@ namespace Apache
m_typeRegistry = gcnew Apache::Geode::Client::TypeRegistry(this);
}
+ Cache::~Cache() {
+ if (m_nativeptr) {
+ try {
+ if (!IsClosed) {
+ Close();
+ }
+ }
+ catch (...) {
+ }
+ finally{
+ delete m_nativeptr;
+ m_nativeptr = nullptr;
+ }
+ }
+ }
+
String^ Cache::Name::get( )
{
try
diff --git a/clicache/src/Cache.hpp b/clicache/src/Cache.hpp
index 11ab23d..7d979dd 100644
--- a/clicache/src/Cache.hpp
+++ b/clicache/src/Cache.hpp
@@ -81,6 +81,8 @@ namespace Apache
: public IGeodeCache
{
public:
+
+ ~Cache();
/// <summary>
/// Initializes the cache from an XML file.
diff --git a/clicache/src/native_shared_ptr.hpp
b/clicache/src/native_shared_ptr.hpp
index e01cbe3..572c80a 100644
--- a/clicache/src/native_shared_ptr.hpp
+++ b/clicache/src/native_shared_ptr.hpp
@@ -42,7 +42,10 @@ namespace Apache
}
!native_shared_ptr() {
- delete ptr;
+ if (ptr) {
+ delete ptr;
+ ptr = nullptr;
+ }
}
inline _T* get() {
diff --git a/cppcache/include/geode/Cache.hpp b/cppcache/include/geode/Cache.hpp
index 2b625ff..bee6b68 100644
--- a/cppcache/include/geode/Cache.hpp
+++ b/cppcache/include/geode/Cache.hpp
@@ -176,8 +176,8 @@ class APACHE_GEODE_EXPORT Cache : public GeodeCache {
*
* @see RegionService
* @see PoolFactory#setMultiuserAuthentication(boolean)
- * @return the {@link RegionService} instance associated with a user and
given
- * properties.
+ * @return the {@link RegionService} instance associated with a user and
+ * given properties.
* @throws UnsupportedOperationException
* when invoked with multiuser-authentication as false.
*
diff --git a/cppcache/src/CacheImpl.cpp b/cppcache/src/CacheImpl.cpp
index 16539fe..fdaf206 100644
--- a/cppcache/src/CacheImpl.cpp
+++ b/cppcache/src/CacheImpl.cpp
@@ -323,6 +323,8 @@ void CacheImpl::close(bool keepalive) {
m_expiryTaskManager->stop();
+ m_threadPool.shutDown();
+
try {
getDistributedSystem().disconnect();
} catch (const apache::geode::client::NotConnectedException&) {
diff --git a/cppcache/src/ThreadPool.cpp b/cppcache/src/ThreadPool.cpp
index f9212d9..6f43baa 100644
--- a/cppcache/src/ThreadPool.cpp
+++ b/cppcache/src/ThreadPool.cpp
@@ -14,7 +14,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
#include "ThreadPool.hpp"
#include "DistributedSystemImpl.hpp"
