Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Release 2.1] Fix wrong data blended with transactions in .NET core #1051

Merged
merged 1 commit into from
Apr 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3404,7 +3404,9 @@ override public bool Read()
private bool TryReadInternal(bool setTimeout, out bool more)
{
SqlStatistics statistics = null;
long scopeID = SqlClientEventSource.Log.TryScopeEnterEvent("<sc.SqlDataReader.Read|API> {0}", ObjectID);
long scopeID = SqlClientEventSource.Log.TryScopeEnterEvent("SqlDataReader.TryReadInternal | API | Object Id {0}", ObjectID);
RuntimeHelpers.PrepareConstrainedRegions();

try
{
statistics = SqlStatistics.StartTimer(Statistics);
Expand Down Expand Up @@ -3554,6 +3556,39 @@ private bool TryReadInternal(bool setTimeout, out bool more)

return true;
}
catch (OutOfMemoryException e)
{
_isClosed = true;
SqlConnection con = _connection;
if (con != null)
{
con.Abort(e);
}
throw;
}
catch (StackOverflowException e)
{
_isClosed = true;
SqlConnection con = _connection;
if (con != null)
{
con.Abort(e);
}
throw;
}
/* Even though ThreadAbortException exists in .NET Core,
* since Abort is not supported, the common language runtime won't ever throw ThreadAbortException.
* just to keep a common codebase!*/
catch (System.Threading.ThreadAbortException e)
{
_isClosed = true;
SqlConnection con = _connection;
if (con != null)
{
con.Abort(e);
}
throw;
}
finally
{
SqlStatistics.StopTimer(statistics);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,10 @@ public void Rollback(SinglePhaseEnlistment enlistment)
connection.ExecuteTransaction(SqlInternalConnection.TransactionRequest.Rollback, null, System.Data.IsolationLevel.Unspecified, _internalTransaction, true);
}
}
catch (SqlException)
catch (SqlException e)
{
ADP.TraceExceptionWithoutRethrow(e);

// Doom the connection, to make sure that the transaction is
// eventually rolled back.
// VSTS 144562: doom the connection while having the lock on it to prevent race condition with "Transaction Ended" Event
Expand All @@ -273,8 +275,9 @@ public void Rollback(SinglePhaseEnlistment enlistment)
// we have the tracing that we're doing to fallback on for the
// investigation.
}
catch (InvalidOperationException)
catch (InvalidOperationException e)
{
ADP.TraceExceptionWithoutRethrow(e);
connection.DoomThisConnection();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1116,9 +1116,13 @@ bool isDelegateControlRequest
stateObj = _parser.GetSession(this);
mustPutSession = true;
}
else if (internalTransaction.OpenResultsCount != 0)
if (internalTransaction.OpenResultsCount != 0)
{
//throw SQL.CannotCompleteDelegatedTransactionWithOpenResults(this);
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}",
ObjectID,
internalTransaction.OpenResultsCount,
_parser.MARSOn);
throw SQL.CannotCompleteDelegatedTransactionWithOpenResults(this, _parser.MARSOn);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,13 @@ internal static Exception SqlDependencyNoMatchingServerDatabaseStart()
//
// SQL.SqlDelegatedTransaction
//
static internal Exception CannotCompleteDelegatedTransactionWithOpenResults(SqlInternalConnectionTds internalConnection, bool marsOn)
{
SqlErrorCollection errors = new SqlErrorCollection();
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));
return SqlException.CreateException(errors, null, internalConnection);
}

internal static TransactionPromotionException PromotionFailed(Exception inner)
{
TransactionPromotionException e = new TransactionPromotionException(System.StringsHelper.GetString(Strings.SqlDelegatedTransaction_PromotionFailed), inner);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1380,8 +1380,12 @@ internal void ExecuteTransactionYukon(
stateObj = _parser.GetSession(this);
mustPutSession = true;
}
else if (internalTransaction.OpenResultsCount != 0)
if (internalTransaction.OpenResultsCount != 0)
{
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}",
ObjectID,
internalTransaction.OpenResultsCount,
_parser.MARSOn);
throw SQL.CannotCompleteDelegatedTransactionWithOpenResults(this, _parser.MARSOn);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,99 @@ namespace Microsoft.Data.SqlClient.ManualTesting.Tests
{
public static class TransactionTest
{
public static TheoryData<string> PoolEnabledConnectionStrings =>
new TheoryData<string>
{
new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString)
{
MultipleActiveResultSets = false,
Pooling = true,
MaxPoolSize = 1
}.ConnectionString
, new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString)
{
Pooling = true,
MultipleActiveResultSets = true
}.ConnectionString
};

public static TheoryData<string> PoolDisabledConnectionStrings =>
new TheoryData<string>
{
new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString)
{
Pooling = false,
MultipleActiveResultSets = false
}.ConnectionString
, new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString)
{
Pooling = false,
MultipleActiveResultSets = true
}.ConnectionString
};

[ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))]
[MemberData(nameof(PoolEnabledConnectionStrings))]
public static void ReadNextQueryAfterTxAbortedPoolEnabled(string connString)
=> ReadNextQueryAfterTxAbortedTest(connString);

// Azure SQL has no DTC support
[ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureServer))]
[MemberData(nameof(PoolDisabledConnectionStrings))]
public static void ReadNextQueryAfterTxAbortedPoolDisabled(string connString)
=> ReadNextQueryAfterTxAbortedTest(connString);

private static void ReadNextQueryAfterTxAbortedTest(string connString)
{
using (System.Transactions.TransactionScope scope = new System.Transactions.TransactionScope())
{
using (SqlConnection sqlConnection = new SqlConnection(connString))
{
SqlCommand cmd = new SqlCommand("SELECT 1", sqlConnection);
sqlConnection.Open();
var reader = cmd.ExecuteReader();
// Disposing Transaction Scope before completing read triggers GitHub issue #980 use-case that leads to wrong data in future rounds.
scope.Dispose();
}

using (SqlConnection sqlConnection = new SqlConnection(connString))
using (SqlCommand cmd = new SqlCommand("SELECT 2", sqlConnection))
{
sqlConnection.Open();
using (SqlDataReader reader = cmd.ExecuteReader())
{
bool result = reader.Read();
Assert.True(result);
Assert.Equal(2, reader.GetValue(0));
}
}

using (SqlConnection sqlConnection = new SqlConnection(connString))
using (SqlCommand cmd = new SqlCommand("SELECT 3", sqlConnection))
{
sqlConnection.Open();
using (SqlDataReader reader = cmd.ExecuteReaderAsync().Result)
{
bool result = reader.ReadAsync().Result;
Assert.True(result);
Assert.Equal(3, reader.GetValue(0));
}
}

using (SqlConnection sqlConnection = new SqlConnection(connString))
using (SqlCommand cmd = new SqlCommand("SELECT TOP(1) 4 Clm0 FROM sysobjects FOR XML AUTO", sqlConnection))
{
sqlConnection.Open();
using (System.Xml.XmlReader reader = cmd.ExecuteXmlReader())
{
bool result = reader.Read();
Assert.True(result);
Assert.Equal("4", reader[0]);
}
}
}
}

// 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.
[ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))]
public static void TestMain()
Expand Down