Port CoreFx PR 38271: Fix Statement Command Cancellation (Managed SNI)

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 class SNINpHandle : SNIHandle
 
         private readonly string _targetServer;
         private readonly object _callbackObject;
+        private readonly object _sendSync;
 
         private Stream _stream;
         private NamedPipeClientStream _pipeStream;
@@ -37,6 +39,7 @@ internal class SNINpHandle : SNIHandle
 
         public SNINpHandle(string serverName, string pipeName, long timerExpire, object callbackObject)
         {
+            _sendSync = new object();
             _targetServer = serverName;
             _callbackObject = callbackObject;
 
@@ -193,20 +196,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)
                 {
-                    return ReportErrorAndReleasePacket(packet, ioe);
+                    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)
+                {
+                    Monitor.Exit(this);
                 }
             }
         }

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIPacket.cs

@@ -19,7 +19,6 @@ internal partial class SNIPacket : IDisposable, IEquatable<SNIPacket>
         private int _offset;
         private string _description;
         private SNIAsyncCallback _completionCallback;
-
         private bool _isBufferFromArrayPool;
 
         public SNIPacket() { }
@@ -50,6 +49,11 @@ public string Description
         /// </summary>
         public int DataLeft => (_length - _offset);
 
+        /// <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>

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNITcpHandle.cs

@@ -24,6 +24,7 @@ internal 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
             {
@@ -420,24 +422,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);
                 }
             }
         }

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObjectManaged.cs

@@ -163,6 +163,7 @@ internal override PacketHandle CreateAndSetAttentionPacket()
                 SetPacketData(PacketHandle.FromManagedPacket(attnPacket), SQL.AttentionHeader, TdsEnums.HEADER_LEN);
                 _sniAsyncAttnPacket = attnPacket;
             }
+            PacketHandle.FromManagedPacket(_sniAsyncAttnPacket).ManagedPacket.IsOutOfBand = true;
             return PacketHandle.FromManagedPacket(_sniAsyncAttnPacket);
         }
 

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlCommand/SqlCommandCancelTest.cs

@@ -282,5 +282,65 @@ private static void TimeOutDuringRead(string constr)
                 throw;
             }
         }
+
+        [CheckConnStrSetupFact]
+        public static void CancelDoesNotWait()
+        {
+            const int delaySeconds = 30;
+            const int cancelSeconds = 1;
+
+            using (SqlConnection conn = new SqlConnection(s_connStr))
+            using (var cmd = new SqlCommand($"WAITFOR DELAY '00:00:{delaySeconds:D2}'", conn))
+            {
+                conn.Open();
+
+                Task.Delay(TimeSpan.FromSeconds(cancelSeconds))
+                    .ContinueWith(t => cmd.Cancel());
+
+                DateTime started = DateTime.UtcNow;
+                DateTime ended = default;
+                Exception exception = null;
+                try
+                {
+                    cmd.ExecuteNonQuery();
+                }
+                catch (Exception ex)
+                {
+                    exception = ex;
+                }
+                ended = DateTime.UtcNow;
+
+                Assert.NotNull(exception);
+                Assert.InRange((ended - started).TotalSeconds, cancelSeconds, delaySeconds - 1);
+            }
+        }
+
+        [CheckConnStrSetupFact]
+        public static async Task AsyncCancelDoesNotWait()
+        {
+            const int delaySeconds = 30;
+            const int cancelSeconds = 1;
+
+            using (SqlConnection conn = new SqlConnection(s_connStr))
+            using (var cmd = new SqlCommand($"WAITFOR DELAY '00:00:{delaySeconds:D2}'", conn))
+            {
+                await conn.OpenAsync();
+
+                DateTime started = DateTime.UtcNow;
+                Exception exception = null;
+                try
+                {
+                    await cmd.ExecuteNonQueryAsync(new CancellationTokenSource(1000).Token);
+                }
+                catch (Exception ex)
+                {
+                    exception = ex;
+                }
+                DateTime ended = DateTime.UtcNow;
+
+                Assert.NotNull(exception);
+                Assert.InRange((ended - started).TotalSeconds, cancelSeconds, delaySeconds - 1);
+            }
+        }
     }
 }