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

Merge common code bases for TdsParserStateObject.cs (4) #2254

Merged
merged 19 commits into from
Jan 17, 2024
Merged

Merge common code bases for TdsParserStateObject.cs (4) #2254

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
40d1351
Merge NullBitmap struct of TdsParserStateObject.
panoskj Nov 29, 2023
cae8c3f
Merge "Value Skip Logic" methods of TdsParserStateObject.
panoskj Nov 29, 2023
fde2178
Add RuntimeHelpers.PrepareConstrainedRegions stub in TdsParserStateOb…
panoskj Nov 29, 2023
53e4333
Refactoring of ReadSni in TdsParserStateObject.netcore.cs.
panoskj Nov 29, 2023
9076c6a
Add "missing" RuntimeHelpers.PrepareConstrainedRegions with try/final…
panoskj Nov 29, 2023
cedc2b5
Remove unreachable code from TdsParserStateObject.netfx.cs (condition…
panoskj Nov 29, 2023
40a355c
Replace Thread.MemoryBarrier with Interlocked.MemoryBarrier in TdsPar…
panoskj Nov 29, 2023
2a9558d
Add PacketHandle as an alias of IntPtr in TdsParserStateObject.netfx.…
panoskj Nov 29, 2023
6a8f0d9
Add TdsParserStateObjectFactory.UseManagedSNI, ReadSyncOverAsync and …
panoskj Nov 29, 2023
e22c4e9
Add SessionHandle and ReadAsync in TdsParserStateObject.netfx.cs.
panoskj Nov 29, 2023
4a9ba4e
Port ADP.OnTimeoutAsync to netfx (fixes an edge case bug regarding As…
panoskj Nov 29, 2023
a9b5585
Add CheckConnection in TdsParserStateObject.netfx.cs.
panoskj Nov 29, 2023
e6cb4ab
Whitespace changes. Add a missing private access modifier in TdsParse…
panoskj Dec 4, 2023
0f14302
Merge most "Network/Packet Reading & Processing" methods of TdsParser…
panoskj Dec 4, 2023
e695cc7
NIT: Fix code formatting in UnsafeCreateTimer method.
panoskj Jan 11, 2024
7ca012a
Update src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsP…
panoskj Jan 17, 2024
832e378
Update src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsP…
panoskj Jan 17, 2024
240e242
Update src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsP…
panoskj Jan 17, 2024
619bb4c
Removed dead code.
panoskj Jan 17, 2024
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
152 changes: 69 additions & 83 deletions ...osoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,17 @@ namespace Microsoft.Data.SqlClient
{
internal abstract partial class TdsParserStateObject
{
private struct RuntimeHelpers
{
/// <summary>
/// This is a no-op in netcore version. Only needed for merging with netfx codebase.
/// </summary>
[Conditional("NETFRAMEWORK")]
internal static void PrepareConstrainedRegions()
{
}
}
Comment on lines +18 to +27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the way you addressed the code differences. 👏🏼


private static readonly ContextCallback s_readAsyncCallbackComplete = ReadAsyncCallbackComplete;

// Timeout variables
Expand Down Expand Up @@ -76,34 +87,6 @@ internal abstract SessionHandle SessionHandle
get;
}

private partial struct NullBitmap
{
internal bool TryInitialize(TdsParserStateObject stateObj, int columnsCount)
{
_columnsCount = columnsCount;
// 1-8 columns need 1 byte
// 9-16: 2 bytes, and so on
int bitmapArrayLength = (columnsCount + 7) / 8;

// allow reuse of previously allocated bitmap
if (_nullBitmap == null || _nullBitmap.Length != bitmapArrayLength)
{
_nullBitmap = new byte[bitmapArrayLength];
}

// read the null bitmap compression information from TDS
if (!stateObj.TryReadByteArray(_nullBitmap, _nullBitmap.Length))
{
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}, NBCROW bitmap data. Null Bitmap {1}, Null bitmap length: {2}", stateObj.ObjectID, _nullBitmap, (ushort)_nullBitmap.Length);

return true;
}
}

/////////////////////
// General methods //
/////////////////////
Expand Down Expand Up @@ -278,38 +261,6 @@ internal void StartSession(object cancellationOwner)
_cancellationOwner.Target = cancellationOwner;
}

/////////////////////////////////////////
// Value Skip Logic //
/////////////////////////////////////////


// Reads bytes from the buffer but doesn't return them, in effect simply deleting them.
// Does not handle plp fields, need to use SkipPlpBytesValue for those.
// Does not handle null values or NBC bitmask, ensure the value is not null before calling this method
internal bool TrySkipLongBytes(long num)
{
Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async");

while (num > 0)
{
int cbSkip = (int)Math.Min(int.MaxValue, num);
if (!TryReadByteArray(Span<byte>.Empty, cbSkip))
{
return false;
}
num -= cbSkip;
}

return true;
}

// Reads bytes from the buffer but doesn't return them, in effect simply deleting them.
internal bool TrySkipBytes(int num)
{
Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async");
return TryReadByteArray(Span<byte>.Empty, num);
}

/////////////////////////////////////////
// Network/Packet Reading & Processing //
/////////////////////////////////////////
Expand All @@ -320,6 +271,8 @@ internal bool TrySkipBytes(int num)

internal bool TryReadNetworkPacket()
{
TdsParser.ReliabilitySection.Assert("unreliable call to TryReadNetworkPacket"); // you need to setup for a thread abort somewhere before you call this method

#if DEBUG
Debug.Assert(!_shouldHaveEnoughData || _attentionSent, "Caller said there should be enough data, but we are currently reading a packet");
#endif
Expand Down Expand Up @@ -399,9 +352,12 @@ internal void ReadSniSyncOverAsync()

uint error;

RuntimeHelpers.PrepareConstrainedRegions();
bool shouldDecrement = false;
try
{
TdsParser.ReliabilitySection.Assert("unreliable call to ReadSniSync"); // you need to setup for a thread abort somewhere before you call this method

Interlocked.Increment(ref _readingCount);
shouldDecrement = true;

Expand Down Expand Up @@ -599,6 +555,7 @@ private bool OnTimeoutCore(int expectedState, int targetState, bool asyncClose =
if (!source.Task.IsCompleted)
{
int pendingCallback = IncrementPendingCallbacks();
RuntimeHelpers.PrepareConstrainedRegions();
try
{
// If pendingCallback is at 3, then ReadAsyncCallback hasn't been called yet
Expand Down Expand Up @@ -673,6 +630,7 @@ internal void ReadSni(TaskCompletionSource<object> completion)

uint error = 0;

RuntimeHelpers.PrepareConstrainedRegions();
try
{
Debug.Assert(completion != null, "Async on but null asyncResult passed");
Expand Down Expand Up @@ -707,22 +665,30 @@ internal void ReadSni(TaskCompletionSource<object> completion)
ChangeNetworkPacketTimeout(msecsRemaining, Timeout.Infinite);
}

Interlocked.Increment(ref _readingCount);
SessionHandle handle = default;

SessionHandle handle = SessionHandle;
if (!handle.IsNull)
RuntimeHelpers.PrepareConstrainedRegions();
try
{ }
finally
{
IncrementPendingCallbacks();

readPacket = ReadAsync(handle, out error);
Interlocked.Increment(ref _readingCount);

if (!(TdsEnums.SNI_SUCCESS == error || TdsEnums.SNI_SUCCESS_IO_PENDING == error))
handle = SessionHandle;
if (!handle.IsNull)
{
DecrementPendingCallbacks(false); // Failure - we won't receive callback!
IncrementPendingCallbacks();

readPacket = ReadAsync(handle, out error);

if (!(TdsEnums.SNI_SUCCESS == error || TdsEnums.SNI_SUCCESS_IO_PENDING == error))
{
DecrementPendingCallbacks(false); // Failure - we won't receive callback!
}
}
}

Interlocked.Decrement(ref _readingCount);
Interlocked.Decrement(ref _readingCount);
}

if (handle.IsNull)
{
Expand Down Expand Up @@ -815,24 +781,34 @@ internal bool IsConnectionAlive(bool throwOnException)
else
{
uint error;
SniContext = SniContext.Snix_Connect;

error = CheckConnection();
if ((error != TdsEnums.SNI_SUCCESS) && (error != TdsEnums.SNI_WAIT_TIMEOUT))
RuntimeHelpers.PrepareConstrainedRegions();
try
{
// Connection is dead
SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObject.IsConnectionAlive | Info | State Object Id {0}, received error {1} on idle connection", _objectID, (int)error);
isAlive = false;
if (throwOnException)
TdsParser.ReliabilitySection.Assert("unreliable call to IsConnectionAlive"); // you need to setup for a thread abort somewhere before you call this method

SniContext = SniContext.Snix_Connect;

error = CheckConnection();
if ((error != TdsEnums.SNI_SUCCESS) && (error != TdsEnums.SNI_WAIT_TIMEOUT))
{
// Get the error from SNI so that we can throw the correct exception
AddError(_parser.ProcessSNIError(this));
ThrowExceptionAndWarning();
// Connection is dead
SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObject.IsConnectionAlive | Info | State Object Id {0}, received error {1} on idle connection", _objectID, (int)error);
isAlive = false;
if (throwOnException)
{
// Get the error from SNI so that we can throw the correct exception
AddError(_parser.ProcessSNIError(this));
ThrowExceptionAndWarning();
}
}
else
{
_lastSuccessfulIOTimer._value = DateTime.UtcNow.Ticks;
}
}
else
finally
{
_lastSuccessfulIOTimer._value = DateTime.UtcNow.Ticks;
}
}
}
Expand Down Expand Up @@ -875,6 +851,8 @@ internal bool ValidateSNIConnection()
// This method should only be called by ReadSni! If not - it may have problems with timeouts!
private void ReadSniError(TdsParserStateObject stateObj, uint error)
{
TdsParser.ReliabilitySection.Assert("unreliable call to ReadSniSyncError"); // you need to setup for a thread abort somewhere before you call this method

if (TdsEnums.SNI_WAIT_TIMEOUT == error)
{
Debug.Assert(_syncOverAsync, "Should never reach here with async on!");
Expand All @@ -897,7 +875,7 @@ private void ReadSniError(TdsParserStateObject stateObj, uint error)
stateObj.SendAttention(mustTakeWriteLock: true);

PacketHandle syncReadPacket = default;

RuntimeHelpers.PrepareConstrainedRegions();
bool shouldDecrement = false;
try
{
Expand Down Expand Up @@ -1097,6 +1075,7 @@ public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error)
return;
}

RuntimeHelpers.PrepareConstrainedRegions();
bool processFinallyBlock = true;
try
{
Expand Down Expand Up @@ -1318,6 +1297,8 @@ public void WriteAsyncCallback(IntPtr key, PacketHandle packet, uint sniError)
//
internal void WriteSecureString(SecureString secureString)
{
TdsParser.ReliabilitySection.Assert("unreliable call to WriteSecureString"); // you need to setup for a thread abort somewhere before you call this method

Debug.Assert(_securePasswords[0] == null || _securePasswords[1] == null, "There are more than two secure passwords");

int index = _securePasswords[0] != null ? 1 : 0;
Expand Down Expand Up @@ -1392,6 +1373,8 @@ internal Task WaitForAccumulatedWrites()
// and then the buffer is re-initialized in flush() and then the byte is put in the buffer.
internal void WriteByte(byte b)
{
TdsParser.ReliabilitySection.Assert("unreliable call to WriteByte"); // you need to setup for a thread abort somewhere before you call this method

Debug.Assert(_outBytesUsed <= _outBuff.Length, "ERROR - TDSParser: _outBytesUsed > _outBuff.Length");

// check to make sure we haven't used the full amount of space available in the buffer, if so, flush it
Expand Down Expand Up @@ -1427,6 +1410,8 @@ private Task WriteBytes(ReadOnlySpan<byte> b, int len, int offsetBuffer, bool ca
}
try
{
TdsParser.ReliabilitySection.Assert("unreliable call to WriteByteArray"); // you need to setup for a thread abort somewhere before you call this method

bool async = _parser._asyncWrite; // NOTE: We are capturing this now for the assert after the Task is returned, since WritePacket will turn off async if there is an exception
Debug.Assert(async || _asyncWriteCount == 0);
// Do we have to send out in packet size chunks, or can we rely on netlib layer to break it up?
Expand Down Expand Up @@ -1789,6 +1774,7 @@ internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = fa

PacketHandle attnPacket = CreateAndSetAttentionPacket();

RuntimeHelpers.PrepareConstrainedRegions();
try
{
// Dev11 #344723: SqlClient stress test suspends System_Data!Tcp::ReadSync via a call to SqlDataReader::Close
Expand Down
69 changes: 1 addition & 68 deletions ...Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,34 +106,6 @@ internal uint Status
}
}

private partial struct NullBitmap
{
internal bool TryInitialize(TdsParserStateObject stateObj, int columnsCount)
{
_columnsCount = columnsCount;
// 1-8 columns need 1 byte
// 9-16: 2 bytes, and so on
int bitmapArrayLength = (columnsCount + 7) / 8;

// allow reuse of previously allocated bitmap
if (_nullBitmap == null || _nullBitmap.Length != bitmapArrayLength)
{
_nullBitmap = new byte[bitmapArrayLength];
}

// read the null bitmap compression information from TDS
if (!stateObj.TryReadByteArray(_nullBitmap, _nullBitmap.Length))
{
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}, NBCROW bitmap data. Null Bitmap {1}, Null bitmap length: {2}", stateObj.ObjectID, _nullBitmap, (ushort)_nullBitmap.Length);

return true;
}
}

/////////////////////
// General methods //
/////////////////////
Expand Down Expand Up @@ -372,38 +344,6 @@ internal void StartSession(int objectID)
_allowObjectID = objectID;
}

/////////////////////////////////////////
// Value Skip Logic //
/////////////////////////////////////////


// Reads bytes from the buffer but doesn't return them, in effect simply deleting them.
// Does not handle plp fields, need to use SkipPlpBytesValue for those.
// Does not handle null values or NBC bitmask, ensure the value is not null before calling this method
internal bool TrySkipLongBytes(long num)
{
Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async");

while (num > 0)
{
int cbSkip = (int)Math.Min(int.MaxValue, num);
if (!TryReadByteArray(Span<byte>.Empty, cbSkip))
{
return false;
}
num -= cbSkip;
}

return true;
}

// Reads bytes from the buffer but doesn't return them, in effect simply deleting them.
internal bool TrySkipBytes(int num)
{
Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async");
return TryReadByteArray(Span<byte>.Empty, num);
}

/////////////////////////////////////////
// Network/Packet Reading & Processing //
/////////////////////////////////////////
Expand Down Expand Up @@ -916,7 +856,6 @@ internal bool IsConnectionAlive(bool throwOnException)
else
{
uint error;
IntPtr readPacket = IntPtr.Zero;

RuntimeHelpers.PrepareConstrainedRegions();
try
Expand Down Expand Up @@ -947,12 +886,6 @@ internal bool IsConnectionAlive(bool throwOnException)
}
finally
{
if (readPacket != IntPtr.Zero)
{
// Be sure to release packet, otherwise it will be leaked by native.
SNINativeMethodWrapper.SNIPacketRelease(readPacket);
Copy link
Contributor Author

@panoskj panoskj Dec 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this code is unreachable, the enclosing try/finally serves no purpose other than whatever RuntimeHelpers.PrepareConstrainedRegions does. I have preserved the try/finally (by adding it in .net core) but I suspect it can be removed, because according to the documentation (emphasis mine):

The PrepareConstrainedRegions method must immediately precede a try block and marks catch, finally, and fault blocks as constrained execution regions.

If I understand correctly, RuntimeHelpers.PrepareConstrainedRegions only affects catch/finally and has no effect if they are empty.

}

}
}
}
Expand Down Expand Up @@ -1837,7 +1770,7 @@ private Task SNIWritePacket(SNIHandle handle, SNIPacket packet, out uint sniErro
return task;
}

#pragma warning restore 420
#pragma warning restore 420

// Sends an attention signal - executing thread will consume attn.
internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = false)
Expand Down
Loading