Implement IAsyncDisposable and remove ChainedAsyncResult as it's buggy

dotnet · Jul 29, 2022 · fb66b1e · fb66b1e
1 parent 1f11516
commit fb66b1e
Show file tree

Hide file tree

Showing 18 changed files with 788 additions and 746 deletions.
diff --git a/src/System.Private.ServiceModel/src/System/ServiceModel/ChannelFactory.cs b/src/System.Private.ServiceModel/src/System/ServiceModel/ChannelFactory.cs
@@ -12,7 +12,7 @@
 
 namespace System.ServiceModel
 {
-    public abstract class ChannelFactory : CommunicationObject, IChannelFactory, IDisposable
+    public abstract class ChannelFactory : CommunicationObject, IChannelFactory, IDisposable, IAsyncDisposable
     {
         private string _configurationName;
         private ClientCredentials _readOnlyClientCredentials;
@@ -164,6 +164,31 @@ void IDisposable.Dispose()
             Close();
         }
 
+        async ValueTask IAsyncDisposable.DisposeAsync()
+        {
+            try
+            {
+                // Only want to call Close if it is in the Opened state
+                if (State == CommunicationState.Opened)
+                {
+                    await ((IAsyncCommunicationObject)this).CloseAsync(DefaultCloseTimeout);
+                }
+                // Anything not closed by this point should be aborted
+                if (State != CommunicationState.Closed)
+                {
+                    Abort();
+                }
+            }
+            catch (CommunicationException)
+            {
+                Abort();
+            }
+            catch (TimeoutException)
+            {
+                Abort();
+            }
+        }
+
         private void EnsureSecurityCredentialsManager(ServiceEndpoint endpoint)
         {
             Fx.Assert(State == CommunicationState.Created || State == CommunicationState.Opening, "");

diff --git a/src/System.Private.ServiceModel/src/System/ServiceModel/Channels/ChainedAsyncResult.cs b/src/System.Private.ServiceModel/src/System/ServiceModel/Channels/ChainedAsyncResult.cs
diff --git a/src/System.Private.ServiceModel/src/System/ServiceModel/Channels/ChannelFactoryBase.cs b/src/System.Private.ServiceModel/src/System/ServiceModel/Channels/ChannelFactoryBase.cs
@@ -194,14 +194,12 @@ protected internal override Task OnCloseAsync(TimeSpan timeout)
 
         protected override IAsyncResult OnBeginClose(TimeSpan timeout, AsyncCallback callback, object state)
         {
-            return new ChainedCloseAsyncResult(timeout, callback, state,
-                _channels.BeginClose, _channels.EndClose,
-                _channels.ToArray());
+            return OnCloseAsync(timeout).ToApm(callback, state);
         }
 
         protected override void OnEndClose(IAsyncResult result)
         {
-            ChainedCloseAsyncResult.End(result);
+            result.ToApmEnd();
         }
 
         private async Task OnCloseAsyncInternal(TimeSpan timeout)
@@ -214,15 +212,10 @@ private async Task OnCloseAsyncInternal(TimeSpan timeout)
             }
 
             // CommunicationObjectManager (_channels) is not a CommunicationObject,
-            // so just choose existing synchronous or asynchronous close
-            if (_isSynchronousClose)
-            {
-                await TaskHelpers.CallActionAsync(_channels.Close, timeoutHelper.RemainingTime());
-            }
-            else
-            {
-                await Task.Factory.FromAsync(_channels.BeginClose, _channels.EndClose, timeoutHelper.RemainingTime(), TaskCreationOptions.None);
-            }
+            // and it's close method waits for it to not be busy. Calling CloseAsync
+            // is the correct option here as there's no need to block this thread
+            // waiting on a signal from another thread to notify it's no longer busy.
+            await _channels.CloseAsync(timeoutHelper.RemainingTime());
         }
     }
 }
diff --git a/src/System.Private.ServiceModel/src/System/ServiceModel/Channels/LayeredChannelFactory.cs b/src/System.Private.ServiceModel/src/System/ServiceModel/Channels/LayeredChannelFactory.cs
@@ -46,12 +46,12 @@ protected override void OnEndOpen(IAsyncResult result)
 
         protected override IAsyncResult OnBeginClose(TimeSpan timeout, AsyncCallback callback, object state)
         {
-            return new ChainedCloseAsyncResult(timeout, callback, state, base.OnBeginClose, base.OnEndClose, InnerChannelFactory);
+            return OnCloseAsync(timeout).ToApm(callback, state);
         }
 
         protected override void OnEndClose(IAsyncResult result)
         {
-            ChainedCloseAsyncResult.End(result);
+            result.ToApmEnd();
         }
 
         protected internal override async Task OnCloseAsync(TimeSpan timeout)