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

Fix wrong data blended with transactions in .NET core #1023

Merged
merged 7 commits into from
Apr 15, 2021
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -3412,6 +3412,8 @@ private bool TryReadInternal(bool setTimeout, out bool more)
{
SqlStatistics statistics = null;
long scopeID = SqlClientEventSource.Log.TryScopeEnterEvent("SqlDataReader.TryReadInternal | API | Object Id {0}", ObjectID);
RuntimeHelpers.PrepareConstrainedRegions();

try
{
statistics = SqlStatistics.StartTimer(Statistics);
Expand Down Expand Up @@ -3561,6 +3563,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)
cheenamalhotra marked this conversation as resolved.
Show resolved Hide resolved
{
_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,12 @@ 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 going to doom cause of having OpenResultsCount {1} > 0.",
ObjectID,
internalTransaction.OpenResultsCount);
DavoudEshtehari marked this conversation as resolved.
Show resolved Hide resolved
throw SQL.CannotCompleteDelegatedTransactionWithOpenResults(this, _parser.MARSOn);
cheenamalhotra marked this conversation as resolved.
Show resolved Hide resolved
}
}

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));
cheenamalhotra marked this conversation as resolved.
Show resolved Hide resolved
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,11 @@ 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 going to doom cause of having OpenResultsCount {1} > 0.",
ObjectID,
internalTransaction.OpenResultsCount);
DavoudEshtehari marked this conversation as resolved.
Show resolved Hide resolved
throw SQL.CannotCompleteDelegatedTransactionWithOpenResults(this, _parser.MARSOn);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,88 @@ namespace Microsoft.Data.SqlClient.ManualTesting.Tests
{
public static class TransactionTest
{
public static TheoryData<string> ConnectionStrings =>
new TheoryData<string>
{
new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString)
{
Pooling = false,
MultipleActiveResultSets = false
}.ConnectionString
, new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString)
{
MultipleActiveResultSets = false,
Pooling = true,
MaxPoolSize = 1
}.ConnectionString
, new SqlConnectionStringBuilder(DataTestUtility.TCPConnectionString)
{
MultipleActiveResultSets = true
}.ConnectionString
};

// 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.
[ConditionalTheory(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup), nameof(DataTestUtility.IsNotAzureSynapse))]
DavoudEshtehari marked this conversation as resolved.
Show resolved Hide resolved
[MemberData(nameof(ConnectionStrings))]
public static void ReadNextQueryAfterTxAborted(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();
scope.Dispose();
DavoudEshtehari marked this conversation as resolved.
Show resolved Hide resolved
}

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);
if (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);
if (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);
if (result)
cheenamalhotra marked this conversation as resolved.
Show resolved Hide resolved
{
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