Skip to content

Commit

Permalink
Reset cached async state on disposing a command (dotnet#1128)
Browse files Browse the repository at this point in the history
  • Loading branch information
Davoud Eshtehari committed Aug 30, 2021
1 parent f59df96 commit 56a1da1
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 19 deletions.
20 changes: 20 additions & 0 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIStreams.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ public override async Task<int> ReadAsync(byte[] buffer, int offset, int count,
{
return await base.ReadAsync(buffer, offset, count, cancellationToken).ConfigureAwait(false);
}
catch (System.Exception e)
{
SqlClientEventSource.Log.TrySNITraceEvent("SNISslStream.ReadAsync | ERR | Internal Exception occurred while reading data: {0}", args0: e?.Message);
throw;
}
finally
{
_readAsyncSemaphore.Release();
Expand All @@ -47,6 +52,11 @@ public override async Task WriteAsync(byte[] buffer, int offset, int count, Canc
{
await base.WriteAsync(buffer, offset, count, cancellationToken).ConfigureAwait(false);
}
catch (System.Exception e)
{
SqlClientEventSource.Log.TrySNITraceEvent("SNISslStream.ReadAsync | ERR | Internal Exception occurred while writing data: {0}", args0: e?.Message);
throw;
}
finally
{
_writeAsyncSemaphore.Release();
Expand Down Expand Up @@ -76,6 +86,11 @@ public override async Task<int> ReadAsync(byte[] buffer, int offset, int count,
{
return await base.ReadAsync(buffer, offset, count, cancellationToken).ConfigureAwait(false);
}
catch (System.Exception e)
{
SqlClientEventSource.Log.TrySNITraceEvent("SNINetworkStream.ReadAsync | ERR | Internal Exception occurred while reading data: {0}", args0: e?.Message);
throw;
}
finally
{
_readAsyncSemaphore.Release();
Expand All @@ -90,6 +105,11 @@ public override async Task WriteAsync(byte[] buffer, int offset, int count, Canc
{
await base.WriteAsync(buffer, offset, count, cancellationToken).ConfigureAwait(false);
}
catch (System.Exception e)
{
SqlClientEventSource.Log.TrySNITraceEvent("SNINetworkStream.ReadAsync | ERR | Internal Exception occurred while writing data: {0}", args0: e?.Message);
throw;
}
finally
{
_writeAsyncSemaphore.Release();
Expand Down
87 changes: 69 additions & 18 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlCommand.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,8 @@ internal bool IsActiveConnectionValid(SqlConnection activeConnection)

internal void ResetAsyncState()
{
SqlClientEventSource.Log.TryTraceEvent("CachedAsyncState.ResetAsyncState | API | ObjectId {0}, Client Connection Id {1}, AsyncCommandInProgress={2}",
_cachedAsyncConnection?.ObjectID, _cachedAsyncConnection?.ClientConnectionId, _cachedAsyncConnection?.AsyncCommandInProgress);
_cachedAsyncCloseCount = -1;
_cachedAsyncResult = null;
if (_cachedAsyncConnection != null)
Expand All @@ -252,6 +254,7 @@ internal void SetActiveConnectionAndResult(TaskCompletionSource<object> completi
{
Debug.Assert(activeConnection != null, "Unexpected null connection argument on SetActiveConnectionAndResult!");
TdsParser parser = activeConnection?.Parser;
SqlClientEventSource.Log.TryTraceEvent("SqlCommand.SetActiveConnectionAndResult | API | ObjectId {0}, Client Connection Id {1}, MARS={2}", activeConnection?.ObjectID, activeConnection?.ClientConnectionId, parser?.MARSOn);
if ((parser == null) || (parser.State == TdsParserState.Closed) || (parser.State == TdsParserState.Broken))
{
throw ADP.ClosedConnectionError();
Expand Down Expand Up @@ -972,8 +975,12 @@ override protected DbParameter CreateDbParameter()
override protected void Dispose(bool disposing)
{
if (disposing)
{ // release managed objects
{
// release managed objects
_cachedMetaData = null;

// reset async cache information to allow a second async execute
_cachedAsyncState?.ResetAsyncState();
}
// release unmanaged objects
base.Dispose(disposing);
Expand Down Expand Up @@ -1213,14 +1220,23 @@ private void BeginExecuteNonQueryInternalReadStage(TaskCompletionSource<object>
cachedAsyncState.SetActiveConnectionAndResult(completion, nameof(EndExecuteNonQuery), _activeConnection);
_stateObj.ReadSni(completion);
}
// Cause of a possible unstable runtime situation on facing with `OutOfMemoryException` and `StackOverflowException` exceptions,
// trying to call further functions in the catch of either may fail that should be considered on debuging!
catch (System.OutOfMemoryException e)
{
_activeConnection.Abort(e);
throw;
}
catch (System.StackOverflowException e)
{
_activeConnection.Abort(e);
throw;
}
catch (Exception)
{
// Similarly, if an exception occurs put the stateObj back into the pool.
// and reset async cache information to allow a second async execute
if (null != _cachedAsyncState)
{
_cachedAsyncState.ResetAsyncState();
}
_cachedAsyncState?.ResetAsyncState();
ReliablePutStateObject();
throw;
}
Expand All @@ -1229,7 +1245,9 @@ private void BeginExecuteNonQueryInternalReadStage(TaskCompletionSource<object>
private void VerifyEndExecuteState(Task completionTask, string endMethod, bool fullCheckForColumnEncryption = false)
{
Debug.Assert(completionTask != null);

SqlClientEventSource.Log.TryTraceEvent("SqlCommand.VerifyEndExecuteState | API | ObjectId {0}, Client Connection Id {1}, MARS={2}, AsyncCommandInProgress={3}",
_activeConnection?.ObjectID, _activeConnection?.ClientConnectionId,
_activeConnection?.Parser?.MARSOn, _activeConnection?.AsyncCommandInProgress);
if (completionTask.IsCanceled)
{
if (_stateObj != null)
Expand Down Expand Up @@ -1336,10 +1354,7 @@ public int EndExecuteNonQueryAsync(IAsyncResult asyncResult)
if (asyncException != null)
{
// Leftover exception from the Begin...InternalReadStage
if (cachedAsyncState != null)
{
cachedAsyncState.ResetAsyncState();
}
cachedAsyncState?.ResetAsyncState();
ReliablePutStateObject();
throw asyncException.InnerException;
}
Expand Down Expand Up @@ -1402,6 +1417,9 @@ private int EndExecuteNonQueryInternal(IAsyncResult asyncResult)

private object InternalEndExecuteNonQuery(IAsyncResult asyncResult, bool isInternal, [CallerMemberName] string endMethod = "")
{
SqlClientEventSource.Log.TryTraceEvent("SqlCommand.InternalEndExecuteNonQuery | INFO | ObjectId {0}, Client Connection Id {1}, MARS={2}, AsyncCommandInProgress={3}",
_activeConnection?.ObjectID, _activeConnection?.ClientConnectionId,
_activeConnection?.Parser?.MARSOn, _activeConnection?.AsyncCommandInProgress);
VerifyEndExecuteState((Task)asyncResult, endMethod);
WaitForAsyncResults(asyncResult, isInternal);

Expand Down Expand Up @@ -1486,6 +1504,8 @@ private object InternalEndExecuteNonQuery(IAsyncResult asyncResult, bool isInter

private Task InternalExecuteNonQuery(TaskCompletionSource<object> completion, bool sendToPipe, int timeout, out bool usedCache, bool asyncWrite = false, bool inRetry = false, [CallerMemberName] string methodName = "")
{
SqlClientEventSource.Log.TryTraceEvent("SqlCommand.InternalExecuteNonQuery | INFO | ObjectId {0}, Client Connection Id {1}, AsyncCommandInProgress={2}",
_activeConnection?.ObjectID, _activeConnection?.ClientConnectionId, _activeConnection?.AsyncCommandInProgress);
bool isAsync = (null != completion);
usedCache = false;

Expand Down Expand Up @@ -1716,14 +1736,25 @@ private void BeginExecuteXmlReaderInternalReadStage(TaskCompletionSource<object>
_cachedAsyncState.SetActiveConnectionAndResult(completion, nameof(EndExecuteXmlReader), _activeConnection);
_stateObj.ReadSni(completion);
}
// Cause of a possible unstable runtime situation on facing with `OutOfMemoryException` and `StackOverflowException` exceptions,
// trying to call further functions in the catch of either may fail that should be considered on debuging!
catch (System.OutOfMemoryException e)
{
_activeConnection.Abort(e);
completion.TrySetException(e);
throw;
}
catch (System.StackOverflowException e)
{
_activeConnection.Abort(e);
completion.TrySetException(e);
throw;
}
catch (Exception e)
{
// Similarly, if an exception occurs put the stateObj back into the pool.
// and reset async cache information to allow a second async execute
if (null != _cachedAsyncState)
{
_cachedAsyncState.ResetAsyncState();
}
_cachedAsyncState?.ResetAsyncState();
ReliablePutStateObject();
completion.TrySetException(e);
}
Expand All @@ -1750,6 +1781,7 @@ private XmlReader EndExecuteXmlReaderAsync(IAsyncResult asyncResult)
Exception asyncException = ((Task)asyncResult).Exception;
if (asyncException != null)
{
cachedAsyncState?.ResetAsyncState();
ReliablePutStateObject();
throw asyncException.InnerException;
}
Expand Down Expand Up @@ -1944,6 +1976,7 @@ internal SqlDataReader EndExecuteReaderAsync(IAsyncResult asyncResult)
Exception asyncException = ((Task)asyncResult).Exception;
if (asyncException != null)
{
cachedAsyncState?.ResetAsyncState();
ReliablePutStateObject();
throw asyncException.InnerException;
}
Expand All @@ -1967,6 +2000,9 @@ internal SqlDataReader EndExecuteReaderAsync(IAsyncResult asyncResult)

private SqlDataReader EndExecuteReaderInternal(IAsyncResult asyncResult)
{
SqlClientEventSource.Log.TryTraceEvent("SqlCommand.EndExecuteReaderInternal | API | ObjectId {0}, Client Connection Id {1}, MARS={2}, AsyncCommandInProgress={3}",
_activeConnection?.ObjectID, _activeConnection?.ClientConnectionId,
_activeConnection?.Parser?.MARSOn, _activeConnection?.AsyncCommandInProgress);
SqlStatistics statistics = null;
bool success = false;
int? sqlExceptionNumber = null;
Expand Down Expand Up @@ -2337,28 +2373,43 @@ long firstAttemptStart
private void BeginExecuteReaderInternalReadStage(TaskCompletionSource<object> completion)
{
Debug.Assert(completion != null, "CompletionSource should not be null");
SqlClientEventSource.Log.TryCorrelationTraceEvent("SqlCommand.BeginExecuteReaderInternalReadStage | INFO | Correlation | Object Id {0}, Activity Id {1}, Client Connection Id {2}, Command Text '{3}'", ObjectID, ActivityCorrelator.Current, Connection?.ClientConnectionId, CommandText);
// Read SNI does not have catches for async exceptions, handle here.
try
{
// must finish caching information before ReadSni which can activate the callback before returning
cachedAsyncState.SetActiveConnectionAndResult(completion, nameof(EndExecuteReader), _activeConnection);
_stateObj.ReadSni(completion);
}
// Cause of a possible unstable runtime situation on facing with `OutOfMemoryException` and `StackOverflowException` exceptions,
// trying to call further functions in the catch of either may fail that should be considered on debuging!
catch (System.OutOfMemoryException e)
{
_activeConnection.Abort(e);
completion.TrySetException(e);
throw;
}
catch (System.StackOverflowException e)
{
_activeConnection.Abort(e);
completion.TrySetException(e);
throw;
}
catch (Exception e)
{
// Similarly, if an exception occurs put the stateObj back into the pool.
// and reset async cache information to allow a second async execute
if (null != _cachedAsyncState)
{
_cachedAsyncState.ResetAsyncState();
}
_cachedAsyncState?.ResetAsyncState();
ReliablePutStateObject();
completion.TrySetException(e);
}
}

private SqlDataReader InternalEndExecuteReader(IAsyncResult asyncResult, bool isInternal, string endMethod)
{
SqlClientEventSource.Log.TryTraceEvent("SqlCommand.InternalEndExecuteReader | INFO | ObjectId {0}, Client Connection Id {1}, MARS={2}, AsyncCommandInProgress={3}",
_activeConnection?.ObjectID, _activeConnection?.ClientConnectionId,
_activeConnection?.Parser?.MARSOn, _activeConnection?.AsyncCommandInProgress);
VerifyEndExecuteState((Task)asyncResult, endMethod);
WaitForAsyncResults(asyncResult, isInternal);

Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2859,6 +2859,7 @@ public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error)
// to the outstanding GCRoot until AppDomain.Unload.
// We live with the above for the time being due to the constraints of the current
// reliability infrastructure provided by the CLR.
SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObject.ReadAsyncCallback | Info | State Object Id {0}, received error {1} on idle connection", _objectID, (int)error);

TaskCompletionSource<object> source = _networkPacketTaskSource;
#if DEBUG
Expand Down

0 comments on commit 56a1da1

Please sign in to comment.