Skip to content

Commit c489f19

Browse files
Revert change that made SqlDataReader.ReadAsync() non-blocking (#547)
1 parent 0af9f32 commit c489f19

File tree

3 files changed

+108
-4
lines changed

3 files changed

+108
-4
lines changed

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2846,6 +2846,12 @@ private bool TryProcessDone(SqlCommand cmd, SqlDataReader reader, ref RunBehavio
28462846
ushort status;
28472847
int count;
28482848

2849+
// This is added back since removing it from here introduces regressions in Managed SNI.
2850+
// It forces SqlDataReader.ReadAsync() method to run synchronously,
2851+
// and will block the calling thread until data is fed from SQL Server.
2852+
// TODO Investigate better solution to support non-blocking ReadAsync().
2853+
stateObj._syncOverAsync = true;
2854+
28492855
// status
28502856
// command
28512857
// rowcount (valid only if DONE_COUNT bit is set)
@@ -2945,11 +2951,10 @@ private bool TryProcessDone(SqlCommand cmd, SqlDataReader reader, ref RunBehavio
29452951
}
29462952
}
29472953

2948-
// _attentionSent set by 'SendAttention'
29492954
// _pendingData set by e.g. 'TdsExecuteSQLBatch'
29502955
// _hasOpenResult always set to true by 'WriteMarsHeader'
29512956
//
2952-
if (!stateObj._attentionSent && !stateObj.HasPendingData && stateObj.HasOpenResult)
2957+
if (!stateObj.HasPendingData && stateObj.HasOpenResult)
29532958
{
29542959
/*
29552960
Debug.Assert(!((sqlTransaction != null && _distributedTransaction != null) ||

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3336,11 +3336,10 @@ private bool TryProcessDone(SqlCommand cmd, SqlDataReader reader, ref RunBehavio
33363336
}
33373337
}
33383338

3339-
// _attentionSent set by 'SendAttention'
33403339
// _pendingData set by e.g. 'TdsExecuteSQLBatch'
33413340
// _hasOpenResult always set to true by 'WriteMarsHeader'
33423341
//
3343-
if (!stateObj._attentionSent && !stateObj._pendingData && stateObj._hasOpenResult)
3342+
if (!stateObj._pendingData && stateObj._hasOpenResult)
33443343
{
33453344
/*
33463345
Debug.Assert(!((sqlTransaction != null && _distributedTransaction != null) ||

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

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,106 @@ public static void AsyncCancelDoesNotWaitNP()
212212
AsyncCancelDoesNotWait(np_connStr).Wait();
213213
}
214214

215+
[CheckConnStrSetupFact]
216+
public static void TCPAttentionPacketTestTransaction()
217+
{
218+
CancelFollowedByTransaction(tcp_connStr);
219+
}
220+
221+
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))]
222+
[PlatformSpecific(TestPlatforms.Windows)]
223+
public static void NPAttentionPacketTestTransaction()
224+
{
225+
CancelFollowedByTransaction(np_connStr);
226+
}
227+
228+
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))]
229+
public static void TCPAttentionPacketTestAlerts()
230+
{
231+
CancelFollowedByAlert(tcp_connStr);
232+
}
233+
234+
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))]
235+
[PlatformSpecific(TestPlatforms.Windows)]
236+
public static void NPAttentionPacketTestAlerts()
237+
{
238+
CancelFollowedByAlert(np_connStr);
239+
}
240+
241+
private static void CancelFollowedByTransaction(string constr)
242+
{
243+
using (SqlConnection connection = new SqlConnection(constr))
244+
{
245+
connection.Open();
246+
using (SqlCommand cmd = connection.CreateCommand())
247+
{
248+
cmd.CommandText = @"SELECT @@VERSION";
249+
using (var r = cmd.ExecuteReader())
250+
{
251+
cmd.Cancel();
252+
}
253+
}
254+
using (SqlTransaction transaction = connection.BeginTransaction())
255+
{ }
256+
}
257+
}
258+
259+
private static void CancelFollowedByAlert(string constr)
260+
{
261+
var alertName = "myAlert" + Guid.NewGuid().ToString();
262+
// Since Alert conditions are randomly generated,
263+
// we will retry on unexpected error messages to avoid collision in pipelines.
264+
var n = new Random().Next(1, 100);
265+
bool retry = true;
266+
int retryAttempt = 0;
267+
while (retry && retryAttempt < 3)
268+
{
269+
try
270+
{
271+
using (var conn = new SqlConnection(constr))
272+
{
273+
conn.Open();
274+
using (SqlCommand cmd = conn.CreateCommand())
275+
{
276+
cmd.CommandText = "SELECT @@VERSION";
277+
using (var reader = cmd.ExecuteReader())
278+
{
279+
cmd.Cancel(); // Sends Attention
280+
}
281+
}
282+
using (SqlCommand cmd = conn.CreateCommand())
283+
{
284+
cmd.CommandText = $@"EXEC msdb.dbo.sp_add_alert @name=N'{alertName}',
285+
@performance_condition = N'SQLServer:General Statistics|User Connections||>|{n}'";
286+
cmd.ExecuteNonQuery();
287+
cmd.CommandText = @"USE [msdb]";
288+
cmd.ExecuteNonQuery();
289+
cmd.CommandText = $@"/****** Object: Alert [{alertName}] Script Date: {DateTime.Now} ******/
290+
IF EXISTS (SELECT name FROM msdb.dbo.sysalerts WHERE name = N'{alertName}')
291+
EXEC msdb.dbo.sp_delete_alert @name=N'{alertName}'";
292+
cmd.ExecuteNonQuery();
293+
}
294+
}
295+
}
296+
catch (Exception e)
297+
{
298+
if (retryAttempt >= 3 || e.Message.Contains("The transaction operation cannot be performed"))
299+
{
300+
Assert.False(true, $"Retry Attempt: {retryAttempt} | Unexpected Exception occurred: {e.Message}");
301+
}
302+
else
303+
{
304+
retry = true;
305+
retryAttempt++;
306+
Console.WriteLine($"CancelFollowedByAlert Test retry attempt : {retryAttempt}");
307+
Thread.Sleep(500);
308+
continue;
309+
}
310+
}
311+
retry = false;
312+
}
313+
}
314+
215315
private static void MultiThreadedCancel(string constr, bool async)
216316
{
217317
using (SqlConnection con = new SqlConnection(constr))

0 commit comments

Comments
 (0)