Merge pull request #6794 from jtattermusch/csharp_misc_fixes

C# assorted GA improvements (docs and polish)

10 files changed, 61 insertions, 30 deletions

diff --git a/src/csharp/Grpc.Core.Tests/ChannelTest.cs b/src/csharp/Grpc.Core.Tests/ChannelTest.cs
index 850d70ce92..db0ef3a4cd 100644
--- a/src/csharp/Grpc.Core.Tests/ChannelTest.cs
+++ b/src/csharp/Grpc.Core.Tests/ChannelTest.cs
@@ -71,7 +71,7 @@ namespace Grpc.Core.Tests
         public void WaitForStateChangedAsync_InvalidArgument()
         {
             var channel = new Channel("localhost", ChannelCredentials.Insecure);
-            Assert.ThrowsAsync(typeof(ArgumentException), async () => await channel.WaitForStateChangedAsync(ChannelState.FatalFailure));
+            Assert.ThrowsAsync(typeof(ArgumentException), async () => await channel.WaitForStateChangedAsync(ChannelState.Shutdown));
             channel.ShutdownAsync().Wait();
         }
 
@@ -102,11 +102,11 @@ namespace Grpc.Core.Tests
         }
 
         [Test]
-        public async Task StateIsFatalFailureAfterShutdown()
+        public async Task StateIsShutdownAfterShutdown()
         {
             var channel = new Channel("localhost", ChannelCredentials.Insecure);
             await channel.ShutdownAsync();
-            Assert.AreEqual(ChannelState.FatalFailure, channel.State);
+            Assert.AreEqual(ChannelState.Shutdown, channel.State);
         }
 
         [Test]
diff --git a/src/csharp/Grpc.Core/AsyncDuplexStreamingCall.cs b/src/csharp/Grpc.Core/AsyncDuplexStreamingCall.cs
index e75108c7e5..68fd6d0b05 100644
--- a/src/csharp/Grpc.Core/AsyncDuplexStreamingCall.cs
+++ b/src/csharp/Grpc.Core/AsyncDuplexStreamingCall.cs
@@ -117,6 +117,10 @@ namespace Grpc.Core
         /// Otherwise, requests cancellation of the call which should terminate all pending async operations associated with the call.
         /// As a result, all resources being used by the call should be released eventually.
         /// </summary>
+        /// <remarks>
+        /// Normally, there is no need for you to dispose the call unless you want to utilize the
+        /// "Cancel" semantics of invoking <c>Dispose</c>.
+        /// </remarks>
         public void Dispose()
         {
             disposeAction.Invoke();
diff --git a/src/csharp/Grpc.Core/AsyncServerStreamingCall.cs b/src/csharp/Grpc.Core/AsyncServerStreamingCall.cs
index f953091984..5777c72615 100644
--- a/src/csharp/Grpc.Core/AsyncServerStreamingCall.cs
+++ b/src/csharp/Grpc.Core/AsyncServerStreamingCall.cs
@@ -103,6 +103,10 @@ namespace Grpc.Core
         /// Otherwise, requests cancellation of the call which should terminate all pending async operations associated with the call.
         /// As a result, all resources being used by the call should be released eventually.
         /// </summary>
+        /// <remarks>
+        /// Normally, there is no need for you to dispose the call unless you want to utilize the
+        /// "Cancel" semantics of invoking <c>Dispose</c>.
+        /// </remarks>
         public void Dispose()
         {
             disposeAction.Invoke();
diff --git a/src/csharp/Grpc.Core/AsyncUnaryCall.cs b/src/csharp/Grpc.Core/AsyncUnaryCall.cs
index 97df8f5e91..d180c27922 100644
--- a/src/csharp/Grpc.Core/AsyncUnaryCall.cs
+++ b/src/csharp/Grpc.Core/AsyncUnaryCall.cs
@@ -112,6 +112,10 @@ namespace Grpc.Core
         /// Otherwise, requests cancellation of the call which should terminate all pending async operations associated with the call.
         /// As a result, all resources being used by the call should be released eventually.
         /// </summary>
+        /// <remarks>
+        /// Normally, there is no need for you to dispose the call unless you want to utilize the
+        /// "Cancel" semantics of invoking <c>Dispose</c>.
+        /// </remarks>
         public void Dispose()
         {
             disposeAction.Invoke();
diff --git a/src/csharp/Grpc.Core/CallOptions.cs b/src/csharp/Grpc.Core/CallOptions.cs
index 9ca88849ee..35548cfc96 100644
--- a/src/csharp/Grpc.Core/CallOptions.cs
+++ b/src/csharp/Grpc.Core/CallOptions.cs
@@ -88,7 +88,13 @@ namespace Grpc.Core
         }
 
         /// <summary>
-        /// Token that can be used for cancelling the call.
+        /// Token that can be used for cancelling the call on the client side.
+        /// Cancelling the token will request cancellation
+        /// of the remote call. Best effort will be made to deliver the cancellation
+        /// notification to the server and interaction of the call with the server side
+        /// will be terminated. Unless the call finishes before the cancellation could
+        /// happen (there is an inherent race),
+        /// the call will finish with <c>StatusCode.Cancelled</c> status.
         /// </summary>
         public CancellationToken CancellationToken
         {
diff --git a/src/csharp/Grpc.Core/Channel.cs b/src/csharp/Grpc.Core/Channel.cs
index 886adfec33..9cee752663 100644
--- a/src/csharp/Grpc.Core/Channel.cs
+++ b/src/csharp/Grpc.Core/Channel.cs
@@ -104,7 +104,7 @@ namespace Grpc.Core
 
         /// <summary>
         /// Gets current connectivity state of this channel.
-        /// After channel is has been shutdown, <c>ChannelState.FatalFailure</c> will be returned.
+        /// After channel is has been shutdown, <c>ChannelState.Shutdown</c> will be returned.
         /// </summary>
         public ChannelState State
         {
@@ -121,8 +121,8 @@ namespace Grpc.Core
         /// </summary>
         public Task WaitForStateChangedAsync(ChannelState lastObservedState, DateTime? deadline = null)
         {
-            GrpcPreconditions.CheckArgument(lastObservedState != ChannelState.FatalFailure,
-                "FatalFailure is a terminal state. No further state changes can occur.");
+            GrpcPreconditions.CheckArgument(lastObservedState != ChannelState.Shutdown,
+                "Shutdown is a terminal state. No further state changes can occur.");
             var tcs = new TaskCompletionSource<object>();
             var deadlineTimespec = deadline.HasValue ? Timespec.FromDateTime(deadline.Value) : Timespec.InfFuture;
             var handler = new BatchCompletionDelegate((success, ctx) =>
@@ -172,7 +172,7 @@ namespace Grpc.Core
         /// <summary>
         /// Allows explicitly requesting channel to connect without starting an RPC.
         /// Returned task completes once state Ready was seen. If the deadline is reached,
-        /// or channel enters the FatalFailure state, the task is cancelled.
+        /// or channel enters the Shutdown state, the task is cancelled.
         /// There is no need to call this explicitly unless your use case requires that.
         /// Starting an RPC on a new channel will request connection implicitly.
         /// </summary>
@@ -182,9 +182,9 @@ namespace Grpc.Core
             var currentState = GetConnectivityState(true);
             while (currentState != ChannelState.Ready)
             {
-                if (currentState == ChannelState.FatalFailure)
+                if (currentState == ChannelState.Shutdown)
                 {
-                    throw new OperationCanceledException("Channel has reached FatalFailure state.");
+                    throw new OperationCanceledException("Channel has reached Shutdown state.");
                 }
                 await WaitForStateChangedAsync(currentState, deadline).ConfigureAwait(false);
                 currentState = GetConnectivityState(false);
@@ -192,9 +192,16 @@ namespace Grpc.Core
         }
 
         /// <summary>
-        /// Waits until there are no more active calls for this channel and then cleans up
-        /// resources used by this channel.
+        /// Shuts down the channel cleanly. It is strongly recommended to shutdown
+        /// all previously created channels before exiting from the process.
         /// </summary>
+        /// <remarks>
+        /// This method doesn't wait for all calls on this channel to finish (nor does
+        /// it explicitly cancel all outstanding calls). It is user's responsibility to make sure
+        /// all the calls on this channel have finished (successfully or with an error)
+        /// before shutting down the channel to ensure channel shutdown won't impact
+        /// the outcome of those remote calls.
+        /// </remarks>
         public async Task ShutdownAsync()
         {
             lock (myLock)
@@ -264,7 +271,7 @@ namespace Grpc.Core
             }
             catch (ObjectDisposedException)
             {
-                return ChannelState.FatalFailure;
+                return ChannelState.Shutdown;
             }
         }
 
diff --git a/src/csharp/Grpc.Core/ChannelState.cs b/src/csharp/Grpc.Core/ChannelState.cs
index d293b98f75..a6c3b2a488 100644
--- a/src/csharp/Grpc.Core/ChannelState.cs
+++ b/src/csharp/Grpc.Core/ChannelState.cs
@@ -64,6 +64,6 @@ namespace Grpc.Core
         /// <summary>
         /// Channel has seen a failure that it cannot recover from
         /// </summary>
-        FatalFailure
+        Shutdown
     }
 }
diff --git a/src/csharp/Grpc.Core/Server.cs b/src/csharp/Grpc.Core/Server.cs
index 069185e13a..6bd7900561 100644
--- a/src/csharp/Grpc.Core/Server.cs
+++ b/src/csharp/Grpc.Core/Server.cs
@@ -152,6 +152,9 @@ namespace Grpc.Core
         /// cleans up used resources. The returned task finishes when shutdown procedure
         /// is complete.
         /// </summary>
+        /// <remarks>
+        /// It is strongly recommended to shutdown all previously created servers before exiting from the process.
+        /// </remarks>
         public async Task ShutdownAsync()
         {
             lock (myLock)
@@ -173,6 +176,9 @@ namespace Grpc.Core
         /// Requests server shutdown while cancelling all the in-progress calls.
         /// The returned task finishes when shutdown procedure is complete.
         /// </summary>
+        /// <remarks>
+        /// It is strongly recommended to shutdown all previously created servers before exiting from the process.
+        /// </remarks>
         public async Task KillAsync()
         {
             lock (myLock)
diff --git a/src/csharp/Grpc.Examples/MathGrpc.cs b/src/csharp/Grpc.Examples/MathGrpc.cs
index 9319e311ee..4bbefcbe01 100644
--- a/src/csharp/Grpc.Examples/MathGrpc.cs
+++ b/src/csharp/Grpc.Examples/MathGrpc.cs
@@ -85,8 +85,8 @@ namespace Math {
     public abstract class MathBase
     {
       /// <summary>
-      ///  Div divides args.dividend by args.divisor and returns the quotient and
-      ///  remainder.
+      ///  Div divides DivArgs.dividend by DivArgs.divisor and returns the quotient
+      ///  and remainder.
       /// </summary>
       public virtual global::System.Threading.Tasks.Task<global::Math.DivReply> Div(global::Math.DivArgs request, ServerCallContext context)
       {
@@ -105,7 +105,7 @@ namespace Math {
       }
 
       /// <summary>
-      ///  Fib generates numbers in the Fibonacci sequence.  If args.limit > 0, Fib
+      ///  Fib generates numbers in the Fibonacci sequence.  If FibArgs.limit > 0, Fib
       ///  generates up to limit numbers; otherwise it continues until the call is
       ///  canceled.  Unlike Fib above, Fib has no final FibReply.
       /// </summary>
@@ -144,32 +144,32 @@ namespace Math {
       }
 
       /// <summary>
-      ///  Div divides args.dividend by args.divisor and returns the quotient and
-      ///  remainder.
+      ///  Div divides DivArgs.dividend by DivArgs.divisor and returns the quotient
+      ///  and remainder.
       /// </summary>
       public virtual global::Math.DivReply Div(global::Math.DivArgs request, Metadata headers = null, DateTime? deadline = null, CancellationToken cancellationToken = default(CancellationToken))
       {
         return Div(request, new CallOptions(headers, deadline, cancellationToken));
       }
       /// <summary>
-      ///  Div divides args.dividend by args.divisor and returns the quotient and
-      ///  remainder.
+      ///  Div divides DivArgs.dividend by DivArgs.divisor and returns the quotient
+      ///  and remainder.
       /// </summary>
       public virtual global::Math.DivReply Div(global::Math.DivArgs request, CallOptions options)
       {
         return CallInvoker.BlockingUnaryCall(__Method_Div, null, options, request);
       }
       /// <summary>
-      ///  Div divides args.dividend by args.divisor and returns the quotient and
-      ///  remainder.
+      ///  Div divides DivArgs.dividend by DivArgs.divisor and returns the quotient
+      ///  and remainder.
       /// </summary>
       public virtual AsyncUnaryCall<global::Math.DivReply> DivAsync(global::Math.DivArgs request, Metadata headers = null, DateTime? deadline = null, CancellationToken cancellationToken = default(CancellationToken))
       {
         return DivAsync(request, new CallOptions(headers, deadline, cancellationToken));
       }
       /// <summary>
-      ///  Div divides args.dividend by args.divisor and returns the quotient and
-      ///  remainder.
+      ///  Div divides DivArgs.dividend by DivArgs.divisor and returns the quotient
+      ///  and remainder.
       /// </summary>
       public virtual AsyncUnaryCall<global::Math.DivReply> DivAsync(global::Math.DivArgs request, CallOptions options)
       {
@@ -196,7 +196,7 @@ namespace Math {
         return CallInvoker.AsyncDuplexStreamingCall(__Method_DivMany, null, options);
       }
       /// <summary>
-      ///  Fib generates numbers in the Fibonacci sequence.  If args.limit > 0, Fib
+      ///  Fib generates numbers in the Fibonacci sequence.  If FibArgs.limit > 0, Fib
       ///  generates up to limit numbers; otherwise it continues until the call is
       ///  canceled.  Unlike Fib above, Fib has no final FibReply.
       /// </summary>
@@ -205,7 +205,7 @@ namespace Math {
         return Fib(request, new CallOptions(headers, deadline, cancellationToken));
       }
       /// <summary>
-      ///  Fib generates numbers in the Fibonacci sequence.  If args.limit > 0, Fib
+      ///  Fib generates numbers in the Fibonacci sequence.  If FibArgs.limit > 0, Fib
       ///  generates up to limit numbers; otherwise it continues until the call is
       ///  canceled.  Unlike Fib above, Fib has no final FibReply.
       /// </summary>
diff --git a/src/proto/math/math.proto b/src/proto/math/math.proto
index 311e148c02..269c60bde8 100644
--- a/src/proto/math/math.proto
+++ b/src/proto/math/math.proto
@@ -55,8 +55,8 @@ message FibReply {
 }
 
 service Math {
-  // Div divides args.dividend by args.divisor and returns the quotient and
-  // remainder.
+  // Div divides DivArgs.dividend by DivArgs.divisor and returns the quotient
+  // and remainder.
   rpc Div (DivArgs) returns (DivReply) {
   }
 
@@ -67,7 +67,7 @@ service Math {
   rpc DivMany (stream DivArgs) returns (stream DivReply) {
   }
 
-  // Fib generates numbers in the Fibonacci sequence.  If args.limit > 0, Fib
+  // Fib generates numbers in the Fibonacci sequence.  If FibArgs.limit > 0, Fib
   // generates up to limit numbers; otherwise it continues until the call is
   // canceled.  Unlike Fib above, Fib has no final FibReply.
   rpc Fib (FibArgs) returns (stream Num) {