diff --git a/src/csharp/Grpc.Core.Api/ChannelCredentials.cs b/src/csharp/Grpc.Core.Api/ChannelCredentials.cs index fae390ce109..23ede0a512e 100644 --- a/src/csharp/Grpc.Core.Api/ChannelCredentials.cs +++ b/src/csharp/Grpc.Core.Api/ChannelCredentials.cs @@ -31,20 +31,12 @@ namespace Grpc.Core public abstract class ChannelCredentials { static readonly ChannelCredentials InsecureInstance = new InsecureCredentialsImpl(); - - // TODO: caching the instance!!!! - //readonly Lazy cachedNativeCredentials; /// /// Creates a new instance of channel credentials /// public ChannelCredentials() { - // Native credentials object need to be kept alive once initialized for subchannel sharing to work correctly - // with secure connections. See https://github.com/grpc/grpc/issues/15207. - // We rely on finalizer to clean up the native portion of ChannelCredentialsSafeHandle after the ChannelCredentials - // instance becomes unused. - //this.cachedNativeCredentials = new Lazy(() => CreateNativeCredentials()); } /// @@ -77,24 +69,6 @@ namespace Grpc.Core /// public abstract void InternalPopulateConfiguration(ChannelCredentialsConfiguratorBase configurator, object state); - // / - // / Gets native object for the credentials, creating one if it already doesn't exist. May return null if insecure channel - // / should be created. Caller must not call Dispose() on the returned native credentials as their lifetime - // / is managed by this class (and instances of native credentials are cached). - // / - // / The native credentials. - //internal ChannelCredentialsSafeHandle GetNativeCredentials() - //{ - // return cachedNativeCredentials.Value; - //} - - // / - // / Creates a new native object for the credentials. May return null if insecure channel - // / should be created. For internal use only, use instead. - // / - // / The native credentials. - //internal abstract ChannelCredentialsSafeHandle CreateNativeCredentials(); - /// /// Returns true if this credential type allows being composed by CompositeCredentials. /// diff --git a/src/csharp/Grpc.Core/Internal/DefaultChannelCredentialsConfigurator.cs b/src/csharp/Grpc.Core/Internal/DefaultChannelCredentialsConfigurator.cs index 8b9e1758598..aaccbdfe6f7 100644 --- a/src/csharp/Grpc.Core/Internal/DefaultChannelCredentialsConfigurator.cs +++ b/src/csharp/Grpc.Core/Internal/DefaultChannelCredentialsConfigurator.cs @@ -19,6 +19,7 @@ using System; using System.Collections.Generic; using System.Runtime.InteropServices; +using System.Runtime.CompilerServices; using Grpc.Core.Utils; using Grpc.Core.Logging; @@ -31,6 +32,12 @@ namespace Grpc.Core.Internal { static readonly ILogger Logger = GrpcEnvironment.Logger.ForType(); + // Native credentials object need to be kept alive once initialized for subchannel sharing to work correctly + // with secure connections. See https://github.com/grpc/grpc/issues/15207. + // We rely on finalizer to clean up the native portion of ChannelCredentialsSafeHandle after the ChannelCredentials + // instance becomes unused. + static readonly ConditionalWeakTable> CachedNativeCredentials = new ConditionalWeakTable>(); + bool configured; ChannelCredentialsSafeHandle nativeCredentials; @@ -48,18 +55,30 @@ namespace Grpc.Core.Internal { GrpcPreconditions.CheckState(!configured); configured = true; + nativeCredentials = GetOrCreateNativeCredentials((ChannelCredentials) state, + () => CreateNativeSslCredentials(rootCertificates, keyCertificatePair, verifyPeerCallback)); + } + + public override void SetCompositeCredentials(object state, ChannelCredentials channelCredentials, CallCredentials callCredentials) + { + GrpcPreconditions.CheckState(!configured); + configured = true; + nativeCredentials = GetOrCreateNativeCredentials((ChannelCredentials) state, + () => CreateNativeCompositeCredentials(channelCredentials, callCredentials)); + } + + private ChannelCredentialsSafeHandle CreateNativeSslCredentials(string rootCertificates, KeyCertificatePair keyCertificatePair, VerifyPeerCallback verifyPeerCallback) + { IntPtr verifyPeerCallbackTag = IntPtr.Zero; if (verifyPeerCallback != null) { verifyPeerCallbackTag = new VerifyPeerCallbackRegistration(verifyPeerCallback).CallbackRegistration.Tag; } - nativeCredentials = ChannelCredentialsSafeHandle.CreateSslCredentials(rootCertificates, keyCertificatePair, verifyPeerCallbackTag); + return ChannelCredentialsSafeHandle.CreateSslCredentials(rootCertificates, keyCertificatePair, verifyPeerCallbackTag); } - public override void SetCompositeCredentials(object state, ChannelCredentials channelCredentials, CallCredentials callCredentials) + private ChannelCredentialsSafeHandle CreateNativeCompositeCredentials(ChannelCredentials channelCredentials, CallCredentials callCredentials) { - GrpcPreconditions.CheckState(!configured); - configured = true; using (var callCreds = callCredentials.ToNativeCredentials()) { var nativeComposite = ChannelCredentialsSafeHandle.CreateComposite(channelCredentials.ToNativeCredentials(), callCreds); @@ -67,8 +86,32 @@ namespace Grpc.Core.Internal { throw new ArgumentException("Error creating native composite credentials. Likely, this is because you are trying to compose incompatible credentials."); } - nativeCredentials = nativeComposite; + return nativeComposite; + } + } + + private ChannelCredentialsSafeHandle GetOrCreateNativeCredentials(ChannelCredentials key, Func nativeCredentialsFactory) + { + Lazy lazyValue; + while (true) + { + if (CachedNativeCredentials.TryGetValue(key, out lazyValue)) + { + break; + } + + lazyValue = new Lazy(nativeCredentialsFactory); + try + { + CachedNativeCredentials.Add(key, lazyValue); + break; + } + catch (ArgumentException) + { + // key exists, next TryGetValue should fetch the value + } } + return lazyValue.Value; } private class VerifyPeerCallbackRegistration