From 872a9bc588df4c7c1236dfed0947093eb75ac9f0 Mon Sep 17 00:00:00 2001
From: Cheena Malhotra <13396919+cheenamalhotra@users.noreply.github.com>
Date: Mon, 24 Apr 2023 10:02:05 -0700
Subject: [PATCH] [2.1.6] | Fix async deadlock issue when sending attention
fails due to network errors (#1767)
---
.../Data/SqlClient/TdsParserStateObject.cs | 25 ++++++++++---------
.../Data/SqlClient/TdsParserStateObject.cs | 21 ++++++++--------
2 files changed, 24 insertions(+), 22 deletions(-)
diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs
index b60027db1f..e3228c3d7d 100644
--- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs
+++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs
@@ -55,7 +55,7 @@ public TimeoutState(int value)
private const int AttentionTimeoutSeconds = 5;
- private static readonly ContextCallback s_readAdyncCallbackComplete = ReadAsyncCallbackComplete;
+ private static readonly ContextCallback s_readAsyncCallbackComplete = ReadAsyncCallbackComplete;
// Ticks to consider a connection "good" after a successful I/O (10,000 ticks = 1 ms)
// The resolution of the timer is typically in the range 10 to 16 milliseconds according to msdn.
@@ -2308,9 +2308,9 @@ private void OnTimeoutAsync(object state)
}
}
- private bool OnTimeoutSync()
+ private bool OnTimeoutSync(bool asyncClose = false)
{
- return OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredSync);
+ return OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredSync, asyncClose);
}
///
@@ -2319,8 +2319,9 @@ private bool OnTimeoutSync()
///
/// the state that is the expected current state, state will change only if this is correct
/// the state that will be changed to if the expected state is correct
+ /// any close action to be taken by an async task to avoid deadlock.
/// boolean value indicating whether the call changed the timeout state
- private bool OnTimeoutCore(int expectedState, int targetState)
+ private bool OnTimeoutCore(int expectedState, int targetState, bool asyncClose = false)
{
Debug.Assert(targetState == TimeoutState.ExpiredAsync || targetState == TimeoutState.ExpiredSync, "OnTimeoutCore must have an expiry state as the targetState");
@@ -2353,7 +2354,7 @@ private bool OnTimeoutCore(int expectedState, int targetState)
{
try
{
- SendAttention(mustTakeWriteLock: true);
+ SendAttention(mustTakeWriteLock: true, asyncClose);
}
catch (Exception e)
{
@@ -2893,7 +2894,7 @@ public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error)
// synchrnously and then call OnTimeoutSync to force an atomic change of state.
if (TimeoutHasExpired)
{
- OnTimeoutSync();
+ OnTimeoutSync(true);
}
// try to change to the stopped state but only do so if currently in the running state
@@ -2924,7 +2925,7 @@ public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error)
{
if (_executionContext != null)
{
- ExecutionContext.Run(_executionContext, s_readAdyncCallbackComplete, source);
+ ExecutionContext.Run(_executionContext, s_readAsyncCallbackComplete, source);
}
else
{
@@ -3407,7 +3408,7 @@ private void CancelWritePacket()
#pragma warning disable 0420 // a reference to a volatile field will not be treated as volatile
- private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccumulate, bool callerHasConnectionLock)
+ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccumulate, bool callerHasConnectionLock, bool asyncClose = false)
{
// Check for a stored exception
var delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null);
@@ -3500,7 +3501,7 @@ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccu
{
SqlClientEventSource.Log.TryTraceEvent(" write async returned error code {0}", (int)error);
AddError(_parser.ProcessSNIError(this));
- ThrowExceptionAndWarning();
+ ThrowExceptionAndWarning(false, asyncClose);
}
AssertValidState();
completion.SetResult(null);
@@ -3535,7 +3536,7 @@ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccu
{
SqlClientEventSource.Log.TryTraceEvent(" write async returned error code {0}", (int)sniError);
AddError(_parser.ProcessSNIError(this));
- ThrowExceptionAndWarning(callerHasConnectionLock);
+ ThrowExceptionAndWarning(callerHasConnectionLock, asyncClose);
}
AssertValidState();
}
@@ -3547,7 +3548,7 @@ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccu
internal abstract uint WritePacket(PacketHandle packet, bool sync);
// Sends an attention signal - executing thread will consume attn.
- internal void SendAttention(bool mustTakeWriteLock = false)
+ internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = false)
{
if (!_attentionSent)
{
@@ -3589,7 +3590,7 @@ internal void SendAttention(bool mustTakeWriteLock = false)
uint sniError;
_parser._asyncWrite = false; // stop async write
- SNIWritePacket(attnPacket, out sniError, canAccumulate: false, callerHasConnectionLock: false);
+ SNIWritePacket(attnPacket, out sniError, canAccumulate: false, callerHasConnectionLock: false, asyncClose);
SqlClientEventSource.Log.TryTraceEvent(" Send Attention ASync.");
}
finally
diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs
index 0b2baeb0be..04086edb47 100644
--- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs
+++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs
@@ -2390,9 +2390,9 @@ private void OnTimeoutAsync(object state)
}
}
- private bool OnTimeoutSync()
+ private bool OnTimeoutSync(bool asyncClose = false)
{
- return OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredSync);
+ return OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredSync, asyncClose);
}
///
@@ -2401,8 +2401,9 @@ private bool OnTimeoutSync()
///
/// the state that is the expected current state, state will change only if this is correct
/// the state that will be changed to if the expected state is correct
+ /// any close action to be taken by an async task to avoid deadlock.
/// boolean value indicating whether the call changed the timeout state
- private bool OnTimeoutCore(int expectedState, int targetState)
+ private bool OnTimeoutCore(int expectedState, int targetState, bool asyncClose = false)
{
Debug.Assert(targetState == TimeoutState.ExpiredAsync || targetState == TimeoutState.ExpiredSync, "OnTimeoutCore must have an expiry state as the targetState");
@@ -2436,7 +2437,7 @@ private bool OnTimeoutCore(int expectedState, int targetState)
{
try
{
- SendAttention(mustTakeWriteLock: true);
+ SendAttention(mustTakeWriteLock: true, asyncClose);
}
catch (Exception e)
{
@@ -2978,7 +2979,7 @@ public void ReadAsyncCallback(IntPtr key, IntPtr packet, UInt32 error)
// synchrnously and then call OnTimeoutSync to force an atomic change of state.
if (TimeoutHasExpired)
{
- OnTimeoutSync();
+ OnTimeoutSync(asyncClose: true);
}
// try to change to the stopped state but only do so if currently in the running state
@@ -3464,7 +3465,7 @@ private void CancelWritePacket()
#pragma warning disable 420 // a reference to a volatile field will not be treated as volatile
- private Task SNIWritePacket(SNIHandle handle, SNIPacket packet, out UInt32 sniError, bool canAccumulate, bool callerHasConnectionLock)
+ private Task SNIWritePacket(SNIHandle handle, SNIPacket packet, out UInt32 sniError, bool canAccumulate, bool callerHasConnectionLock, bool asyncClose = false)
{
// Check for a stored exception
var delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null);
@@ -3555,7 +3556,7 @@ private Task SNIWritePacket(SNIHandle handle, SNIPacket packet, out UInt32 sniEr
SqlClientEventSource.Log.TryTraceEvent(" write async returned error code {0}", (int)error);
AddError(_parser.ProcessSNIError(this));
- ThrowExceptionAndWarning();
+ ThrowExceptionAndWarning(false, asyncClose);
}
AssertValidState();
completion.SetResult(null);
@@ -3592,7 +3593,7 @@ private Task SNIWritePacket(SNIHandle handle, SNIPacket packet, out UInt32 sniEr
{
SqlClientEventSource.Log.TryTraceEvent(" write async returned error code {0}", (int)sniError);
AddError(_parser.ProcessSNIError(this));
- ThrowExceptionAndWarning(callerHasConnectionLock);
+ ThrowExceptionAndWarning(callerHasConnectionLock, false);
}
AssertValidState();
}
@@ -3602,7 +3603,7 @@ private Task SNIWritePacket(SNIHandle handle, SNIPacket packet, out UInt32 sniEr
#pragma warning restore 420
// Sends an attention signal - executing thread will consume attn.
- internal void SendAttention(bool mustTakeWriteLock = false)
+ internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = false)
{
if (!_attentionSent)
{
@@ -3649,7 +3650,7 @@ internal void SendAttention(bool mustTakeWriteLock = false)
UInt32 sniError;
_parser._asyncWrite = false; // stop async write
- SNIWritePacket(Handle, attnPacket, out sniError, canAccumulate: false, callerHasConnectionLock: false);
+ SNIWritePacket(Handle, attnPacket, out sniError, canAccumulate: false, callerHasConnectionLock: false, asyncClose);
SqlClientEventSource.Log.TryTraceEvent(" Send Attention ASync.", "Info");
}
finally