diff options
-rw-r--r-- | src/core/lib/gpr/fork.cc | 16 | ||||
-rw-r--r-- | src/csharp/Grpc.Core.Tests/Internal/DefaultObjectPoolTest.cs | 18 | ||||
-rw-r--r-- | src/csharp/Grpc.Core/GrpcEnvironment.cs | 4 | ||||
-rw-r--r-- | src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs | 22 | ||||
-rw-r--r-- | src/csharp/Grpc.Core/Internal/DefaultObjectPool.cs | 11 | ||||
-rw-r--r-- | src/csharp/Grpc.Core/Internal/IPooledObject.cs | 34 | ||||
-rw-r--r-- | src/csharp/Grpc.Core/Internal/RequestCallContextSafeHandle.cs | 23 | ||||
-rwxr-xr-x | tools/run_tests/run_tests_matrix.py | 13 |
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, |