Merge pull request dotnet#1159 from roncain/async-close

Fix channel factory async close path to not run synchronously
joshfree · May 19, 2016 · cf483f1 · cf483f1
2 parents 75213b9 + 8030e74
commit cf483f1
Show file tree

Hide file tree

Showing 4 changed files with 266 additions and 2 deletions.
diff --git a/src/System.Private.ServiceModel/src/System/ServiceModel/Channels/ChannelFactoryBase.cs b/src/System.Private.ServiceModel/src/System/ServiceModel/Channels/ChannelFactoryBase.cs
@@ -175,6 +175,11 @@ protected override void OnClose(TimeSpan timeout)
             _channels.Close(timeoutHelper.RemainingTime());
         }
 
+        protected internal override Task OnCloseAsync(TimeSpan timeout)
+        {
+            return OnCloseAsyncInternal(timeout);
+        }
+
         protected override IAsyncResult OnBeginClose(TimeSpan timeout, AsyncCallback callback, object state)
         {
             return new ChainedCloseAsyncResult(timeout, callback, state,
@@ -186,5 +191,25 @@ protected override void OnEndClose(IAsyncResult result)
         {
             ChainedCloseAsyncResult.End(result);
         }
+
+        private async Task OnCloseAsyncInternal(TimeSpan timeout)
+        {
+            IChannel[] currentChannels = _channels.ToArray();
+            TimeoutHelper timeoutHelper = new TimeoutHelper(timeout);
+            foreach (IChannel channel in currentChannels)
+            {
+                IAsyncCommunicationObject iAsyncChannel = channel as IAsyncCommunicationObject;
+                if (iAsyncChannel != null)
+                {
+                    await iAsyncChannel.CloseAsync(timeoutHelper.RemainingTime());
+                }
+                else
+                {
+                    channel.Close(timeoutHelper.RemainingTime());
+                }
+            }
+
+            await Task.Factory.FromAsync(_channels.BeginClose, _channels.EndClose, timeout, TaskCreationOptions.None);
+        }
     }
 }
diff --git a/src/System.Private.ServiceModel/src/System/ServiceModel/Channels/HttpChannelFactory.cs b/src/System.Private.ServiceModel/src/System/ServiceModel/Channels/HttpChannelFactory.cs
@@ -520,8 +520,7 @@ internal protected override Task OnOpenAsync(TimeSpan timeout)
 
         protected internal override Task OnCloseAsync(TimeSpan timeout)
         {
-            OnClose(timeout);
-            return TaskHelpers.CompletedTask();
+            return base.OnCloseAsync(timeout);
         }
 
         protected override void OnClosed()

diff --git a/src/System.Private.ServiceModel/src/System/ServiceModel/Channels/TransportChannelFactory.cs b/src/System.Private.ServiceModel/src/System/ServiceModel/Channels/TransportChannelFactory.cs
@@ -4,6 +4,7 @@
 
 
 using System.Collections.ObjectModel;
+using System.Threading.Tasks;
 
 namespace System.ServiceModel.Channels
 {
@@ -144,6 +145,12 @@ protected override void OnClose(TimeSpan timeout)
             base.OnClose(timeout);
         }
 
+        internal protected override Task OnCloseAsync(TimeSpan timeout)
+        {
+            OnCloseOrAbort();
+            return base.OnCloseAsync(timeout);
+        }
+
         private void OnCloseOrAbort()
         {
             if (_bufferManager != null)

diff --git a/src/System.Private.ServiceModel/tests/Scenarios/Contract/Service/ServiceContractTests.cs b/src/System.Private.ServiceModel/tests/Scenarios/Contract/Service/ServiceContractTests.cs
@@ -672,4 +672,237 @@ public static void ServiceContract_Call_Operation_With_MessageParameterAttribute
             ScenarioTestHelpers.CloseCommunicationObjects((ICommunicationObject)serviceProxy, factory);
         }
     }
+
+    [Fact]
+    [OuterLoop]
+    public static void BasicHttp_Abort_ChannelFactory_Operations_Active()
+    {
+        // Test creates 2 channels from a single channel factory and
+        // aborts the channel factory while both channels are executing
+        // operations.  This verifies the operations are cancelled and
+        // the channel factory is in the correct state.
+        BasicHttpBinding binding = null;
+        TimeSpan delayOperation = TimeSpan.FromSeconds(3);
+        ChannelFactory<IWcfService> factory = null;
+        IWcfService serviceProxy1 = null;
+        IWcfService serviceProxy2 = null;
+
+        try
+        {
+            // *** SETUP *** \\
+            binding = new BasicHttpBinding(BasicHttpSecurityMode.None);
+            binding.CloseTimeout = ScenarioTestHelpers.TestTimeout;
+            factory = new ChannelFactory<IWcfService>(binding, new EndpointAddress(Endpoints.HttpBaseAddress_Basic));
+            serviceProxy1 = factory.CreateChannel();
+            serviceProxy2 = factory.CreateChannel();
+
+            // *** EXECUTE *** \\
+            Task<string> t1 = serviceProxy1.EchoWithTimeoutAsync("first", delayOperation);
+            Task<string> t2 = serviceProxy2.EchoWithTimeoutAsync("second", delayOperation);
+            factory.Abort();
+
+            // *** VALIDATE *** \\
+            Assert.True(factory.State == CommunicationState.Closed,
+                        String.Format("Expected factory state 'Closed', actual was '{0}'", factory.State));
+
+            Assert.Throws<TaskCanceledException>(() => t1.GetAwaiter().GetResult());
+            Assert.Throws<TaskCanceledException>(() => t2.GetAwaiter().GetResult());
+
+            Assert.True(((ICommunicationObject)serviceProxy1).State == CommunicationState.Closed,
+                            String.Format("Expected channel 1 state 'Closed', actual was '{0}'", ((ICommunicationObject)serviceProxy1).State));
+            Assert.True(((ICommunicationObject)serviceProxy2).State == CommunicationState.Closed,
+                String.Format("Expected channel 2 state 'Closed', actual was '{0}'", ((ICommunicationObject)serviceProxy2).State));
+
+            // *** CLEANUP *** \\
+            ((ICommunicationObject)serviceProxy1).Abort();
+            ((ICommunicationObject)serviceProxy2).Abort();
+        }
+        finally
+        {
+            // *** ENSURE CLEANUP *** \\
+            ScenarioTestHelpers.CloseCommunicationObjects((ICommunicationObject)serviceProxy1,
+                                                          (ICommunicationObject)serviceProxy2, 
+                                                          factory);
+        }
+    }
+
+    [Fact]
+    [OuterLoop]
+    public static void BasicHttp_Close_ChannelFactory_Operations_Active()
+    {
+        // Test creates 2 channels from a single channel factory and
+        // closes the channel factory while both channels are executing
+        // operations.  This verifies the operations are cancelled and
+        // the channel factory is in the correct state.
+        BasicHttpBinding binding = null;
+        TimeSpan delayOperation = TimeSpan.FromSeconds(3);
+        ChannelFactory<IWcfService> factory = null;
+        IWcfService serviceProxy1 = null;
+        IWcfService serviceProxy2 = null;
+        string expectedEcho1 = "first";
+        string expectedEcho2 = "second";
+
+        try
+        {
+            // *** SETUP *** \\
+            binding = new BasicHttpBinding(BasicHttpSecurityMode.None);
+            binding.CloseTimeout = ScenarioTestHelpers.TestTimeout;
+            factory = new ChannelFactory<IWcfService>(binding, new EndpointAddress(Endpoints.HttpBaseAddress_Basic));
+            serviceProxy1 = factory.CreateChannel();
+            serviceProxy2 = factory.CreateChannel();
+
+            // *** EXECUTE *** \\
+            Task<string> t1 = serviceProxy1.EchoWithTimeoutAsync(expectedEcho1, delayOperation);
+            Task<string> t2 = serviceProxy2.EchoWithTimeoutAsync(expectedEcho2, delayOperation);
+            factory.Close();
+
+            // *** VALIDATE *** \\
+            Assert.True(factory.State == CommunicationState.Closed,
+                        String.Format("Expected factory state 'Closed', actual was '{0}'", factory.State));
+
+            Exception exception1 = null;
+            Exception exception2 = null;
+            string actualEcho1 = null;
+            string actualEcho2 = null;
+
+            // Verification is slightly more complex for the close with active operations because
+            // we don't know which might have completed first and whether the channel factory
+            // was able to close and dispose either channel before it completed.  So we just
+            // ensure the Tasks complete with an exception or a successful return and have
+            // been closed by the factory.
+            try
+            {
+                actualEcho1 = t1.GetAwaiter().GetResult();
+            }
+            catch (Exception e)
+            {
+                exception1 = e;
+            }
+
+            try
+            {
+                actualEcho2 = t2.GetAwaiter().GetResult();
+            }
+            catch (Exception e)
+            {
+                exception2 = e;
+            }
+
+            Assert.True(exception1 != null || actualEcho1 != null, "First operation should have thrown Exception or returned an echo");
+            Assert.True(exception2 != null || actualEcho2!= null, "Second operation should have thrown Exception or returned an echo");
+
+            Assert.True(actualEcho1 == null || String.Equals(expectedEcho1, actualEcho1),
+                        String.Format("First operation returned '{0}' but expected '{1}'.", expectedEcho1, actualEcho1));
+
+            Assert.True(actualEcho2 == null || String.Equals(expectedEcho2, actualEcho2),
+            String.Format("Second operation returned '{0}' but expected '{1}'.", expectedEcho2, actualEcho2));
+
+            Assert.True(((ICommunicationObject)serviceProxy1).State == CommunicationState.Closed,
+                            String.Format("Expected channel 1 state 'Closed', actual was '{0}'", ((ICommunicationObject)serviceProxy1).State));
+            Assert.True(((ICommunicationObject)serviceProxy2).State == CommunicationState.Closed,
+                String.Format("Expected channel 2 state 'Closed', actual was '{0}'", ((ICommunicationObject)serviceProxy2).State));
+
+            // *** CLEANUP *** \\
+            ((ICommunicationObject)serviceProxy1).Abort();
+            ((ICommunicationObject)serviceProxy2).Abort();
+        }
+        finally
+        {
+            // *** ENSURE CLEANUP *** \\
+            ScenarioTestHelpers.CloseCommunicationObjects((ICommunicationObject)serviceProxy1,
+                                                          (ICommunicationObject)serviceProxy2,
+                                                          factory);
+        }
+    }
+
+    [Fact]
+    [OuterLoop]
+    public static void BasicHttp_Async_Close_ChannelFactory_Operations_Active()
+    {
+        // Test creates 2 channels from a single channel factory and
+        // asynchronously closes the channel factory while both channels are
+        // executing operations.  This verifies the operations are cancelled and
+        // the channel factory is in the correct state.
+        BasicHttpBinding binding = null;
+        TimeSpan delayOperation = TimeSpan.FromSeconds(3);
+        ChannelFactory<IWcfService> factory = null;
+        IWcfService serviceProxy1 = null;
+        IWcfService serviceProxy2 = null;
+        string expectedEcho1 = "first";
+        string expectedEcho2 = "second";
+
+        try
+        {
+            // *** SETUP *** \\
+            binding = new BasicHttpBinding(BasicHttpSecurityMode.None);
+            binding.CloseTimeout = ScenarioTestHelpers.TestTimeout;
+            factory = new ChannelFactory<IWcfService>(binding, new EndpointAddress(Endpoints.HttpBaseAddress_Basic));
+            serviceProxy1 = factory.CreateChannel();
+            serviceProxy2 = factory.CreateChannel();
+
+            // *** EXECUTE *** \\
+            Task<string> t1 = serviceProxy1.EchoWithTimeoutAsync(expectedEcho1, delayOperation);
+            Task<string> t2 = serviceProxy2.EchoWithTimeoutAsync(expectedEcho2, delayOperation);
+            Task factoryTask = Task.Factory.FromAsync(factory.BeginClose, factory.EndClose, TaskCreationOptions.None);
+
+            // *** VALIDATE *** \\
+            factoryTask.GetAwaiter().GetResult();
+            Assert.True(factory.State == CommunicationState.Closed,
+                        String.Format("Expected factory state 'Closed', actual was '{0}'", factory.State));
+
+            Exception exception1 = null;
+            Exception exception2 = null;
+            string actualEcho1 = null;
+            string actualEcho2 = null;
+
+            // Verification is slightly more complex for the close with active operations because
+            // we don't know which might have completed first and whether the channel factory
+            // was able to close and dispose either channel before it completed.  So we just
+            // ensure the Tasks complete with an exception or a successful return and have
+            // been closed by the factory.
+            try
+            {
+                actualEcho1 = t1.GetAwaiter().GetResult();
+            }
+            catch (Exception e)
+            {
+                exception1 = e;
+            }
+
+            try
+            {
+                actualEcho2 = t2.GetAwaiter().GetResult();
+            }
+            catch (Exception e)
+            {
+                exception2 = e;
+            }
+
+            Assert.True(exception1 != null || actualEcho1 != null, "First operation should have thrown Exception or returned an echo");
+            Assert.True(exception2 != null || actualEcho2 != null, "Second operation should have thrown Exception or returned an echo");
+
+            Assert.True(actualEcho1 == null || String.Equals(expectedEcho1, actualEcho1),
+                        String.Format("First operation returned '{0}' but expected '{1}'.", expectedEcho1, actualEcho1));
+
+            Assert.True(actualEcho2 == null || String.Equals(expectedEcho2, actualEcho2),
+            String.Format("Second operation returned '{0}' but expected '{1}'.", expectedEcho2, actualEcho2));
+
+            Assert.True(((ICommunicationObject)serviceProxy1).State == CommunicationState.Closed,
+                            String.Format("Expected channel 1 state 'Closed', actual was '{0}'", ((ICommunicationObject)serviceProxy1).State));
+            Assert.True(((ICommunicationObject)serviceProxy2).State == CommunicationState.Closed,
+                String.Format("Expected channel 2 state 'Closed', actual was '{0}'", ((ICommunicationObject)serviceProxy2).State));
+
+            // *** CLEANUP *** \\
+            ((ICommunicationObject)serviceProxy1).Abort();
+            ((ICommunicationObject)serviceProxy2).Abort();
+        }
+        finally
+        {
+            // *** ENSURE CLEANUP *** \\
+            ScenarioTestHelpers.CloseCommunicationObjects((ICommunicationObject)serviceProxy1,
+                                                          (ICommunicationObject)serviceProxy2,
+                                                          factory);
+        }
+    }
+
 }