From 816cf4c82e8f6292147499b4685e30030a3f751d Mon Sep 17 00:00:00 2001 From: Cheena Malhotra Date: Wed, 11 Dec 2019 14:01:58 -0800 Subject: [PATCH] Revert Async changes to SNIPacket to fix deadlock issues Fixes #262 by reverting dotnet/corefx#34184 until PR #335 is ready with complete fix. --- .../src/Microsoft.Data.SqlClient.csproj | 2 - .../SqlClient/SNI/SNIPacket.NetCoreApp.cs | 125 ------------------ .../SqlClient/SNI/SNIPacket.NetStandard.cs | 120 ----------------- .../Microsoft/Data/SqlClient/SNI/SNIPacket.cs | 71 +++++++++- 4 files changed, 70 insertions(+), 248 deletions(-) delete mode 100644 src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.NetCoreApp.cs delete mode 100644 src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.NetStandard.cs diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj index b0a0b70040..481308bd83 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft.Data.SqlClient.csproj @@ -44,7 +44,6 @@ - @@ -56,7 +55,6 @@ - diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.NetCoreApp.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.NetCoreApp.cs deleted file mode 100644 index f6fa20907d..0000000000 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.NetCoreApp.cs +++ /dev/null @@ -1,125 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// 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; -using System.IO; -using System.Threading; -using System.Threading.Tasks; - -namespace Microsoft.Data.SqlClient.SNI -{ - internal partial class SNIPacket - { - /// - /// Read data from a stream asynchronously - /// - /// Stream to read from - /// Completion callback - public void ReadFromStreamAsync(Stream stream, SNIAsyncCallback callback) - { - // Treat local function as a static and pass all params otherwise as async will allocate - async Task ReadFromStreamAsync(SNIPacket packet, SNIAsyncCallback cb, ValueTask valueTask) - { - bool error = false; - try - { - packet._dataLength = await valueTask.ConfigureAwait(false); - if (packet._dataLength == 0) - { - SNILoadHandle.SingletonInstance.LastError = new SNIError(SNIProviders.TCP_PROV, 0, SNICommon.ConnTerminatedError, string.Empty); - error = true; - } - } - catch (Exception ex) - { - SNILoadHandle.SingletonInstance.LastError = new SNIError(SNIProviders.TCP_PROV, SNICommon.InternalExceptionError, ex); - error = true; - } - - if (error) - { - packet.Release(); - } - - cb(packet, error ? TdsEnums.SNI_ERROR : TdsEnums.SNI_SUCCESS); - } - - ValueTask vt = stream.ReadAsync(new Memory(_data, _headerLength, _dataCapacity), CancellationToken.None); - - if (vt.IsCompletedSuccessfully) - { - _dataLength = vt.Result; - // Zero length to go via async local function as is error condition - if (_dataLength > 0) - { - callback(this, TdsEnums.SNI_SUCCESS); - - // Completed - return; - } - else - { - // Avoid consuming the same instance twice. - vt = new ValueTask(_dataLength); - } - } - - // Not complete or error call the async local function to complete - _ = ReadFromStreamAsync(this, callback, vt); - } - - /// - /// Write data to a stream asynchronously - /// - /// Stream to write to - /// - /// - /// - public void WriteToStreamAsync(Stream stream, SNIAsyncCallback callback, SNIProviders provider, bool disposeAfterWriteAsync = false) - { - // Treat local function as a static and pass all params otherwise as async will allocate - async Task WriteToStreamAsync(SNIPacket packet, SNIAsyncCallback cb, SNIProviders providers, bool disposeAfter, ValueTask valueTask) - { - uint status = TdsEnums.SNI_SUCCESS; - try - { - await valueTask.ConfigureAwait(false); - } - catch (Exception e) - { - SNILoadHandle.SingletonInstance.LastError = new SNIError(providers, SNICommon.InternalExceptionError, e); - status = TdsEnums.SNI_ERROR; - } - - cb(packet, status); - - if (disposeAfter) - { - packet.Release(); - } - } - - ValueTask vt = stream.WriteAsync(new Memory(_data, _headerLength, _dataLength), CancellationToken.None); - - if (vt.IsCompletedSuccessfully) - { - // Read the result to register as complete for the ValueTask - vt.GetAwaiter().GetResult(); - - callback(this, TdsEnums.SNI_SUCCESS); - - if (disposeAfterWriteAsync) - { - Release(); - } - - // Completed - return; - } - - // Not complete or error call the async local function to complete - _ = WriteToStreamAsync(this, callback, provider, disposeAfterWriteAsync, vt); - } - } -} diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.NetStandard.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.NetStandard.cs deleted file mode 100644 index 68b1ba7313..0000000000 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.NetStandard.cs +++ /dev/null @@ -1,120 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// 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; -using System.IO; -using System.Threading; -using System.Threading.Tasks; - -namespace Microsoft.Data.SqlClient.SNI -{ - internal partial class SNIPacket - { - /// - /// Read data from a stream asynchronously - /// - /// Stream to read from - /// Completion callback - public void ReadFromStreamAsync(Stream stream, SNIAsyncCallback callback) - { - // Treat local function as a static and pass all params otherwise as async will allocate - async Task ReadFromStreamAsync(SNIPacket packet, SNIAsyncCallback cb, Task task) - { - bool error = false; - try - { - packet._dataLength = await task.ConfigureAwait(false); - if (packet._dataLength == 0) - { - SNILoadHandle.SingletonInstance.LastError = new SNIError(SNIProviders.TCP_PROV, 0, SNICommon.ConnTerminatedError, string.Empty); - error = true; - } - } - catch (Exception ex) - { - SNILoadHandle.SingletonInstance.LastError = new SNIError(SNIProviders.TCP_PROV, SNICommon.InternalExceptionError, ex); - error = true; - } - - if (error) - { - packet.Release(); - } - - cb(packet, error ? TdsEnums.SNI_ERROR : TdsEnums.SNI_SUCCESS); - } - - Task t = stream.ReadAsync(_data, _headerLength, _dataCapacity, CancellationToken.None); - - if ((t.Status & TaskStatus.RanToCompletion) != 0) - { - _dataLength = t.Result; - // Zero length to go via async local function as is error condition - if (_dataLength > 0) - { - callback(this, TdsEnums.SNI_SUCCESS); - - // Completed - return; - } - } - - // Not complete or error call the async local function to complete - _ = ReadFromStreamAsync(this, callback, t); - } - - /// - /// Write data to a stream asynchronously - /// - /// Stream to write to - /// - /// - /// - public void WriteToStreamAsync(Stream stream, SNIAsyncCallback callback, SNIProviders provider, bool disposeAfterWriteAsync = false) - { - // Treat local function as a static and pass all params otherwise as async will allocate - async Task WriteToStreamAsync(SNIPacket packet, SNIAsyncCallback cb, SNIProviders providers, bool disposeAfter, Task task) - { - uint status = TdsEnums.SNI_SUCCESS; - try - { - await task.ConfigureAwait(false); - } - catch (Exception e) - { - SNILoadHandle.SingletonInstance.LastError = new SNIError(providers, SNICommon.InternalExceptionError, e); - status = TdsEnums.SNI_ERROR; - } - - cb(packet, status); - - if (disposeAfter) - { - packet.Release(); - } - } - - Task t = stream.WriteAsync(_data, _headerLength, _dataLength, CancellationToken.None); - - if ((t.Status & TaskStatus.RanToCompletion) != 0) - { - // Read the result to register as complete for the Task - t.GetAwaiter().GetResult(); - - callback(this, TdsEnums.SNI_SUCCESS); - - if (disposeAfterWriteAsync) - { - Release(); - } - - // Completed - return; - } - - // Not complete or error call the async local function to complete - _ = WriteToStreamAsync(this, callback, provider, disposeAfterWriteAsync, t); - } - } -} diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.cs index 0ff733d26a..e852a38be7 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.cs @@ -6,13 +6,15 @@ using System.Buffers; using System.Diagnostics; using System.IO; +using System.Threading; +using System.Threading.Tasks; namespace Microsoft.Data.SqlClient.SNI { /// /// SNI Packet /// - internal sealed partial class SNIPacket + internal sealed class SNIPacket { private int _dataLength; // the length of the data in the data segment, advanced by Append-ing data, does not include smux header length private int _dataCapacity; // the total capacity requested, if the array is rented this may be less than the _data.Length, does not include smux header length @@ -185,6 +187,46 @@ public void ReadFromStream(Stream stream) _dataLength = stream.Read(_data, _headerLength, _dataCapacity); } + /// + /// Read data from a stream asynchronously + /// + /// Stream to read from + /// Completion callback + public void ReadFromStreamAsync(Stream stream, SNIAsyncCallback callback) + { + bool error = false; + + stream.ReadAsync(_data, 0, _dataCapacity, CancellationToken.None).ContinueWith(t => + { + Exception e = t.Exception?.InnerException; + if (e != null) + { + SNILoadHandle.SingletonInstance.LastError = new SNIError(SNIProviders.TCP_PROV, SNICommon.InternalExceptionError, e); + error = true; + } + else + { + _dataLength = t.Result; + + if (_dataLength == 0) + { + SNILoadHandle.SingletonInstance.LastError = new SNIError(SNIProviders.TCP_PROV, 0, SNICommon.ConnTerminatedError, string.Empty); + error = true; + } + } + + if (error) + { + Release(); + } + + callback(this, error ? TdsEnums.SNI_ERROR : TdsEnums.SNI_SUCCESS); + }, + CancellationToken.None, + TaskContinuationOptions.DenyChildAttach, + TaskScheduler.Default); + } + /// /// Write data to a stream synchronously /// @@ -194,6 +236,33 @@ public void WriteToStream(Stream stream) stream.Write(_data, _headerLength, _dataLength); } + /// + /// Write data to a stream asynchronously + /// + /// Stream to write to + /// SNI Asynchronous Callback + /// SNI provider identifier + /// Bool flag to decide whether or not to dispose after Write Async operation + public async void WriteToStreamAsync(Stream stream, SNIAsyncCallback callback, SNIProviders provider, bool disposeAfterWriteAsync = false) + { + uint status = TdsEnums.SNI_SUCCESS; + try + { + await stream.WriteAsync(_data, 0, _dataLength, CancellationToken.None).ConfigureAwait(false); + } + catch (Exception e) + { + SNILoadHandle.SingletonInstance.LastError = new SNIError(provider, SNICommon.InternalExceptionError, e); + status = TdsEnums.SNI_ERROR; + } + callback(this, status); + + if (disposeAfterWriteAsync) + { + Dispose(); + } + } + /// /// Get hash code ///