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.

Sign up for GitHub

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

Jump to bottom

Code Merge | TdsParserStateObject alignment #2073

Merged
merged 2 commits into from Jul 6, 2023
Merged

Code Merge | TdsParserStateObject alignment #2073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
07f8ca9
Code Merge: Bring TdsParserStateObject closer together between netcor…
David-Engel Jun 28, 2023
c6737a3
Address feedback and remove excessive trace entry
David-Engel Jun 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 26 additions & 21 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs
View file
Open in desktop
Expand Up @@ -98,8 +98,9 @@ internal bool TryInitialize(TdsParserStateObject stateObj, int columnsCount)
return false;
}

SqlClientEventSource.Log.TryAdvancedTraceEvent("TdsParserStateObject.NullBitmap.Initialize | INFO | ADV | State Object Id {0}, NBCROW bitmap received, column count = {1}", stateObj._objectID, columnsCount);
SqlClientEventSource.Log.TryAdvancedTraceBinEvent("TdsParserStateObject.NullBitmap.Initialize | INFO | ADV | State Object Id {0}, Null Bitmap length {1}, NBCROW bitmap data: {2}", stateObj._objectID, (ushort)_nullBitmap.Length, _nullBitmap);
SqlClientEventSource.Log.TryAdvancedTraceEvent("TdsParserStateObject.NullBitmap.Initialize | INFO | ADV | State Object Id {0}, NBCROW bitmap received, column count = {1}", stateObj.ObjectID, columnsCount);
SqlClientEventSource.Log.TryAdvancedTraceBinEvent("TdsParserStateObject.NullBitmap.Initialize | INFO | ADV | State Object Id {0}, NBCROW bitmap data. Null Bitmap {1}, Null bitmap length: {2}", stateObj.ObjectID, _nullBitmap, (ushort)_nullBitmap.Length);

return true;
}
}
Expand Down Expand Up @@ -484,7 +485,7 @@ internal bool TryReadChar(out char value)

AssertValidState();
value = (char)((buffer[1] << 8) + buffer[0]);

return true;
}

Expand Down Expand Up @@ -543,7 +544,6 @@ internal bool TryReadInt32(out int value)
AssertValidState();
value = (buffer[3] << 24) + (buffer[2] << 16) + (buffer[1] << 8) + buffer[0];
return true;

}

// This method is safe to call when doing async without snapshot
Expand All @@ -564,7 +564,7 @@ internal bool TryReadInt64(out long value)
// then use ReadByteArray since the logic is there to take care of that.

int bytesRead;
if (!TryReadByteArray(_bTmp.AsSpan(_bTmpRead), 8 - _bTmpRead, out bytesRead))
if (!TryReadByteArray(_bTmp.AsSpan(start: _bTmpRead), 8 - _bTmpRead, out bytesRead))
{
Debug.Assert(_bTmpRead + bytesRead <= 8, "Read more data than required");
_bTmpRead += bytesRead;
Expand Down Expand Up @@ -642,7 +642,7 @@ internal bool TryReadUInt32(out uint value)
// then use ReadByteArray since the logic is there to take care of that.

int bytesRead;
if (!TryReadByteArray(_bTmp.AsSpan(_bTmpRead), 4 - _bTmpRead, out bytesRead))
if (!TryReadByteArray(_bTmp.AsSpan(start: _bTmpRead), 4 - _bTmpRead, out bytesRead))
{
Debug.Assert(_bTmpRead + bytesRead <= 4, "Read more data than required");
_bTmpRead += bytesRead;
Expand Down Expand Up @@ -1828,13 +1828,13 @@ public void ProcessSniPacket(PacketHandle packet, uint error)
}

SniReadStatisticsAndTracing();

SqlClientEventSource.Log.TryAdvancedTraceBinEvent("TdsParser.ReadNetworkPacketAsyncCallback | INFO | ADV | State Object Id {0}, Packet read. In Buffer {1}, In Bytes Read: {2}", ObjectID, _inBuff, (ushort)_inBytesRead);

AssertValidState();
}
else
{
throw SQL.ParsingError();
throw SQL.ParsingError(ParsingErrorState.ProcessSniPacketFailed);
}
}
}
Expand Down Expand Up @@ -1896,7 +1896,6 @@ 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);
David-Engel marked this conversation as resolved.
Show resolved Hide resolved

TaskCompletionSource<object> source = _networkPacketTaskSource;
#if DEBUG
Expand All @@ -1919,7 +1918,7 @@ public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error)
Debug.Assert(CheckPacket(packet, source) && source != null, "AsyncResult null on callback");

if (_parser.MARSOn)
{
{
// Only take reset lock on MARS and Async.
CheckSetResetConnectionState(error, CallbackType.Read);
}
Expand All @@ -1928,10 +1927,10 @@ public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error)

// The timer thread may be unreliable under high contention scenarios. It cannot be
// assumed that the timeout has happened on the timer thread callback. Check the timeout
// synchrnously and then call OnTimeoutSync to force an atomic change of state.
// synchrnously and then call OnTimeoutSync to force an atomic change of state.
if (TimeoutHasExpired)
{
OnTimeoutSync(true);
OnTimeoutSync(asyncClose: true);
}

// try to change to the stopped state but only do so if currently in the running state
Expand Down Expand Up @@ -2125,6 +2124,13 @@ public void WriteAsyncCallback(IntPtr key, PacketHandle packet, uint sniError)
/////////////////////////////////////////
// Network/Packet Writing & Processing //
/////////////////////////////////////////

//
// Takes a secure string and offsets and saves them for a write latter when the information is written out to SNI Packet
// This method is provided to better handle the life cycle of the clear text of the secure string
// This method also ensures that the clear text is not held in the unpined managed buffer so that it avoids getting moved around by CLR garbage collector
// TdsParserStaticMethods.EncryptPassword operation is also done in the unmanaged buffer for the clear text later
//
internal void WriteSecureString(SecureString secureString)
{
Debug.Assert(_securePasswords[0] == null || _securePasswords[1] == null, "There are more than two secure passwords");
Expand Down Expand Up @@ -2363,11 +2369,11 @@ internal Task WritePacket(byte flushMode, bool canAccumulate = false)
// However, since we don't know the version prior to login Is2005OrNewer was always false prior to login
// So removing the Is2005OrNewer check causes issues since the login packet happens to meet the rest of the conditions below
// So we need to avoid this check prior to login completing
state == TdsParserState.OpenLoggedIn &&
!_bulkCopyOpperationInProgress && // ignore the condition checking for bulk copy
_outBytesUsed == (_outputHeaderLen + BitConverter.ToInt32(_outBuff, _outputHeaderLen))
state == TdsParserState.OpenLoggedIn
&& !_bulkCopyOpperationInProgress // ignore the condition checking for bulk copy
&& _outBytesUsed == (_outputHeaderLen + BitConverter.ToInt32(_outBuff, _outputHeaderLen))
&& _outputPacketCount == 0
|| _outBytesUsed == _outputHeaderLen
|| _outBytesUsed == _outputHeaderLen
&& _outputPacketCount == 0)
{
return null;
Expand Down Expand Up @@ -2420,6 +2426,7 @@ internal Task WritePacket(byte flushMode, bool canAccumulate = false)
// If we have been canceled, then ensure that we write the ATTN packet as well
task = AsyncHelper.CreateContinuationTask(task, CancelWritePacket);
}

return task;
}

Expand All @@ -2443,7 +2450,7 @@ private void CancelWritePacket()
}
}

#pragma warning disable 0420 // a reference to a volatile field will not be treated as volatile
#pragma warning disable 420 // a reference to a volatile field will not be treated as volatile

private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccumulate, bool callerHasConnectionLock, bool asyncClose = false)
{
Expand All @@ -2456,9 +2463,7 @@ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccu

Task task = null;
_writeCompletionSource = null;

PacketHandle packetPointer = EmptyReadPacket;

bool sync = !_parser._asyncWrite;
if (sync && _asyncWriteCount > 0)
{ // for example, SendAttention while there are writes pending
Expand Down Expand Up @@ -2493,7 +2498,7 @@ private Task SNIWritePacket(PacketHandle packet, out uint sniError, bool canAccu

if (sniError == TdsEnums.SNI_SUCCESS_IO_PENDING)
{
Debug.Assert(!sync, "Completion should be handled in SniManagedWwrapper");
Debug.Assert(!sync, "Completion should be handled in SniManagedWrapper");
Interlocked.Increment(ref _asyncWriteCount);
Debug.Assert(_asyncWriteCount >= 0);
if (!canAccumulate)
Expand Down Expand Up @@ -2639,7 +2644,7 @@ internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = fa
}
}
#if DEBUG
}
}
#endif

SetTimeoutSeconds(AttentionTimeoutSeconds); // Initialize new attention timeout of 5 seconds.
Expand Down
20 changes: 10 additions & 10 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs
View file
Open in desktop
Expand Up @@ -860,7 +860,7 @@ private bool TryCleanPartialRead()
}

#if DEBUG
if (_stateObj._pendingData)
if (_stateObj.HasPendingData)
{
byte token;
if (!_stateObj.TryPeekByte(out token))
Expand Down Expand Up @@ -1067,7 +1067,7 @@ private bool TryCloseInternal(bool closeReader)
#else
{
#endif //DEBUG
if ((!_isClosed) && (parser != null) && (stateObj != null) && (stateObj._pendingData))
if ((!_isClosed) && (parser != null) && (stateObj != null) && (stateObj.HasPendingData))
{

// It is possible for this to be called during connection close on a
Expand Down Expand Up @@ -1333,7 +1333,7 @@ private bool TryConsumeMetaData()
{
// warning: Don't check the MetaData property within this function
// warning: as it will be a reentrant call
while (_parser != null && _stateObj != null && _stateObj._pendingData && !_metaDataConsumed)
while (_parser != null && _stateObj != null && _stateObj.HasPendingData && !_metaDataConsumed)
{
if (_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed)
{
Expand Down Expand Up @@ -3473,7 +3473,7 @@ private bool TryHasMoreResults(out bool moreResults)

Debug.Assert(null != _command, "unexpected null command from the data reader!");

while (_stateObj._pendingData)
while (_stateObj.HasPendingData)
{
byte token;
if (!_stateObj.TryPeekByte(out token))
Expand Down Expand Up @@ -3563,7 +3563,7 @@ private bool TryHasMoreRows(out bool moreRows)
moreRows = false;
return true;
}
if (_stateObj._pendingData)
if (_stateObj.HasPendingData)
{
// Consume error's, info's, done's on HasMoreRows, so user obtains error on Read.
// Previous bug where Read() would return false with error on the wire in the case
Expand Down Expand Up @@ -3620,7 +3620,7 @@ private bool TryHasMoreRows(out bool moreRows)
moreRows = false;
return false;
}
if (_stateObj._pendingData)
if (_stateObj.HasPendingData)
{
if (!_stateObj.TryPeekByte(out b))
{
Expand Down Expand Up @@ -3964,7 +3964,7 @@ private bool TryReadInternal(bool setTimeout, out bool more)
if (moreRows)
{
// read the row from the backend (unless it's an altrow were the marker is already inside the altrow ...)
while (_stateObj._pendingData)
while (_stateObj.HasPendingData)
{
if (_altRowStatus != ALTROWSTATUS.AltRow)
{
Expand Down Expand Up @@ -3996,7 +3996,7 @@ private bool TryReadInternal(bool setTimeout, out bool more)
}
}

if (!_stateObj._pendingData)
if (!_stateObj.HasPendingData)
{
if (!TryCloseInternal(false /*closeReader*/))
{
Expand All @@ -4020,7 +4020,7 @@ private bool TryReadInternal(bool setTimeout, out bool more)
{
// if we are in SingleRow mode, and we've read the first row,
// read the rest of the rows, if any
while (_stateObj._pendingData && !_sharedState._dataReady)
while (_stateObj.HasPendingData && !_sharedState._dataReady)
{
if (!_parser.TryRun(RunBehavior.ReturnImmediately, _command, this, null, _stateObj, out _sharedState._dataReady))
{
Expand Down Expand Up @@ -4061,7 +4061,7 @@ private bool TryReadInternal(bool setTimeout, out bool more)
more = false;

#if DEBUG
if ((!_sharedState._dataReady) && (_stateObj._pendingData))
if ((!_sharedState._dataReady) && (_stateObj.HasPendingData))
{
byte token;
if (!_stateObj.TryPeekByte(out token))
Expand Down
6 changes: 3 additions & 3 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs
View file
Open in desktop
Expand Up @@ -915,11 +915,11 @@ override internal void ValidateConnectionForExecute(SqlCommand command)
// or if MARS is off, then a datareader exists
throw ADP.OpenReaderExists(parser.MARSOn); // MDAC 66411
}
else if (!parser.MARSOn && parser._physicalStateObj._pendingData)
else if (!parser.MARSOn && parser._physicalStateObj.HasPendingData)
{
parser.DrainData(parser._physicalStateObj);
}
Debug.Assert(!parser._physicalStateObj._pendingData, "Should not have a busy physicalStateObject at this point!");
Debug.Assert(!parser._physicalStateObj.HasPendingData, "Should not have a busy physicalStateObject at this point!");

parser.RollbackOrphanedAPITransactions();
}
Expand Down Expand Up @@ -1078,7 +1078,7 @@ private void ResetConnection()
// obtains a clone.

Debug.Assert(!HasLocalTransactionFromAPI, "Upon ResetConnection SqlInternalConnectionTds has a currently ongoing local transaction.");
Debug.Assert(!_parser._physicalStateObj._pendingData, "Upon ResetConnection SqlInternalConnectionTds has pending data.");
Debug.Assert(!_parser._physicalStateObj.HasPendingData, "Upon ResetConnection SqlInternalConnectionTds has pending data.");

if (_fResetConnection)
{
Expand Down