remove ValueTask overrides on netcore2.1

dotnet · Mar 14, 2021 · dce70b3 · dce70b3
1 parent 91c53e4
commit dce70b3
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 16 deletions.
diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj
@@ -308,6 +308,7 @@
 		<Compile Include="Microsoft\Data\SqlClient\TdsParser.NetStandard.cs" />
     <Compile Include="Microsoft\Data\SqlClient\SNI\ConcurrentQueueSemaphore.NetStandard.cs" />   
     <Compile Include="Microsoft\Data\SqlClient\SNI\SslOverTdsStream.NetStandard.cs" />
+    <Compile Include="Microsoft\Data\SqlClient\SNI\SNIStreams.Task.cs" />
     <Compile Include="Microsoft\Data\SqlClient\Reliability\SqlConfigurableRetryLogicManager.NetStandard.cs" />
     <Compile Include="Microsoft\Data\SqlClient\SNI\SNIStreams.NetStandard.cs" />
 	</ItemGroup>
@@ -354,7 +355,8 @@
     <Compile Include="Microsoft\Data\SqlClient\Reliability\SqlConfigurableRetryLogicManager.NetCoreApp.cs" />
     <Compile Include="Microsoft\Data\SqlClient\Reliability\SqlAppContextSwitchManager.NetCoreApp.cs" />
     <Compile Include="Microsoft\Data\SqlClient\SNI\ConcurrentQueueSemaphore.NetCoreApp.cs" />
-    <Compile Include="Microsoft\Data\SqlClient\SNI\SNIStreams.NetCoreApp.cs" />
+    <Compile Condition="$(TargetFramework.StartsWith('netcoreapp2.'))" Include="Microsoft\Data\SqlClient\SNI\SNIStreams.Task.cs" />
+    <Compile Condition="!$(TargetFramework.StartsWith('netcoreapp2.'))" Include="Microsoft\Data\SqlClient\SNI\SNIStreams.ValueTask.cs" />
   </ItemGroup>
   <ItemGroup Condition="'$(OSGroup)' != 'AnyOS' AND '$(TargetGroup)' == 'netcoreapp' AND '$(BuildSimulator)' == 'true'">
     <Compile Include="Microsoft\Data\SqlClient\SimulatorEnclaveProvider.NetCoreApp.cs" />
@@ -799,6 +801,7 @@
   </ItemGroup>
   <ItemGroup>
     <Compile Include="Microsoft\Data\SqlClient\AAsyncCallContext.cs" />
+
     <Compile Include="Resources\Strings.Designer.cs">
       <DesignTime>True</DesignTime>
       <AutoGen>True</AutoGen>

diff --git a/...qlClient/netcore/src/Microsoft/Data/SqlClient/SNI/ConcurrentQueueSemaphore.NetStandard.cs b/...qlClient/netcore/src/Microsoft/Data/SqlClient/SNI/ConcurrentQueueSemaphore.NetStandard.cs
@@ -11,14 +11,22 @@ internal sealed partial class ConcurrentQueueSemaphore
     {
         public Task WaitAsync(CancellationToken cancellationToken)
         {
-            var tcs = new TaskCompletionSource<bool>();
-            _queue.Enqueue(tcs);
-            _semaphore.WaitAsync().ContinueWith(
-                continuationAction: s_continuePop,
-                state: _queue,
-                cancellationToken: cancellationToken
-            );
-            return tcs.Task;
+            // try sync wait with 0 which will not block to see if we need to do an async wait
+            if (_semaphore.Wait(0, cancellationToken))
+            {
+                return Task.CompletedTask;
+            }
+            else
+            {
+                var tcs = new TaskCompletionSource<bool>();
+                _queue.Enqueue(tcs);
+                _semaphore.WaitAsync().ContinueWith(
+                    continuationAction: s_continuePop,
+                    state: _queue,
+                    cancellationToken: cancellationToken
+                );
+                return tcs.Task;
+            }
         }
     }
 }
diff --git a/...a/SqlClient/SNI/SNIStreams.NetStandard.cs → ...oft/Data/SqlClient/SNI/SNIStreams.Task.cs b/...a/SqlClient/SNI/SNIStreams.NetStandard.cs → ...oft/Data/SqlClient/SNI/SNIStreams.Task.cs
@@ -2,14 +2,17 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System.Net.Security;
-using System.IO;
 using System.Threading;
 using System.Threading.Tasks;
-using System.Net.Sockets;
+
 
 namespace Microsoft.Data.SqlClient.SNI
 {
+    // NetCore2.1:
+    // DO NOT OVERRIDE ValueTask versions of ReadAsync and WriteAsync because the underlying SslStream implements them
+    // by calling the Task versions which are already overridden meaning that if a caller uses Task WriteAsync this would
+    // call ValueTask WriteAsync which then called TaskWriteAsync introducing a lock cycle and never return
+
     internal sealed partial class SNISslStream
     {
         public override async Task<int> ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)

diff --git a/...ta/SqlClient/SNI/SNIStreams.NetCoreApp.cs → ...ata/SqlClient/SNI/SNIStreams.ValueTask.cs b/...ta/SqlClient/SNI/SNIStreams.NetCoreApp.cs → ...ata/SqlClient/SNI/SNIStreams.ValueTask.cs
@@ -2,16 +2,12 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-using System.Net.Security;
-using System.IO;
 using System.Threading;
 using System.Threading.Tasks;
-using System.Net.Sockets;
 using System;
 
 namespace Microsoft.Data.SqlClient.SNI
 {
-
     internal sealed partial class SNISslStream
     {
         public override Task<int> ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)