aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
-rw-r--r--src/core/lib/gpr/fork.cc16
-rw-r--r--src/csharp/Grpc.Core.Tests/Internal/DefaultObjectPoolTest.cs18
-rw-r--r--src/csharp/Grpc.Core/GrpcEnvironment.cs4
-rw-r--r--src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs22
-rw-r--r--src/csharp/Grpc.Core/Internal/DefaultObjectPool.cs11
-rw-r--r--src/csharp/Grpc.Core/Internal/IPooledObject.cs34
-rw-r--r--src/csharp/Grpc.Core/Internal/RequestCallContextSafeHandle.cs23
-rwxr-xr-xtools/run_tests/run_tests_matrix.py13
8 files changed, 120 insertions, 21 deletions
diff --git a/src/core/lib/gpr/fork.cc b/src/core/lib/gpr/fork.cc
index c47e686378..92023f4350 100644
--- a/src/core/lib/gpr/fork.cc
+++ b/src/core/lib/gpr/fork.cc
@@ -38,18 +38,32 @@ void grpc_fork_support_init() {
fork_support_enabled = 1;
#else
fork_support_enabled = 0;
+#endif
+ bool env_var_set = false;
char* env = gpr_getenv("GRPC_ENABLE_FORK_SUPPORT");
if (env != nullptr) {
static const char* truthy[] = {"yes", "Yes", "YES", "true",
"True", "TRUE", "1"};
+ static const char* falsey[] = {"no", "No", "NO", "false",
+ "False", "FALSE", "0"};
for (size_t i = 0; i < GPR_ARRAY_SIZE(truthy); i++) {
if (0 == strcmp(env, truthy[i])) {
fork_support_enabled = 1;
+ env_var_set = true;
+ break;
+ }
+ }
+ if (!env_var_set) {
+ for (size_t i = 0; i < GPR_ARRAY_SIZE(falsey); i++) {
+ if (0 == strcmp(env, falsey[i])) {
+ fork_support_enabled = 0;
+ env_var_set = true;
+ break;
+ }
}
}
gpr_free(env);
}
-#endif
if (override_fork_support_enabled != -1) {
fork_support_enabled = override_fork_support_enabled;
}
diff --git a/src/csharp/Grpc.Core.Tests/Internal/DefaultObjectPoolTest.cs b/src/csharp/Grpc.Core.Tests/Internal/DefaultObjectPoolTest.cs
index b6bb0a9eae..9c6f8a2117 100644
--- a/src/csharp/Grpc.Core.Tests/Internal/DefaultObjectPoolTest.cs
+++ b/src/csharp/Grpc.Core.Tests/Internal/DefaultObjectPoolTest.cs
@@ -60,6 +60,16 @@ namespace Grpc.Core.Internal.Tests
}
[Test]
+ public void LeaseSetsReturnAction()
+ {
+ var pool = new DefaultObjectPool<TestPooledObject>(() => new TestPooledObject(), 10, 0);
+ var origLeased = pool.Lease();
+ origLeased.ReturnAction(origLeased);
+ pool.Dispose();
+ Assert.AreNotSame(origLeased, pool.Lease());
+ }
+
+ [Test]
public void Constructor()
{
Assert.Throws<ArgumentNullException>(() => new DefaultObjectPool<TestPooledObject>(null, 10, 2));
@@ -67,8 +77,14 @@ namespace Grpc.Core.Internal.Tests
Assert.Throws<ArgumentException>(() => new DefaultObjectPool<TestPooledObject>(() => new TestPooledObject(), 10, -1));
}
- class TestPooledObject : IDisposable
+ class TestPooledObject : IPooledObject<TestPooledObject>
{
+ public Action<TestPooledObject> ReturnAction;
+
+ public void SetReturnToPoolAction(Action<TestPooledObject> returnAction)
+ {
+ this.ReturnAction = returnAction;
+ }
public void Dispose()
{
diff --git a/src/csharp/Grpc.Core/GrpcEnvironment.cs b/src/csharp/Grpc.Core/GrpcEnvironment.cs
index 6296f1863b..6bb2f6c3e5 100644
--- a/src/csharp/Grpc.Core/GrpcEnvironment.cs
+++ b/src/csharp/Grpc.Core/GrpcEnvironment.cs
@@ -299,8 +299,8 @@ namespace Grpc.Core
private GrpcEnvironment()
{
GrpcNativeInit();
- batchContextPool = new DefaultObjectPool<BatchContextSafeHandle>(() => BatchContextSafeHandle.Create(this.batchContextPool), batchContextPoolSharedCapacity, batchContextPoolThreadLocalCapacity);
- requestCallContextPool = new DefaultObjectPool<RequestCallContextSafeHandle>(() => RequestCallContextSafeHandle.Create(this.requestCallContextPool), requestCallContextPoolSharedCapacity, requestCallContextPoolThreadLocalCapacity);
+ batchContextPool = new DefaultObjectPool<BatchContextSafeHandle>(() => BatchContextSafeHandle.Create(), batchContextPoolSharedCapacity, batchContextPoolThreadLocalCapacity);
+ requestCallContextPool = new DefaultObjectPool<RequestCallContextSafeHandle>(() => RequestCallContextSafeHandle.Create(), requestCallContextPoolSharedCapacity, requestCallContextPoolThreadLocalCapacity);
threadPool = new GrpcThreadPool(this, GetThreadPoolSizeOrDefault(), GetCompletionQueueCountOrDefault(), inlineHandlers);
threadPool.Start();
}
diff --git a/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs b/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs
index 83385ad7d3..53a859d18f 100644
--- a/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs
+++ b/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs
@@ -33,22 +33,21 @@ namespace Grpc.Core.Internal
/// <summary>
/// grpcsharp_batch_context
/// </summary>
- internal class BatchContextSafeHandle : SafeHandleZeroIsInvalid, IOpCompletionCallback
+ internal class BatchContextSafeHandle : SafeHandleZeroIsInvalid, IOpCompletionCallback, IPooledObject<BatchContextSafeHandle>
{
static readonly NativeMethods Native = NativeMethods.Get();
static readonly ILogger Logger = GrpcEnvironment.Logger.ForType<BatchContextSafeHandle>();
- IObjectPool<BatchContextSafeHandle> ownedByPool;
+ Action<BatchContextSafeHandle> returnToPoolAction;
CompletionCallbackData completionCallbackData;
private BatchContextSafeHandle()
{
}
- public static BatchContextSafeHandle Create(IObjectPool<BatchContextSafeHandle> ownedByPool = null)
+ public static BatchContextSafeHandle Create()
{
var ctx = Native.grpcsharp_batch_context_create();
- ctx.ownedByPool = ownedByPool;
return ctx;
}
@@ -60,6 +59,12 @@ namespace Grpc.Core.Internal
}
}
+ public void SetReturnToPoolAction(Action<BatchContextSafeHandle> returnAction)
+ {
+ GrpcPreconditions.CheckState(returnToPoolAction == null);
+ returnToPoolAction = returnAction;
+ }
+
public void SetCompletionCallback(BatchCompletionDelegate callback, object state)
{
GrpcPreconditions.CheckState(completionCallbackData.Callback == null);
@@ -109,10 +114,15 @@ namespace Grpc.Core.Internal
public void Recycle()
{
- if (ownedByPool != null)
+ if (returnToPoolAction != null)
{
Native.grpcsharp_batch_context_reset(this);
- ownedByPool.Return(this);
+
+ var origReturnAction = returnToPoolAction;
+ // Not clearing all the references to the pool could prevent garbage collection of the pool object
+ // and thus cause memory leaks.
+ returnToPoolAction = null;
+ origReturnAction(this);
}
else
{
diff --git a/src/csharp/Grpc.Core/Internal/DefaultObjectPool.cs b/src/csharp/Grpc.Core/Internal/DefaultObjectPool.cs
index 2f030f3e02..0e1dc4d158 100644
--- a/src/csharp/Grpc.Core/Internal/DefaultObjectPool.cs
+++ b/src/csharp/Grpc.Core/Internal/DefaultObjectPool.cs
@@ -27,9 +27,10 @@ namespace Grpc.Core.Internal
/// Pool of objects that combines a shared pool and a thread local pool.
/// </summary>
internal class DefaultObjectPool<T> : IObjectPool<T>
- where T : class, IDisposable
+ where T : class, IPooledObject<T>
{
readonly object myLock = new object();
+ readonly Action<T> returnAction;
readonly Func<T> itemFactory;
// Queue shared between threads, access needs to be synchronized.
@@ -54,6 +55,7 @@ namespace Grpc.Core.Internal
{
GrpcPreconditions.CheckArgument(sharedCapacity >= 0);
GrpcPreconditions.CheckArgument(threadLocalCapacity >= 0);
+ this.returnAction = Return;
this.itemFactory = GrpcPreconditions.CheckNotNull(itemFactory, nameof(itemFactory));
this.sharedQueue = new Queue<T>(sharedCapacity);
this.sharedCapacity = sharedCapacity;
@@ -74,6 +76,13 @@ namespace Grpc.Core.Internal
/// </summary>
public T Lease()
{
+ var item = LeaseInternal();
+ item.SetReturnToPoolAction(returnAction);
+ return item;
+ }
+
+ private T LeaseInternal()
+ {
var localData = threadLocalData.Value;
if (localData.Queue.Count > 0)
{
diff --git a/src/csharp/Grpc.Core/Internal/IPooledObject.cs b/src/csharp/Grpc.Core/Internal/IPooledObject.cs
new file mode 100644
index 0000000000..e20bd51dce
--- /dev/null
+++ b/src/csharp/Grpc.Core/Internal/IPooledObject.cs
@@ -0,0 +1,34 @@
+#region Copyright notice and license
+
+// Copyright 2018 gRPC authors.
+//
+// Licensed 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.
+
+#endregion
+
+using System;
+
+namespace Grpc.Core.Internal
+{
+ /// <summary>
+ /// An object that can be pooled in <c>IObjectPool</c>.
+ /// </summary>
+ /// <typeparam name="T"></typeparam>
+ internal interface IPooledObject<T> : IDisposable
+ {
+ /// <summary>
+ /// Set the action that will be invoked to return a leased object to the pool.
+ /// </summary>
+ void SetReturnToPoolAction(Action<T> returnAction);
+ }
+}
diff --git a/src/csharp/Grpc.Core/Internal/RequestCallContextSafeHandle.cs b/src/csharp/Grpc.Core/Internal/RequestCallContextSafeHandle.cs
index 59e9d9b1ab..ebc2d6d8d6 100644
--- a/src/csharp/Grpc.Core/Internal/RequestCallContextSafeHandle.cs
+++ b/src/csharp/Grpc.Core/Internal/RequestCallContextSafeHandle.cs
@@ -20,26 +20,26 @@ using System;
using System.Runtime.InteropServices;
using Grpc.Core;
using Grpc.Core.Logging;
+using Grpc.Core.Utils;
namespace Grpc.Core.Internal
{
/// <summary>
/// grpcsharp_request_call_context
/// </summary>
- internal class RequestCallContextSafeHandle : SafeHandleZeroIsInvalid, IOpCompletionCallback
+ internal class RequestCallContextSafeHandle : SafeHandleZeroIsInvalid, IOpCompletionCallback, IPooledObject<RequestCallContextSafeHandle>
{
static readonly NativeMethods Native = NativeMethods.Get();
static readonly ILogger Logger = GrpcEnvironment.Logger.ForType<RequestCallContextSafeHandle>();
- IObjectPool<RequestCallContextSafeHandle> ownedByPool;
+ Action<RequestCallContextSafeHandle> returnToPoolAction;
private RequestCallContextSafeHandle()
{
}
- public static RequestCallContextSafeHandle Create(IObjectPool<RequestCallContextSafeHandle> ownedByPool = null)
+ public static RequestCallContextSafeHandle Create()
{
var ctx = Native.grpcsharp_request_call_context_create();
- ctx.ownedByPool = ownedByPool;
return ctx;
}
@@ -51,6 +51,12 @@ namespace Grpc.Core.Internal
}
}
+ public void SetReturnToPoolAction(Action<RequestCallContextSafeHandle> returnAction)
+ {
+ GrpcPreconditions.CheckState(returnToPoolAction == null);
+ returnToPoolAction = returnAction;
+ }
+
public RequestCallCompletionDelegate CompletionCallback { get; set; }
// Gets data of server_rpc_new completion.
@@ -76,10 +82,15 @@ namespace Grpc.Core.Internal
public void Recycle()
{
- if (ownedByPool != null)
+ if (returnToPoolAction != null)
{
Native.grpcsharp_request_call_context_reset(this);
- ownedByPool.Return(this);
+
+ var origReturnAction = returnToPoolAction;
+ // Not clearing all the references to the pool could prevent garbage collection of the pool object
+ // and thus cause memory leaks.
+ returnToPoolAction = null;
+ origReturnAction(this);
}
else
{
diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py
index 344035478b..ae3a28bde5 100755
--- a/tools/run_tests/run_tests_matrix.py
+++ b/tools/run_tests/run_tests_matrix.py
@@ -44,14 +44,19 @@ _DEFAULT_INNER_JOBS = 2
_REPORT_SUFFIX = 'sponge_log.xml'
+def _safe_report_name(name):
+ """Reports with '+' in target name won't show correctly in ResultStore"""
+ return name.replace('+', 'p')
+
+
def _report_filename(name):
"""Generates report file name"""
- return 'report_%s_%s' % (name, _REPORT_SUFFIX)
+ return 'report_%s_%s' % (_safe_report_name(name), _REPORT_SUFFIX)
def _report_filename_internal_ci(name):
"""Generates report file name that leads to better presentation by internal CI"""
- return '%s/%s' % (name, _REPORT_SUFFIX)
+ return '%s/%s' % (_safe_report_name(name), _REPORT_SUFFIX)
def _docker_jobspec(name,
@@ -68,7 +73,7 @@ def _docker_jobspec(name,
'-j',
str(inner_jobs), '-x',
_report_filename(name), '--report_suite_name',
- '%s' % name
+ '%s' % _safe_report_name(name)
] + runtests_args,
environ=runtests_envs,
shortname='run_tests_%s' % name,
@@ -95,7 +100,7 @@ def _workspace_jobspec(name,
'-t', '-j',
str(inner_jobs), '-x',
'../%s' % _report_filename(name), '--report_suite_name',
- '%s' % name
+ '%s' % _safe_report_name(name)
] + runtests_args,
environ=env,
shortname='run_tests_%s' % name,