Fix | Port CoreFx PR 38271: Fix Statement Command Cancellation (Manag…

…ed SNI) (#248)
dotnet · Jan 8, 2020 · a700c9b · a700c9b
1 parent 02d7383
commit a700c9b
Show file tree

Hide file tree

Showing 5 changed files with 297 additions and 64 deletions.
diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNINpHandle.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNINpHandle.cs
@@ -9,6 +9,7 @@
 using System.Net.Security;
 using System.Security.Authentication;
 using System.Security.Cryptography.X509Certificates;
+using System.Threading;
 
 namespace Microsoft.Data.SqlClient.SNI
 {
@@ -22,6 +23,7 @@ internal sealed class SNINpHandle : SNIHandle
 
         private readonly string _targetServer;
         private readonly object _callbackObject;
+        private readonly object _sendSync;
 
         private Stream _stream;
         private NamedPipeClientStream _pipeStream;
@@ -38,6 +40,7 @@ internal sealed class SNINpHandle : SNIHandle
 
         public SNINpHandle(string serverName, string pipeName, long timerExpire, object callbackObject)
         {
+            _sendSync = new object();
             _targetServer = serverName;
             _callbackObject = callbackObject;
 
@@ -206,20 +209,48 @@ public override uint ReceiveAsync(ref SNIPacket packet)
 
         public override uint Send(SNIPacket packet)
         {
-            lock (this)
+            bool releaseLock = false;
+            try
             {
-                try
+                // is the packet is marked out out-of-band (attention packets only) it must be
+                // sent immediately even if a send of recieve operation is already in progress
+                // because out of band packets are used to cancel ongoing operations
+                // so try to take the lock if possible but continue even if it can't be taken
+                if (packet.IsOutOfBand)
                 {
-                    packet.WriteToStream(_stream);
-                    return TdsEnums.SNI_SUCCESS;
+                    Monitor.TryEnter(this, ref releaseLock);
                 }
-                catch (ObjectDisposedException ode)
+                else
                 {
-                    return ReportErrorAndReleasePacket(packet, ode);
+                    Monitor.Enter(this);
+                    releaseLock = true;
                 }
-                catch (IOException ioe)
+
+                // this lock ensures that two packets are not being written to the transport at the same time
+                // so that sending a standard and an out-of-band packet are both written atomically no data is 
+                // interleaved
+                lock (_sendSync)
+                {
+                    try
+                    {
+                        packet.WriteToStream(_stream);
+                        return TdsEnums.SNI_SUCCESS;
+                    }
+                    catch (ObjectDisposedException ode)
+                    {
+                        return ReportErrorAndReleasePacket(packet, ode);
+                    }
+                    catch (IOException ioe)
+                    {
+                        return ReportErrorAndReleasePacket(packet, ioe);
+                    }
+                }
+            }
+            finally
+            {
+                if (releaseLock)
                 {
-                    return ReportErrorAndReleasePacket(packet, ioe);
+                    Monitor.Exit(this);
                 }
             }
         }

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
@@ -40,6 +40,11 @@ public SNIPacket(int headerSize, int dataSize)
         /// </summary>
         public int DataLeft => (_dataLength - _dataOffset);
 
+        /// <summary>
+        /// Indicates that the packet should be sent out of band bypassing the normal send-recieve lock
+        /// </summary>
+        public bool IsOutOfBand { get; set; }
+
         /// <summary>
         /// Length of data
         /// </summary>

diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNITcpHandle.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNITcpHandle.cs
@@ -24,6 +24,7 @@ internal sealed class SNITCPHandle : SNIHandle
     {
         private readonly string _targetServer;
         private readonly object _callbackObject;
+        private readonly object _sendSync;
         private readonly Socket _socket;
         private NetworkStream _tcpStream;
 
@@ -104,6 +105,7 @@ public SNITCPHandle(string serverName, int port, long timerExpire, object callba
         {
             _callbackObject = callbackObject;
             _targetServer = serverName;
+            _sendSync = new object();
 
             try
             {
@@ -432,24 +434,52 @@ public override void SetBufferSize(int bufferSize)
         /// <returns>SNI error code</returns>
         public override uint Send(SNIPacket packet)
         {
-            lock (this)
+            bool releaseLock = false;
+            try
             {
-                try
+                // is the packet is marked out out-of-band (attention packets only) it must be
+                // sent immediately even if a send of recieve operation is already in progress
+                // because out of band packets are used to cancel ongoing operations
+                // so try to take the lock if possible but continue even if it can't be taken
+                if (packet.IsOutOfBand)
                 {
-                    packet.WriteToStream(_stream);
-                    return TdsEnums.SNI_SUCCESS;
+                    Monitor.TryEnter(this, ref releaseLock);
                 }
-                catch (ObjectDisposedException ode)
+                else
                 {
-                    return ReportTcpSNIError(ode);
+                    Monitor.Enter(this);
+                    releaseLock = true;
                 }
-                catch (SocketException se)
+
+                // this lock ensures that two packets are not being written to the transport at the same time
+                // so that sending a standard and an out-of-band packet are both written atomically no data is 
+                // interleaved
+                lock (_sendSync)
                 {
-                    return ReportTcpSNIError(se);
+                    try
+                    {
+                        packet.WriteToStream(_stream);
+                        return TdsEnums.SNI_SUCCESS;
+                    }
+                    catch (ObjectDisposedException ode)
+                    {
+                        return ReportTcpSNIError(ode);
+                    }
+                    catch (SocketException se)
+                    {
+                        return ReportTcpSNIError(se);
+                    }
+                    catch (IOException ioe)
+                    {
+                        return ReportTcpSNIError(ioe);
+                    }
                 }
-                catch (IOException ioe)
+            }
+            finally
+            {
+                if (releaseLock)
                 {
-                    return ReportTcpSNIError(ioe);
+                    Monitor.Exit(this);
                 }
             }
         }

diff --git a/...rosoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs b/...rosoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs
@@ -137,6 +137,7 @@ internal override PacketHandle CreateAndSetAttentionPacket()
         {
             PacketHandle packetHandle = GetResetWritePacket(TdsEnums.HEADER_LEN);
             SetPacketData(packetHandle, SQL.AttentionHeader, TdsEnums.HEADER_LEN);
+            packetHandle.ManagedPacket.IsOutOfBand = true;
             return packetHandle;
         }