Skip to content

Commit

Permalink
Fix | Fixes driver behavior of sending Attention signal for successfu…
Browse files Browse the repository at this point in the history 
…l Bulk Copy operation (#308)
  • Loading branch information
@cheenamalhotra
cheenamalhotra authored Nov 15, 2019
1 parent 6085573 commit c402a17
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 38 deletions.
18 changes: 9 additions & 9 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2649,7 +2649,7 @@ private void CopyBatchesAsyncContinuedOnError(bool cleanupParser)

if (_stateObj != null)
{
CleanUpStateObjectOnError();
CleanUpStateObject();
}
}
catch (OutOfMemoryException)
Expand All @@ -2672,7 +2672,7 @@ private void CopyBatchesAsyncContinuedOnError(bool cleanupParser)
}

// Cleans the stateobj. Used in a number of places, specially in exceptions.
private void CleanUpStateObjectOnError()
private void CleanUpStateObject(bool isCancelRequested = true)
{
if (_stateObj != null)
{
Expand All @@ -2682,7 +2682,7 @@ private void CleanUpStateObjectOnError()
_stateObj.ResetBuffer();
_stateObj.ResetPacketCounters();
// If _parser is closed, sending attention will raise debug assertion, so we avoid it (but not calling CancelRequest).
if (_parser.State == TdsParserState.OpenNotLoggedIn || _parser.State == TdsParserState.OpenLoggedIn)
if (isCancelRequested && (_parser.State == TdsParserState.OpenNotLoggedIn || _parser.State == TdsParserState.OpenLoggedIn))
{
_stateObj.CancelRequest();
}
Expand Down Expand Up @@ -2743,7 +2743,7 @@ private void WriteToServerInternalRestContinuedAsync(BulkCopySimpleResultSet int
_localColumnMappings = null;
try
{
CleanUpStateObjectOnError();
CleanUpStateObject();
}
finally
{
Expand All @@ -2759,7 +2759,7 @@ private void WriteToServerInternalRestContinuedAsync(BulkCopySimpleResultSet int
_localColumnMappings = null;
try
{
CleanUpStateObjectOnError();
CleanUpStateObject(isCancelRequested: false);
}
finally
{
Expand All @@ -2786,11 +2786,11 @@ private void WriteToServerInternalRestContinuedAsync(BulkCopySimpleResultSet int

try
{
CleanUpStateObjectOnError();
CleanUpStateObject(isCancelRequested: false);
}
catch (Exception cleanupEx)
{
Debug.Fail("Unexpected exception during CleanUpstateObjectOnError (ignored)", cleanupEx.ToString());
Debug.Fail($"Unexpected exception during {nameof(CleanUpStateObject)} (ignored)", cleanupEx.ToString());
}

if (source != null)
Expand All @@ -2805,11 +2805,11 @@ private void WriteToServerInternalRestContinuedAsync(BulkCopySimpleResultSet int

try
{
CleanUpStateObjectOnError();
CleanUpStateObject();
}
catch (Exception cleanupEx)
{
Debug.Fail("Unexpected exception during CleanUpstateObjectOnError (ignored)", cleanupEx.ToString());
Debug.Fail($"Unexpected exception during {nameof(CleanUpStateObject)} (ignored)", cleanupEx.ToString());
}

if (source != null)
Expand Down
63 changes: 35 additions & 28 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,14 @@ public SourceColumnMetadata(ValueMethod method, bool isSqlType, bool isDataFeed)
// TODO: I will make this internal to use Reflection.
#if DEBUG
internal static bool _setAlwaysTaskOnWrite = false; //when set and in DEBUG mode, TdsParser::WriteBulkCopyValue will always return a task
internal static bool SetAlwaysTaskOnWrite {
set {
internal static bool SetAlwaysTaskOnWrite
{
set
{
_setAlwaysTaskOnWrite = value;
}
get{
get
{
return _setAlwaysTaskOnWrite;
}
}
Expand Down Expand Up @@ -1072,7 +1075,8 @@ private object GetValueFromSourceRow(int destRowIndex, out bool isSqlType, out b
isNull = (columnAsINullable != null) && columnAsINullable.IsNull;
}
#if DEBUG
else if (!isNull) {
else if (!isNull)
{
Debug.Assert(!(value is INullable) || !((INullable)value).IsNull, "IsDBNull returned false, but GetValue returned a null INullable");
}
#endif
Expand Down Expand Up @@ -2151,7 +2155,8 @@ private Task WriteRowSourceToServerAsync(int columnCount, CancellationToken ctok
TdsParser.ReliabilitySection tdsReliabilitySection = new TdsParser.ReliabilitySection();

RuntimeHelpers.PrepareConstrainedRegions();
try {
try
{
tdsReliabilitySection.Start();
#else // !DEBUG
{
Expand Down Expand Up @@ -2188,7 +2193,8 @@ private Task WriteRowSourceToServerAsync(int columnCount, CancellationToken ctok
}

#if DEBUG
finally {
finally
{
tdsReliabilitySection.Stop();
}
#endif //DEBUG
Expand Down Expand Up @@ -2896,24 +2902,26 @@ private void CopyBatchesAsyncContinuedOnError(bool cleanupParser)
#if DEBUG
TdsParser.ReliabilitySection tdsReliabilitySection = new TdsParser.ReliabilitySection();
RuntimeHelpers.PrepareConstrainedRegions();
try {
try
{
tdsReliabilitySection.Start();
#endif //DEBUG
if ((cleanupParser) && (_parser != null) && (_stateObj != null))
{
_parser._asyncWrite = false;
Task task = _parser.WriteBulkCopyDone(_stateObj);
Debug.Assert(task == null, "Write should not pend when error occurs");
RunParser();
}
if ((cleanupParser) && (_parser != null) && (_stateObj != null))
{
_parser._asyncWrite = false;
Task task = _parser.WriteBulkCopyDone(_stateObj);
Debug.Assert(task == null, "Write should not pend when error occurs");
RunParser();
}

if (_stateObj != null)
{
CleanUpStateObjectOnError();
}
if (_stateObj != null)
{
CleanUpStateObject();
}
#if DEBUG
}
finally {
finally
{
tdsReliabilitySection.Stop();
}
#endif //DEBUG
Expand All @@ -2939,7 +2947,7 @@ private void CopyBatchesAsyncContinuedOnError(bool cleanupParser)

//Cleans the stateobj. Used in a number of places, specially in exceptions
//
private void CleanUpStateObjectOnError()
private void CleanUpStateObject(bool isCancelRequested = true)
{
if (_stateObj != null)
{
Expand All @@ -2949,7 +2957,7 @@ private void CleanUpStateObjectOnError()
_stateObj.ResetBuffer();
_stateObj.ResetPacketCounters();
//If _parser is closed, sending attention will raise debug assertion, so we avoid it but not calling CancelRequest;
if (_parser.State == TdsParserState.OpenNotLoggedIn || _parser.State == TdsParserState.OpenLoggedIn)
if (isCancelRequested && (_parser.State == TdsParserState.OpenNotLoggedIn || _parser.State == TdsParserState.OpenLoggedIn))
{
_stateObj.CancelRequest();
}
Expand Down Expand Up @@ -3011,13 +3019,12 @@ private void WriteToServerInternalRestContinuedAsync(BulkCopySimpleResultSet int
_localColumnMappings = null;
try
{
CleanUpStateObjectOnError();
CleanUpStateObject();
}
finally
{
source.SetCanceled();
}
}
else if (task.Exception != null)
{
Expand All @@ -3028,7 +3035,7 @@ private void WriteToServerInternalRestContinuedAsync(BulkCopySimpleResultSet int
_localColumnMappings = null;
try
{
CleanUpStateObjectOnError();
CleanUpStateObject(isCancelRequested: false);
}
finally
{
Expand All @@ -3054,11 +3061,11 @@ private void WriteToServerInternalRestContinuedAsync(BulkCopySimpleResultSet int

try
{
CleanUpStateObjectOnError();
CleanUpStateObject(isCancelRequested: false);
}
catch (Exception cleanupEx)
{
Debug.Fail("Unexpected exception during CleanUpstateObjectOnError (ignored)", cleanupEx.ToString());
Debug.Fail($"Unexpected exception during {nameof(CleanUpStateObject)} (ignored)", cleanupEx.ToString());
}

if (source != null)
Expand All @@ -3073,11 +3080,11 @@ private void WriteToServerInternalRestContinuedAsync(BulkCopySimpleResultSet int

try
{
CleanUpStateObjectOnError();
CleanUpStateObject();
}
catch (Exception cleanupEx)
{
Debug.Fail("Unexpected exception during CleanUpstateObjectOnError (ignored)", cleanupEx.ToString());
Debug.Fail($"Unexpected exception during {nameof(CleanUpStateObject)} (ignored)", cleanupEx.ToString());
}

if (source != null)
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/CopyAllFromReader.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public static void Test(string srcConstr, string dstConstr, string dstTable)
DataTestUtility.AssertEqualsWithDescription((long)0, (long)stats["CursorOpens"], "Non-zero CursorOpens value: " + (long)stats["CursorOpens"]);
DataTestUtility.AssertEqualsWithDescription((long)0, (long)stats["IduRows"], "Non-zero IduRows value: " + (long)stats["IduRows"]);

DataTestUtility.AssertEqualsWithDescription((long)4, stats["BuffersReceived"], "Unexpected BuffersReceived value.");
DataTestUtility.AssertEqualsWithDescription((long)3, stats["BuffersReceived"], "Unexpected BuffersReceived value.");
DataTestUtility.AssertEqualsWithDescription((long)3, stats["BuffersSent"], "Unexpected BuffersSent value.");
DataTestUtility.AssertEqualsWithDescription((long)0, stats["IduCount"], "Unexpected IduCount value.");
DataTestUtility.AssertEqualsWithDescription((long)3, stats["SelectCount"], "Unexpected SelectCount value.");
Expand Down

0 comments on commit c402a17

Please sign in to comment.