diff options
-rw-r--r-- | src/csharp/Grpc.Core.Tests/ChannelCredentialsTest.cs | 37 | ||||
-rw-r--r-- | src/csharp/Grpc.Core.Tests/FakeCredentials.cs | 10 | ||||
-rw-r--r-- | src/csharp/Grpc.Core/Channel.cs | 2 | ||||
-rw-r--r-- | src/csharp/Grpc.Core/ChannelCredentials.cs | 39 |
4 files changed, 63 insertions, 25 deletions
diff --git a/src/csharp/Grpc.Core.Tests/ChannelCredentialsTest.cs b/src/csharp/Grpc.Core.Tests/ChannelCredentialsTest.cs index 62cc904a61..843d88bfb6 100644 --- a/src/csharp/Grpc.Core.Tests/ChannelCredentialsTest.cs +++ b/src/csharp/Grpc.Core.Tests/ChannelCredentialsTest.cs @@ -17,13 +17,7 @@ #endregion using System; -using System.Diagnostics; -using System.Runtime.InteropServices; -using System.Threading; -using System.Threading.Tasks; -using Grpc.Core; using Grpc.Core.Internal; -using Grpc.Core.Utils; using NUnit.Framework; namespace Grpc.Core.Tests @@ -44,9 +38,38 @@ namespace Grpc.Core.Tests Assert.Throws(typeof(ArgumentNullException), () => ChannelCredentials.Create(null, new FakeCallCredentials())); Assert.Throws(typeof(ArgumentNullException), () => ChannelCredentials.Create(new FakeChannelCredentials(true), null)); - + // forbid composing non-composable Assert.Throws(typeof(ArgumentException), () => ChannelCredentials.Create(new FakeChannelCredentials(false), new FakeCallCredentials())); } + + [Test] + public void ChannelCredentials_NativeCredentialsAreReused() + { + // always returning the same native object is critical for subchannel sharing to work with secure channels + var creds = new SslCredentials(); + var nativeCreds1 = creds.GetNativeCredentials(); + var nativeCreds2 = creds.GetNativeCredentials(); + Assert.AreSame(nativeCreds1, nativeCreds2); + } + + [Test] + public void ChannelCredentials_CreateExceptionIsCached() + { + var creds = new ChannelCredentialsWithCreateNativeThrows(); + var ex1 = Assert.Throws(typeof(Exception), () => creds.GetNativeCredentials()); + var ex2 = Assert.Throws(typeof(Exception), () => creds.GetNativeCredentials()); + Assert.AreSame(ex1, ex2); + } + + internal class ChannelCredentialsWithCreateNativeThrows : ChannelCredentials + { + internal override bool IsComposable => false; + + internal override ChannelCredentialsSafeHandle CreateNativeCredentials() + { + throw new Exception("Creation of native credentials has failed on purpose."); + } + } } } diff --git a/src/csharp/Grpc.Core.Tests/FakeCredentials.cs b/src/csharp/Grpc.Core.Tests/FakeCredentials.cs index 7d658576e5..f23c9e9757 100644 --- a/src/csharp/Grpc.Core.Tests/FakeCredentials.cs +++ b/src/csharp/Grpc.Core.Tests/FakeCredentials.cs @@ -16,15 +16,7 @@ #endregion -using System; -using System.Diagnostics; -using System.Runtime.InteropServices; -using System.Threading; -using System.Threading.Tasks; -using Grpc.Core; using Grpc.Core.Internal; -using Grpc.Core.Utils; -using NUnit.Framework; namespace Grpc.Core.Tests { @@ -42,7 +34,7 @@ namespace Grpc.Core.Tests get { return composable; } } - internal override ChannelCredentialsSafeHandle ToNativeCredentials() + internal override ChannelCredentialsSafeHandle CreateNativeCredentials() { return null; } diff --git a/src/csharp/Grpc.Core/Channel.cs b/src/csharp/Grpc.Core/Channel.cs index e9930b6fbc..7912b06b71 100644 --- a/src/csharp/Grpc.Core/Channel.cs +++ b/src/csharp/Grpc.Core/Channel.cs @@ -72,9 +72,9 @@ namespace Grpc.Core this.environment = GrpcEnvironment.AddRef(); this.completionQueue = this.environment.PickCompletionQueue(); - using (var nativeCredentials = credentials.ToNativeCredentials()) using (var nativeChannelArgs = ChannelOptions.CreateChannelArgs(this.options.Values)) { + var nativeCredentials = credentials.GetNativeCredentials(); if (nativeCredentials != null) { this.handle = ChannelSafeHandle.CreateSecure(nativeCredentials, target, nativeChannelArgs); diff --git a/src/csharp/Grpc.Core/ChannelCredentials.cs b/src/csharp/Grpc.Core/ChannelCredentials.cs index ba482897d7..3ce32f31b7 100644 --- a/src/csharp/Grpc.Core/ChannelCredentials.cs +++ b/src/csharp/Grpc.Core/ChannelCredentials.cs @@ -31,6 +31,19 @@ namespace Grpc.Core public abstract class ChannelCredentials { static readonly ChannelCredentials InsecureInstance = new InsecureCredentialsImpl(); + readonly Lazy<ChannelCredentialsSafeHandle> cachedNativeCredentials; + + /// <summary> + /// Creates a new instance of channel credentials + /// </summary> + 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<ChannelCredentialsSafeHandle>(() => CreateNativeCredentials()); + } /// <summary> /// Returns instance of credentials that provides no security and @@ -57,11 +70,22 @@ namespace Grpc.Core } /// <summary> - /// Creates native object for the credentials. May return null if insecure channel - /// should be created. + /// 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 <c>Dispose()</c> on the returned native credentials as their lifetime + /// is managed by this class (and instances of native credentials are cached). + /// </summary> + /// <returns>The native credentials.</returns> + internal ChannelCredentialsSafeHandle GetNativeCredentials() + { + return cachedNativeCredentials.Value; + } + + /// <summary> + /// Creates a new native object for the credentials. May return null if insecure channel + /// should be created. For internal use only, use <see cref="GetNativeCredentials"/> instead. /// </summary> /// <returns>The native credentials.</returns> - internal abstract ChannelCredentialsSafeHandle ToNativeCredentials(); + internal abstract ChannelCredentialsSafeHandle CreateNativeCredentials(); /// <summary> /// Returns <c>true</c> if this credential type allows being composed by <c>CompositeCredentials</c>. @@ -73,7 +97,7 @@ namespace Grpc.Core private sealed class InsecureCredentialsImpl : ChannelCredentials { - internal override ChannelCredentialsSafeHandle ToNativeCredentials() + internal override ChannelCredentialsSafeHandle CreateNativeCredentials() { return null; } @@ -145,7 +169,7 @@ namespace Grpc.Core get { return true; } } - internal override ChannelCredentialsSafeHandle ToNativeCredentials() + internal override ChannelCredentialsSafeHandle CreateNativeCredentials() { return ChannelCredentialsSafeHandle.CreateSslCredentials(rootCertificates, keyCertificatePair); } @@ -173,12 +197,11 @@ namespace Grpc.Core GrpcPreconditions.CheckArgument(channelCredentials.IsComposable, "Supplied channel credentials do not allow composition."); } - internal override ChannelCredentialsSafeHandle ToNativeCredentials() + internal override ChannelCredentialsSafeHandle CreateNativeCredentials() { - using (var channelCreds = channelCredentials.ToNativeCredentials()) using (var callCreds = callCredentials.ToNativeCredentials()) { - var nativeComposite = ChannelCredentialsSafeHandle.CreateComposite(channelCreds, callCreds); + var nativeComposite = ChannelCredentialsSafeHandle.CreateComposite(channelCredentials.GetNativeCredentials(), callCreds); if (nativeComposite.IsInvalid) { throw new ArgumentException("Error creating native composite credentials. Likely, this is because you are trying to compose incompatible credentials."); |