Skip to content

Commit 2e04aa8

Browse files
cheenamalhotracheenamalhotra
authored
[Release 2.1] Fix wrong data blended with transactions in .NET core (#1051)
1 parent 4532c96 commit 2e04aa8

File tree

6 files changed

+152
-6
lines changed

6 files changed

+152
-6
lines changed

‎src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3404,7 +3404,9 @@ override public bool Read()
34043404
private bool TryReadInternal(bool setTimeout, out bool more)
34053405
{
34063406
SqlStatistics statistics = null;
3407-
long scopeID = SqlClientEventSource.Log.TryScopeEnterEvent("<sc.SqlDataReader.Read|API> {0}", ObjectID);
3407+
long scopeID = SqlClientEventSource.Log.TryScopeEnterEvent("SqlDataReader.TryReadInternal | API | Object Id {0}", ObjectID);
3408+
RuntimeHelpers.PrepareConstrainedRegions();
3409+
34083410
try
34093411
{
34103412
statistics = SqlStatistics.StartTimer(Statistics);
@@ -3554,6 +3556,39 @@ private bool TryReadInternal(bool setTimeout, out bool more)
35543556

35553557
return true;
35563558
}
3559+
catch (OutOfMemoryException e)
3560+
{
3561+
_isClosed = true;
3562+
SqlConnection con = _connection;
3563+
if (con != null)
3564+
{
3565+
con.Abort(e);
3566+
}
3567+
throw;
3568+
}
3569+
catch (StackOverflowException e)
3570+
{
3571+
_isClosed = true;
3572+
SqlConnection con = _connection;
3573+
if (con != null)
3574+
{
3575+
con.Abort(e);
3576+
}
3577+
throw;
3578+
}
3579+
/* Even though ThreadAbortException exists in .NET Core,
3580+
* since Abort is not supported, the common language runtime won't ever throw ThreadAbortException.
3581+
* just to keep a common codebase!*/
3582+
catch (System.Threading.ThreadAbortException e)
3583+
{
3584+
_isClosed = true;
3585+
SqlConnection con = _connection;
3586+
if (con != null)
3587+
{
3588+
con.Abort(e);
3589+
}
3590+
throw;
3591+
}
35573592
finally
35583593
{
35593594
SqlStatistics.StopTimer(statistics);

‎src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDelegatedTransaction.cs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -254,8 +254,10 @@ public void Rollback(SinglePhaseEnlistment enlistment)
254254
connection.ExecuteTransaction(SqlInternalConnection.TransactionRequest.Rollback, null, System.Data.IsolationLevel.Unspecified, _internalTransaction, true);
255255
}
256256
}
257-
catch (SqlException)
257+
catch (SqlException e)
258258
{
259+
ADP.TraceExceptionWithoutRethrow(e);
260+
259261
// Doom the connection, to make sure that the transaction is
260262
// eventually rolled back.
261263
// VSTS 144562: doom the connection while having the lock on it to prevent race condition with "Transaction Ended" Event
@@ -273,8 +275,9 @@ public void Rollback(SinglePhaseEnlistment enlistment)
273275
// we have the tracing that we're doing to fallback on for the
274276
// investigation.
275277
}
276-
catch (InvalidOperationException)
278+
catch (InvalidOperationException e)
277279
{
280+
ADP.TraceExceptionWithoutRethrow(e);
278281
connection.DoomThisConnection();
279282
}
280283
}

‎src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1116,9 +1116,13 @@ bool isDelegateControlRequest
11161116
stateObj = _parser.GetSession(this);
11171117
mustPutSession = true;
11181118
}
1119-
else if (internalTransaction.OpenResultsCount != 0)
1119+
if (internalTransaction.OpenResultsCount != 0)
11201120
{
1121-
//throw SQL.CannotCompleteDelegatedTransactionWithOpenResults(this);
1121+
SqlClientEventSource.Log.TryTraceEvent("<sc.SqlInternalConnectionTds.ExecuteTransactionYukon|DATA|CATCH> {0}, Connection is marked to be doomed when closed. Transaction ended with OpenResultsCount {1} > 0, MARSOn {2}",
1122+
ObjectID,
1123+
internalTransaction.OpenResultsCount,
1124+
_parser.MARSOn);
1125+
throw SQL.CannotCompleteDelegatedTransactionWithOpenResults(this, _parser.MARSOn);
11221126
}
11231127
}
11241128

‎src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlUtil.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -786,6 +786,13 @@ internal static Exception SqlDependencyNoMatchingServerDatabaseStart()
786786
//
787787
// SQL.SqlDelegatedTransaction
788788
//
789+
static internal Exception CannotCompleteDelegatedTransactionWithOpenResults(SqlInternalConnectionTds internalConnection, bool marsOn)
790+
{
791+
SqlErrorCollection errors = new SqlErrorCollection();
792+
errors.Add(new SqlError(TdsEnums.TIMEOUT_EXPIRED, (byte)0x00, TdsEnums.MIN_ERROR_CLASS, null, (StringsHelper.GetString(Strings.ADP_OpenReaderExists, marsOn ? ADP.Command : ADP.Connection)), "", 0, TdsEnums.SNI_WAIT_TIMEOUT));
793+
return SqlException.CreateException(errors, null, internalConnection);
794+
}
795+
789796
internal static TransactionPromotionException PromotionFailed(Exception inner)
790797
{
791798
TransactionPromotionException e = new TransactionPromotionException(System.StringsHelper.GetString(Strings.SqlDelegatedTransaction_PromotionFailed), inner);

‎src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1380,8 +1380,12 @@ internal void ExecuteTransactionYukon(
13801380
stateObj = _parser.GetSession(this);
13811381
mustPutSession = true;
13821382
}
1383-
else if (internalTransaction.OpenResultsCount != 0)
1383+
if (internalTransaction.OpenResultsCount != 0)
13841384
{
1385+
SqlClientEventSource.Log.TryTraceEvent("<sc.SqlInternalConnectionTds.ExecuteTransactionYukon|DATA|CATCH> {0}, Connection is marked to be doomed when closed. Transaction ended with OpenResultsCount {1} > 0, MARSOn {2}",
1386+
ObjectID,
1387+
internalTransaction.OpenResultsCount,
1388+
_parser.MARSOn);
13851389
throw SQL.CannotCompleteDelegatedTransactionWithOpenResults(this, _parser.MARSOn);
13861390
}
13871391
}

‎src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/TransactionTest/TransactionTest.cs‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,99 @@ namespace Microsoft.Data.SqlClient.ManualTesting.Tests
1010
{
1111
public static class TransactionTest
1212
{
13+
public static TheoryData<string> PoolEnabledConnectionStrings =>
14+
new TheoryData<string>
15+
{
16+
new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString)
17+
{
18+
MultipleActiveResultSets = false,
19+
Pooling = true,
20+
MaxPoolSize = 1
21+
}.ConnectionString
22+
, new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString)
23+
{
24+
Pooling = true,
25+
MultipleActiveResultSets = true
26+
}.ConnectionString
27+
};
28+
29+
public static TheoryData<string> PoolDisabledConnectionStrings =>
30+
new TheoryData<string>
31+
{
32+
new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString)
33+
{
34+
Pooling = false,
35+
MultipleActiveResultSets = false
36+
}.ConnectionString
37+
, new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString)
38+
{
39+
Pooling = false,
40+
MultipleActiveResultSets = true
41+
}.ConnectionString
42+
};
43+
44+
[ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
45+
[MemberData(nameof(PoolEnabledConnectionStrings))]
46+
public static void ReadNextQueryAfterTxAbortedPoolEnabled(string connString)
47+
=> ReadNextQueryAfterTxAbortedTest(connString);
48+
49+
// Azure SQL has no DTC support
50+
[ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))]
51+
[MemberData(nameof(PoolDisabledConnectionStrings))]
52+
public static void ReadNextQueryAfterTxAbortedPoolDisabled(string connString)
53+
=> ReadNextQueryAfterTxAbortedTest(connString);
54+
55+
private static void ReadNextQueryAfterTxAbortedTest(string connString)
56+
{
57+
using (System.Transactions.TransactionScope scope = new System.Transactions.TransactionScope())
58+
{
59+
using (SqlConnection sqlConnection = new SqlConnection(connString))
60+
{
61+
SqlCommand cmd = new SqlCommand("SELECT 1", sqlConnection);
62+
sqlConnection.Open();
63+
var reader = cmd.ExecuteReader();
64+
// Disposing Transaction Scope before completing read triggers GitHub issue #980 use-case that leads to wrong data in future rounds.
65+
scope.Dispose();
66+
}
67+
68+
using (SqlConnection sqlConnection = new SqlConnection(connString))
69+
using (SqlCommand cmd = new SqlCommand("SELECT 2", sqlConnection))
70+
{
71+
sqlConnection.Open();
72+
using (SqlDataReader reader = cmd.ExecuteReader())
73+
{
74+
bool result = reader.Read();
75+
Assert.True(result);
76+
Assert.Equal(2, reader.GetValue(0));
77+
}
78+
}
79+
80+
using (SqlConnection sqlConnection = new SqlConnection(connString))
81+
using (SqlCommand cmd = new SqlCommand("SELECT 3", sqlConnection))
82+
{
83+
sqlConnection.Open();
84+
using (SqlDataReader reader = cmd.ExecuteReaderAsync().Result)
85+
{
86+
bool result = reader.ReadAsync().Result;
87+
Assert.True(result);
88+
Assert.Equal(3, reader.GetValue(0));
89+
}
90+
}
91+
92+
using (SqlConnection sqlConnection = new SqlConnection(connString))
93+
using (SqlCommand cmd = new SqlCommand("SELECT TOP(1) 4 Clm0 FROM sysobjects FOR XML AUTO", sqlConnection))
94+
{
95+
sqlConnection.Open();
96+
using (System.Xml.XmlReader reader = cmd.ExecuteXmlReader())
97+
{
98+
bool result = reader.Read();
99+
Assert.True(result);
100+
Assert.Equal("4", reader[0]);
101+
}
102+
}
103+
}
104+
}
105+
13106
// Synapse: Enforced unique constraints are not supported. To create an unenforced unique constraint you must include the NOT ENFORCED syntax as part of your statement.
14107
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))]
15108
public static void TestMain()

0 commit comments

Comments
 (0)