Skip to content

Commit 872a9bc

Browse files
[2.1.6] | Fix async deadlock issue when sending attention fails due to network errors (#1767)
1 parent e32dc56 commit 872a9bc

File tree

2 files changed

+24
-22
lines changed

2 files changed

+24
-22
lines changed

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

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public TimeoutState(int value)
5555

5656
private const int AttentionTimeoutSeconds = 5;
5757

58-
private static readonly ContextCallback s_readAdyncCallbackComplete = ReadAsyncCallbackComplete;
58+
private static readonly ContextCallback s_readAsyncCallbackComplete = ReadAsyncCallbackComplete;
5959

6060
// Ticks to consider a connection "good" after a successful I/O (10,000 ticks = 1 ms)
6161
// 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)
23082308
}
23092309
}
23102310

2311-
private bool OnTimeoutSync()
2311+
private bool OnTimeoutSync(bool asyncClose = false)
23122312
{
2313-
return OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredSync);
2313+
return OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredSync, asyncClose);
23142314
}
23152315

23162316
/// <summary>
@@ -2319,8 +2319,9 @@ private bool OnTimeoutSync()
23192319
/// </summary>
23202320
/// <param name="expectedState">the state that is the expected current state, state will change only if this is correct</param>
23212321
/// <param name="targetState">the state that will be changed to if the expected state is correct</param>
2322+
/// <param name="asyncClose">any close action to be taken by an async task to avoid deadlock.</param>
23222323
/// <returns>boolean value indicating whether the call changed the timeout state</returns>
2323-
private bool OnTimeoutCore(int expectedState, int targetState)
2324+
private bool OnTimeoutCore(int expectedState, int targetState, bool asyncClose = false)
23242325
{
23252326
Debug.Assert(targetState == TimeoutState.ExpiredAsync || targetState == TimeoutState.ExpiredSync, "OnTimeoutCore must have an expiry state as the targetState");
23262327

@@ -2353,7 +2354,7 @@ private bool OnTimeoutCore(int expectedState, int targetState)
23532354
{
23542355
try
23552356
{
2356-
SendAttention(mustTakeWriteLock: true);
2357+
SendAttention(mustTakeWriteLock: true, asyncClose);
23572358
}
23582359
catch (Exception e)
23592360
{
@@ -2893,7 +2894,7 @@ public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error)
28932894
// synchrnously and then call OnTimeoutSync to force an atomic change of state.
28942895
if (TimeoutHasExpired)
28952896
{
2896-
OnTimeoutSync();
2897+
OnTimeoutSync(true);
28972898
}
28982899

28992900
// 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)
29242925
{
29252926
if (_executionContext != null)
29262927
{
2927-
ExecutionContext.Run(_executionContext, s_readAdyncCallbackComplete, source);
2928+
ExecutionContext.Run(_executionContext, s_readAsyncCallbackComplete, source);
29282929
}
29292930
else
29302931
{
@@ -3407,7 +3408,7 @@ private void CancelWritePacket()
34073408

34083409
#pragma warning disable 0420 // a reference to a volatile field will not be treated as volatile
34093410

3410-
private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccumulate, bool callerHasConnectionLock)
3411+
private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccumulate, bool callerHasConnectionLock, bool asyncClose = false)
34113412
{
34123413
// Check for a stored exception
34133414
var delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null);
@@ -3500,7 +3501,7 @@ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccu
35003501
{
35013502
SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.WritePacket|Info> write async returned error code {0}", (int)error);
35023503
AddError(_parser.ProcessSNIError(this));
3503-
ThrowExceptionAndWarning();
3504+
ThrowExceptionAndWarning(false, asyncClose);
35043505
}
35053506
AssertValidState();
35063507
completion.SetResult(null);
@@ -3535,7 +3536,7 @@ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccu
35353536
{
35363537
SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.WritePacket|Info> write async returned error code {0}", (int)sniError);
35373538
AddError(_parser.ProcessSNIError(this));
3538-
ThrowExceptionAndWarning(callerHasConnectionLock);
3539+
ThrowExceptionAndWarning(callerHasConnectionLock, asyncClose);
35393540
}
35403541
AssertValidState();
35413542
}
@@ -3547,7 +3548,7 @@ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccu
35473548
internal abstract uint WritePacket(PacketHandle packet, bool sync);
35483549

35493550
// Sends an attention signal - executing thread will consume attn.
3550-
internal void SendAttention(bool mustTakeWriteLock = false)
3551+
internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = false)
35513552
{
35523553
if (!_attentionSent)
35533554
{
@@ -3589,7 +3590,7 @@ internal void SendAttention(bool mustTakeWriteLock = false)
35893590

35903591
uint sniError;
35913592
_parser._asyncWrite = false; // stop async write
3592-
SNIWritePacket(attnPacket, out sniError, canAccumulate: false, callerHasConnectionLock: false);
3593+
SNIWritePacket(attnPacket, out sniError, canAccumulate: false, callerHasConnectionLock: false, asyncClose);
35933594
SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.SendAttention|INFO> Send Attention ASync.");
35943595
}
35953596
finally

src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2390,9 +2390,9 @@ private void OnTimeoutAsync(object state)
23902390
}
23912391
}
23922392

2393-
private bool OnTimeoutSync()
2393+
private bool OnTimeoutSync(bool asyncClose = false)
23942394
{
2395-
return OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredSync);
2395+
return OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredSync, asyncClose);
23962396
}
23972397

23982398
/// <summary>
@@ -2401,8 +2401,9 @@ private bool OnTimeoutSync()
24012401
/// </summary>
24022402
/// <param name="expectedState">the state that is the expected current state, state will change only if this is correct</param>
24032403
/// <param name="targetState">the state that will be changed to if the expected state is correct</param>
2404+
/// <param name="asyncClose">any close action to be taken by an async task to avoid deadlock.</param>
24042405
/// <returns>boolean value indicating whether the call changed the timeout state</returns>
2405-
private bool OnTimeoutCore(int expectedState, int targetState)
2406+
private bool OnTimeoutCore(int expectedState, int targetState, bool asyncClose = false)
24062407
{
24072408
Debug.Assert(targetState == TimeoutState.ExpiredAsync || targetState == TimeoutState.ExpiredSync, "OnTimeoutCore must have an expiry state as the targetState");
24082409

@@ -2436,7 +2437,7 @@ private bool OnTimeoutCore(int expectedState, int targetState)
24362437
{
24372438
try
24382439
{
2439-
SendAttention(mustTakeWriteLock: true);
2440+
SendAttention(mustTakeWriteLock: true, asyncClose);
24402441
}
24412442
catch (Exception e)
24422443
{
@@ -2978,7 +2979,7 @@ public void ReadAsyncCallback(IntPtr key, IntPtr packet, UInt32 error)
29782979
// synchrnously and then call OnTimeoutSync to force an atomic change of state.
29792980
if (TimeoutHasExpired)
29802981
{
2981-
OnTimeoutSync();
2982+
OnTimeoutSync(asyncClose: true);
29822983
}
29832984

29842985
// try to change to the stopped state but only do so if currently in the running state
@@ -3464,7 +3465,7 @@ private void CancelWritePacket()
34643465

34653466
#pragma warning disable 420 // a reference to a volatile field will not be treated as volatile
34663467

3467-
private Task SNIWritePacket(SNIHandle handle, SNIPacket packet, out UInt32 sniError, bool canAccumulate, bool callerHasConnectionLock)
3468+
private Task SNIWritePacket(SNIHandle handle, SNIPacket packet, out UInt32 sniError, bool canAccumulate, bool callerHasConnectionLock, bool asyncClose = false)
34683469
{
34693470
// Check for a stored exception
34703471
var delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null);
@@ -3555,7 +3556,7 @@ private Task SNIWritePacket(SNIHandle handle, SNIPacket packet, out UInt32 sniEr
35553556
SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.WritePacket|Info> write async returned error code {0}", (int)error);
35563557

35573558
AddError(_parser.ProcessSNIError(this));
3558-
ThrowExceptionAndWarning();
3559+
ThrowExceptionAndWarning(false, asyncClose);
35593560
}
35603561
AssertValidState();
35613562
completion.SetResult(null);
@@ -3592,7 +3593,7 @@ private Task SNIWritePacket(SNIHandle handle, SNIPacket packet, out UInt32 sniEr
35923593
{
35933594
SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.WritePacket|Info> write async returned error code {0}", (int)sniError);
35943595
AddError(_parser.ProcessSNIError(this));
3595-
ThrowExceptionAndWarning(callerHasConnectionLock);
3596+
ThrowExceptionAndWarning(callerHasConnectionLock, false);
35963597
}
35973598
AssertValidState();
35983599
}
@@ -3602,7 +3603,7 @@ private Task SNIWritePacket(SNIHandle handle, SNIPacket packet, out UInt32 sniEr
36023603
#pragma warning restore 420
36033604

36043605
// Sends an attention signal - executing thread will consume attn.
3605-
internal void SendAttention(bool mustTakeWriteLock = false)
3606+
internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = false)
36063607
{
36073608
if (!_attentionSent)
36083609
{
@@ -3649,7 +3650,7 @@ internal void SendAttention(bool mustTakeWriteLock = false)
36493650

36503651
UInt32 sniError;
36513652
_parser._asyncWrite = false; // stop async write
3652-
SNIWritePacket(Handle, attnPacket, out sniError, canAccumulate: false, callerHasConnectionLock: false);
3653+
SNIWritePacket(Handle, attnPacket, out sniError, canAccumulate: false, callerHasConnectionLock: false, asyncClose);
36533654
SqlClientEventSource.Log.TryTraceEvent("<sc.TdsParser.SendAttention|{0}> Send Attention ASync.", "Info");
36543655
}
36553656
finally

0 commit comments

Comments
 (0)