Merge pull request #6794 from jtattermusch/csharp_misc_fixes

C# assorted GA improvements (docs and polish)

7 files changed, 42 insertions, 11 deletions

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)