From 840af41ff75cc425cfbcebe2210ae1470ee2d912 Mon Sep 17 00:00:00 2001 From: panoskj Date: Fri, 18 Mar 2022 22:27:27 +0200 Subject: [PATCH 01/22] Merging "Buffer read" methods of TdsParserStateObject (note: includes parts of #667). --- .../Data/SqlClient/TdsParserStateObject.cs | 616 ---------------- .../Data/SqlClient/TdsParserStateObject.cs | 652 ----------------- .../Data/SqlClient/TdsParserStateObject.cs | 656 ++++++++++++++++++ 3 files changed, 656 insertions(+), 1268 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 1565a618a2..16947160f1 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -333,622 +333,6 @@ internal bool HasReceivedColumnMetadata // Buffer read methods - data values // /////////////////////////////////////// - // look at the next byte without pulling it off the wire, don't just return _inBytesUsed since we may - // have to go to the network to get the next byte. - internal bool TryPeekByte(out byte value) - { - if (!TryReadByte(out value)) - { - return false; - } - - // now do fixup - _inBytesPacket++; - _inBytesUsed--; - - AssertValidState(); - return true; - } - - // Takes a byte array, an offset, and a len and fills the array from the offset to len number of - // bytes from the in buffer. - public bool TryReadByteArray(Span buff, int len) - { - return TryReadByteArray(buff, len, out _); - } - - // NOTE: This method must be retriable WITHOUT replaying a snapshot - // Every time you call this method increment the offset and decrease len by the value of totalRead - public bool TryReadByteArray(Span buff, int len, out int totalRead) - { - totalRead = 0; - -#if DEBUG - if (_snapshot != null && _snapshot.DoPend()) - { - _networkPacketTaskSource = new TaskCompletionSource(); - Interlocked.MemoryBarrier(); - - if (s_forcePendingReadsToWaitForUser) - { - _realNetworkPacketTaskSource = new TaskCompletionSource(); - _realNetworkPacketTaskSource.SetResult(null); - } - else - { - _networkPacketTaskSource.TrySetResult(null); - } - return false; - } -#endif - - Debug.Assert(buff == null || buff.Length >= len, "Invalid length sent to ReadByteArray()!"); - - // loop through and read up to array length - while (len > 0) - { - if ((_inBytesPacket == 0) || (_inBytesUsed == _inBytesRead)) - { - if (!TryPrepareBuffer()) - { - return false; - } - } - - int bytesToRead = Math.Min(len, Math.Min(_inBytesPacket, _inBytesRead - _inBytesUsed)); - Debug.Assert(bytesToRead > 0, "0 byte read in TryReadByteArray"); - if (!buff.IsEmpty) - { - ReadOnlySpan copyFrom = new ReadOnlySpan(_inBuff, _inBytesUsed, bytesToRead); - Span copyTo = buff.Slice(totalRead, bytesToRead); - copyFrom.CopyTo(copyTo); - } - - totalRead += bytesToRead; - _inBytesUsed += bytesToRead; - _inBytesPacket -= bytesToRead; - len -= bytesToRead; - - AssertValidState(); - } - - return true; - } - - // Takes no arguments and returns a byte from the buffer. If the buffer is empty, it is filled - // before the byte is returned. - internal bool TryReadByte(out byte value) - { - Debug.Assert(_inBytesUsed >= 0 && _inBytesUsed <= _inBytesRead, "ERROR - TDSParser: _inBytesUsed < 0 or _inBytesUsed > _inBytesRead"); - value = 0; - -#if DEBUG - if (_snapshot != null && _snapshot.DoPend()) - { - _networkPacketTaskSource = new TaskCompletionSource(); - Interlocked.MemoryBarrier(); - - if (s_forcePendingReadsToWaitForUser) - { - _realNetworkPacketTaskSource = new TaskCompletionSource(); - _realNetworkPacketTaskSource.SetResult(null); - } - else - { - _networkPacketTaskSource.TrySetResult(null); - } - return false; - } -#endif - - if ((_inBytesPacket == 0) || (_inBytesUsed == _inBytesRead)) - { - if (!TryPrepareBuffer()) - { - return false; - } - } - - // decrement the number of bytes left in the packet - _inBytesPacket--; - - Debug.Assert(_inBytesPacket >= 0, "ERROR - TDSParser: _inBytesPacket < 0"); - - // return the byte from the buffer and increment the counter for number of bytes used in the in buffer - value = (_inBuff[_inBytesUsed++]); - - AssertValidState(); - return true; - } - - internal bool TryReadChar(out char value) - { - Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - - Span buffer = stackalloc byte[2]; - if (((_inBytesUsed + 2) > _inBytesRead) || (_inBytesPacket < 2)) - { - // If the char isn't fully in the buffer, or if it isn't fully in the packet, - // then use ReadByteArray since the logic is there to take care of that. - if (!TryReadByteArray(buffer, 2)) - { - value = '\0'; - return false; - } - } - else - { - // The entire char is in the packet and in the buffer, so just return it - // and take care of the counters. - buffer = _inBuff.AsSpan(_inBytesUsed, 2); - _inBytesUsed += 2; - _inBytesPacket -= 2; - } - - AssertValidState(); - value = (char)((buffer[1] << 8) + buffer[0]); - return true; - } - - internal bool TryReadInt16(out short value) - { - Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - - Span buffer = stackalloc byte[2]; - if (((_inBytesUsed + 2) > _inBytesRead) || (_inBytesPacket < 2)) - { - // If the int16 isn't fully in the buffer, or if it isn't fully in the packet, - // then use ReadByteArray since the logic is there to take care of that. - if (!TryReadByteArray(buffer, 2)) - { - value = default; - return false; - } - } - else - { - // The entire int16 is in the packet and in the buffer, so just return it - // and take care of the counters. - buffer = _inBuff.AsSpan(_inBytesUsed, 2); - _inBytesUsed += 2; - _inBytesPacket -= 2; - } - - AssertValidState(); - value = (short)((buffer[1] << 8) + buffer[0]); - return true; - } - - internal bool TryReadInt32(out int value) - { - Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - Span buffer = stackalloc byte[4]; - if (((_inBytesUsed + 4) > _inBytesRead) || (_inBytesPacket < 4)) - { - // If the int isn't fully in the buffer, or if it isn't fully in the packet, - // then use ReadByteArray since the logic is there to take care of that. - if (!TryReadByteArray(buffer, 4)) - { - value = 0; - return false; - } - } - else - { - // The entire int is in the packet and in the buffer, so just return it - // and take care of the counters. - buffer = _inBuff.AsSpan(_inBytesUsed, 4); - _inBytesUsed += 4; - _inBytesPacket -= 4; - } - - 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 - internal bool TryReadInt64(out long value) - { - if ((_inBytesPacket == 0) || (_inBytesUsed == _inBytesRead)) - { - if (!TryPrepareBuffer()) - { - value = 0; - return false; - } - } - - if ((_bTmpRead > 0) || (((_inBytesUsed + 8) > _inBytesRead) || (_inBytesPacket < 8))) - { - // If the long isn't fully in the buffer, or if it isn't fully in the packet, - // then use ReadByteArray since the logic is there to take care of that. - - int bytesRead; - if (!TryReadByteArray(_bTmp.AsSpan(_bTmpRead), 8 - _bTmpRead, out bytesRead)) - { - Debug.Assert(_bTmpRead + bytesRead <= 8, "Read more data than required"); - _bTmpRead += bytesRead; - value = 0; - return false; - } - else - { - Debug.Assert(_bTmpRead + bytesRead == 8, "TryReadByteArray returned true without reading all data required"); - _bTmpRead = 0; - AssertValidState(); - value = BitConverter.ToInt64(_bTmp, 0); - return true; - } - } - else - { - // The entire long is in the packet and in the buffer, so just return it - // and take care of the counters. - - value = BitConverter.ToInt64(_inBuff, _inBytesUsed); - - _inBytesUsed += 8; - _inBytesPacket -= 8; - - AssertValidState(); - return true; - } - } - - internal bool TryReadUInt16(out ushort value) - { - Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - - Span buffer = stackalloc byte[2]; - if (((_inBytesUsed + 2) > _inBytesRead) || (_inBytesPacket < 2)) - { - // If the uint16 isn't fully in the buffer, or if it isn't fully in the packet, - // then use ReadByteArray since the logic is there to take care of that. - if (!TryReadByteArray(buffer, 2)) - { - value = default; - return false; - } - } - else - { - // The entire uint16 is in the packet and in the buffer, so just return it - // and take care of the counters. - buffer = _inBuff.AsSpan(_inBytesUsed, 2); - _inBytesUsed += 2; - _inBytesPacket -= 2; - } - - AssertValidState(); - value = (ushort)((buffer[1] << 8) + buffer[0]); - return true; - } - - // This method is safe to call when doing async without replay - internal bool TryReadUInt32(out uint value) - { - if ((_inBytesPacket == 0) || (_inBytesUsed == _inBytesRead)) - { - if (!TryPrepareBuffer()) - { - value = 0; - return false; - } - } - - if ((_bTmpRead > 0) || (((_inBytesUsed + 4) > _inBytesRead) || (_inBytesPacket < 4))) - { - // If the int isn't fully in the buffer, or if it isn't fully in the packet, - // then use ReadByteArray since the logic is there to take care of that. - - int bytesRead; - if (!TryReadByteArray(_bTmp.AsSpan(_bTmpRead), 4 - _bTmpRead, out bytesRead)) - { - Debug.Assert(_bTmpRead + bytesRead <= 4, "Read more data than required"); - _bTmpRead += bytesRead; - value = 0; - return false; - } - else - { - Debug.Assert(_bTmpRead + bytesRead == 4, "TryReadByteArray returned true without reading all data required"); - _bTmpRead = 0; - AssertValidState(); - value = BitConverter.ToUInt32(_bTmp, 0); - return true; - } - } - else - { - // The entire int is in the packet and in the buffer, so just return it - // and take care of the counters. - - value = BitConverter.ToUInt32(_inBuff, _inBytesUsed); - - _inBytesUsed += 4; - _inBytesPacket -= 4; - - AssertValidState(); - return true; - } - } - - internal bool TryReadSingle(out float value) - { - Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - if (((_inBytesUsed + 4) > _inBytesRead) || (_inBytesPacket < 4)) - { - // If the float isn't fully in the buffer, or if it isn't fully in the packet, - // then use ReadByteArray since the logic is there to take care of that. - - if (!TryReadByteArray(_bTmp, 4)) - { - value = default; - return false; - } - - AssertValidState(); - value = BitConverter.ToSingle(_bTmp, 0); - return true; - } - else - { - // The entire float is in the packet and in the buffer, so just return it - // and take care of the counters. - - value = BitConverter.ToSingle(_inBuff, _inBytesUsed); - - _inBytesUsed += 4; - _inBytesPacket -= 4; - - AssertValidState(); - return true; - } - } - - internal bool TryReadDouble(out double value) - { - Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - if (((_inBytesUsed + 8) > _inBytesRead) || (_inBytesPacket < 8)) - { - // If the double isn't fully in the buffer, or if it isn't fully in the packet, - // then use ReadByteArray since the logic is there to take care of that. - - if (!TryReadByteArray(_bTmp, 8)) - { - value = default; - return false; - } - - AssertValidState(); - value = BitConverter.ToDouble(_bTmp, 0); - return true; - } - else - { - // The entire double is in the packet and in the buffer, so just return it - // and take care of the counters. - - value = BitConverter.ToDouble(_inBuff, _inBytesUsed); - - _inBytesUsed += 8; - _inBytesPacket -= 8; - - AssertValidState(); - return true; - } - } - - internal bool TryReadString(int length, out string value) - { - Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - int cBytes = length << 1; - byte[] buf; - int offset = 0; - - if (((_inBytesUsed + cBytes) > _inBytesRead) || (_inBytesPacket < cBytes)) - { - if (_bTmp == null || _bTmp.Length < cBytes) - { - _bTmp = new byte[cBytes]; - } - - if (!TryReadByteArray(_bTmp, cBytes)) - { - value = null; - return false; - } - - // assign local to point to parser scratch buffer - buf = _bTmp; - - AssertValidState(); - } - else - { - // assign local to point to _inBuff - buf = _inBuff; - offset = _inBytesUsed; - _inBytesUsed += cBytes; - _inBytesPacket -= cBytes; - - AssertValidState(); - } - - value = System.Text.Encoding.Unicode.GetString(buf, offset, cBytes); - return true; - } - - internal bool TryReadStringWithEncoding(int length, System.Text.Encoding encoding, bool isPlp, out string value) - { - Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - - if (null == encoding) - { - // Need to skip the current column before throwing the error - this ensures that the state shared between this and the data reader is consistent when calling DrainData - if (isPlp) - { - if (!_parser.TrySkipPlpValue((ulong)length, this, out _)) - { - value = null; - return false; - } - } - else - { - if (!TrySkipBytes(length)) - { - value = null; - return false; - } - } - - _parser.ThrowUnsupportedCollationEncountered(this); - } - byte[] buf = null; - int offset = 0; - - if (isPlp) - { - if (!TryReadPlpBytes(ref buf, 0, int.MaxValue, out length)) - { - value = null; - return false; - } - - AssertValidState(); - } - else - { - if (((_inBytesUsed + length) > _inBytesRead) || (_inBytesPacket < length)) - { - if (_bTmp == null || _bTmp.Length < length) - { - _bTmp = new byte[length]; - } - - if (!TryReadByteArray(_bTmp, length)) - { - value = null; - return false; - } - - // assign local to point to parser scratch buffer - buf = _bTmp; - - AssertValidState(); - } - else - { - // assign local to point to _inBuff - buf = _inBuff; - offset = _inBytesUsed; - _inBytesUsed += length; - _inBytesPacket -= length; - - AssertValidState(); - } - } - - // BCL optimizes to not use char[] underneath - value = encoding.GetString(buf, offset, length); - return true; - } - - internal ulong ReadPlpLength(bool returnPlpNullIfNull) - { - ulong value; - Debug.Assert(_syncOverAsync, "Should not attempt pends in a synchronous call"); - bool result = TryReadPlpLength(returnPlpNullIfNull, out value); - if (!result) - { - throw SQL.SynchronousCallMayNotPend(); - } - return value; - } - - // Reads the length of either the entire data or the length of the next chunk in a - // partially length prefixed data - // After this call, call ReadPlpBytes/ReadPlpUnicodeChars until the specified length of data - // is consumed. Repeat this until ReadPlpLength returns 0 in order to read the - // entire stream. - // When this function returns 0, it means the data stream is read completely and the - // plp state in the tdsparser is cleaned. - internal bool TryReadPlpLength(bool returnPlpNullIfNull, out ulong lengthLeft) - { - uint chunklen; - // bool firstchunk = false; - bool isNull = false; - - Debug.Assert(_longlenleft == 0, "Out of synch length read request"); - if (_longlen == 0) - { - // First chunk is being read. Find out what type of chunk it is - long value; - if (!TryReadInt64(out value)) - { - lengthLeft = 0; - return false; - } - _longlen = (ulong)value; - // firstchunk = true; - } - - if (_longlen == TdsEnums.SQL_PLP_NULL) - { - _longlen = 0; - _longlenleft = 0; - isNull = true; - } - else - { - // Data is coming in uint chunks, read length of next chunk - if (!TryReadUInt32(out chunklen)) - { - lengthLeft = 0; - return false; - } - if (chunklen == TdsEnums.SQL_PLP_CHUNK_TERMINATOR) - { - _longlenleft = 0; - _longlen = 0; - } - else - { - _longlenleft = chunklen; - } - } - - AssertValidState(); - - if (isNull && returnPlpNullIfNull) - { - lengthLeft = TdsEnums.SQL_PLP_NULL; - return true; - } - - lengthLeft = _longlenleft; - return true; - } - - internal int ReadPlpBytesChunk(byte[] buff, int offset, int len) - { - Debug.Assert(_syncOverAsync, "Should not attempt pends in a synchronous call"); - Debug.Assert(_longlenleft > 0, "Read when no data available"); - - int value; - int bytesToRead = (int)Math.Min(_longlenleft, (ulong)len); - bool result = TryReadByteArray(buff.AsSpan(offset), bytesToRead, out value); - _longlenleft -= (ulong)bytesToRead; - if (!result) - { - throw SQL.SynchronousCallMayNotPend(); - } - return value; - } - // Reads the requested number of bytes from a plp data stream, or the entire data if // requested length is -1 or larger than the actual length of data. First call to this method // should be preceeded by a call to ReadPlpLength or ReadDataLength. diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index d8171c63b6..dabad6d98e 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -419,658 +419,6 @@ internal void StartSession(int objectID) // Buffer read methods - data values // /////////////////////////////////////// - // look at the next byte without pulling it off the wire, don't just returun _inBytesUsed since we may - // have to go to the network to get the next byte. - internal bool TryPeekByte(out byte value) - { - if (!TryReadByte(out value)) - { - return false; - } - - // now do fixup - _inBytesPacket++; - _inBytesUsed--; - - AssertValidState(); - return true; - } - - // Takes a byte array, an offset, and a len and fills the array from the offset to len number of - // bytes from the in buffer. - public bool TryReadByteArray(Span buff, int len) - { - return TryReadByteArray(buff, len, out _); - } - - // NOTE: This method must be retriable WITHOUT replaying a snapshot - // Every time you call this method increment the offset and decrease len by the value of totalRead - public bool TryReadByteArray(Span buff, int len, out int totalRead) - { - TdsParser.ReliabilitySection.Assert("unreliable call to ReadByteArray"); // you need to setup for a thread abort somewhere before you call this method - totalRead = 0; - -#if DEBUG - if (_snapshot != null && _snapshot.DoPend()) - { - _networkPacketTaskSource = new TaskCompletionSource(); - Thread.MemoryBarrier(); - - if (s_forcePendingReadsToWaitForUser) - { - _realNetworkPacketTaskSource = new TaskCompletionSource(); - _realNetworkPacketTaskSource.SetResult(null); - } - else - { - _networkPacketTaskSource.TrySetResult(null); - } - return false; - } -#endif - - Debug.Assert(buff.IsEmpty || buff.Length >= len, "Invalid length sent to ReadByteArray()!"); - - // loop through and read up to array length - while (len > 0) - { - if ((_inBytesPacket == 0) || (_inBytesUsed == _inBytesRead)) - { - if (!TryPrepareBuffer()) - { - return false; - } - } - - int bytesToRead = Math.Min(len, Math.Min(_inBytesPacket, _inBytesRead - _inBytesUsed)); - Debug.Assert(bytesToRead > 0, "0 byte read in TryReadByteArray"); - if (!buff.IsEmpty) - { - ReadOnlySpan copyFrom = new ReadOnlySpan(_inBuff, _inBytesUsed, bytesToRead); - Span copyTo = buff.Slice(totalRead, bytesToRead); - copyFrom.CopyTo(copyTo); - } - - totalRead += bytesToRead; - _inBytesUsed += bytesToRead; - _inBytesPacket -= bytesToRead; - len -= bytesToRead; - - AssertValidState(); - } - - return true; - } - - // Takes no arguments and returns a byte from the buffer. If the buffer is empty, it is filled - // before the byte is returned. - internal bool TryReadByte(out byte value) - { - TdsParser.ReliabilitySection.Assert("unreliable call to ReadByte"); // you need to setup for a thread abort somewhere before you call this method - Debug.Assert(_inBytesUsed >= 0 && _inBytesUsed <= _inBytesRead, "ERROR - TDSParser: _inBytesUsed < 0 or _inBytesUsed > _inBytesRead"); - value = 0; - -#if DEBUG - if (_snapshot != null && _snapshot.DoPend()) - { - _networkPacketTaskSource = new TaskCompletionSource(); - Thread.MemoryBarrier(); - - if (s_forcePendingReadsToWaitForUser) - { - _realNetworkPacketTaskSource = new TaskCompletionSource(); - _realNetworkPacketTaskSource.SetResult(null); - } - else - { - _networkPacketTaskSource.TrySetResult(null); - } - return false; - } -#endif - - if ((_inBytesPacket == 0) || (_inBytesUsed == _inBytesRead)) - { - if (!TryPrepareBuffer()) - { - return false; - } - } - - // decrement the number of bytes left in the packet - _inBytesPacket--; - - Debug.Assert(_inBytesPacket >= 0, "ERROR - TDSParser: _inBytesPacket < 0"); - - // return the byte from the buffer and increment the counter for number of bytes used in the in buffer - value = (_inBuff[_inBytesUsed++]); - - AssertValidState(); - return true; - } - - internal bool TryReadChar(out char value) - { - Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - - byte[] buffer; - int offset; - if (((_inBytesUsed + 2) > _inBytesRead) || (_inBytesPacket < 2)) - { - // If the char isn't fully in the buffer, or if it isn't fully in the packet, - // then use ReadByteArray since the logic is there to take care of that. - - if (!TryReadByteArray(_bTmp, 2)) - { - value = '\0'; - return false; - } - - buffer = _bTmp; - offset = 0; - } - else - { - // The entire char is in the packet and in the buffer, so just return it - // and take care of the counters. - - buffer = _inBuff; - offset = _inBytesUsed; - - _inBytesUsed += 2; - _inBytesPacket -= 2; - } - - AssertValidState(); - value = (char)((buffer[offset + 1] << 8) + buffer[offset]); - return true; - } - - internal bool TryReadInt16(out short value) - { - Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - - byte[] buffer; - int offset; - if (((_inBytesUsed + 2) > _inBytesRead) || (_inBytesPacket < 2)) - { - // If the int16 isn't fully in the buffer, or if it isn't fully in the packet, - // then use ReadByteArray since the logic is there to take care of that. - - if (!TryReadByteArray(_bTmp, 2)) - { - value = default; - return false; - } - - buffer = _bTmp; - offset = 0; - } - else - { - // The entire int16 is in the packet and in the buffer, so just return it - // and take care of the counters. - - buffer = _inBuff; - offset = _inBytesUsed; - - _inBytesUsed += 2; - _inBytesPacket -= 2; - } - - AssertValidState(); - value = (short)((buffer[offset + 1] << 8) + buffer[offset]); - return true; - } - - internal bool TryReadInt32(out int value) - { - TdsParser.ReliabilitySection.Assert("unreliable call to ReadInt32"); // you need to setup for a thread abort somewhere before you call this method - Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - if (((_inBytesUsed + 4) > _inBytesRead) || (_inBytesPacket < 4)) - { - // If the int isn't fully in the buffer, or if it isn't fully in the packet, - // then use ReadByteArray since the logic is there to take care of that. - - if (!TryReadByteArray(_bTmp, 4)) - { - value = 0; - return false; - } - - AssertValidState(); - value = BitConverter.ToInt32(_bTmp, 0); - return true; - } - else - { - // The entire int is in the packet and in the buffer, so just return it - // and take care of the counters. - - value = BitConverter.ToInt32(_inBuff, _inBytesUsed); - - _inBytesUsed += 4; - _inBytesPacket -= 4; - - AssertValidState(); - return true; - } - } - - // This method is safe to call when doing async without snapshot - internal bool TryReadInt64(out long value) - { - TdsParser.ReliabilitySection.Assert("unreliable call to ReadInt64"); // you need to setup for a thread abort somewhere before you call this method - if ((_inBytesPacket == 0) || (_inBytesUsed == _inBytesRead)) - { - if (!TryPrepareBuffer()) - { - value = 0; - return false; - } - } - - if ((_bTmpRead > 0) || (((_inBytesUsed + 8) > _inBytesRead) || (_inBytesPacket < 8))) - { - // If the long isn't fully in the buffer, or if it isn't fully in the packet, - // then use ReadByteArray since the logic is there to take care of that. - - int bytesRead; - if (!TryReadByteArray(_bTmp.AsSpan(start: _bTmpRead), 8 - _bTmpRead, out bytesRead)) - { - Debug.Assert(_bTmpRead + bytesRead <= 8, "Read more data than required"); - _bTmpRead += bytesRead; - value = 0; - return false; - } - else - { - Debug.Assert(_bTmpRead + bytesRead == 8, "TryReadByteArray returned true without reading all data required"); - _bTmpRead = 0; - AssertValidState(); - value = BitConverter.ToInt64(_bTmp, 0); - return true; - } - } - else - { - // The entire long is in the packet and in the buffer, so just return it - // and take care of the counters. - - value = BitConverter.ToInt64(_inBuff, _inBytesUsed); - - _inBytesUsed += 8; - _inBytesPacket -= 8; - - AssertValidState(); - return true; - } - } - - internal bool TryReadUInt16(out ushort value) - { - Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - - byte[] buffer; - int offset; - if (((_inBytesUsed + 2) > _inBytesRead) || (_inBytesPacket < 2)) - { - // If the uint16 isn't fully in the buffer, or if it isn't fully in the packet, - // then use ReadByteArray since the logic is there to take care of that. - - if (!TryReadByteArray(_bTmp, 2)) - { - value = default; - return false; - } - - buffer = _bTmp; - offset = 0; - } - else - { - // The entire uint16 is in the packet and in the buffer, so just return it - // and take care of the counters. - - buffer = _inBuff; - offset = _inBytesUsed; - - _inBytesUsed += 2; - _inBytesPacket -= 2; - } - - AssertValidState(); - value = (ushort)((buffer[offset + 1] << 8) + buffer[offset]); - return true; - } - - // This method is safe to call when doing async without replay - internal bool TryReadUInt32(out uint value) - { - TdsParser.ReliabilitySection.Assert("unreliable call to ReadUInt32"); // you need to setup for a thread abort somewhere before you call this method - if ((_inBytesPacket == 0) || (_inBytesUsed == _inBytesRead)) - { - if (!TryPrepareBuffer()) - { - value = 0; - return false; - } - } - - if ((_bTmpRead > 0) || (((_inBytesUsed + 4) > _inBytesRead) || (_inBytesPacket < 4))) - { - // If the int isn't fully in the buffer, or if it isn't fully in the packet, - // then use ReadByteArray since the logic is there to take care of that. - - int bytesRead; - if (!TryReadByteArray(_bTmp.AsSpan(start: _bTmpRead), 4 - _bTmpRead, out bytesRead)) - { - Debug.Assert(_bTmpRead + bytesRead <= 4, "Read more data than required"); - _bTmpRead += bytesRead; - value = 0; - return false; - } - else - { - Debug.Assert(_bTmpRead + bytesRead == 4, "TryReadByteArray returned true without reading all data required"); - _bTmpRead = 0; - AssertValidState(); - value = BitConverter.ToUInt32(_bTmp, 0); - return true; - } - } - else - { - // The entire int is in the packet and in the buffer, so just return it - // and take care of the counters. - - value = BitConverter.ToUInt32(_inBuff, _inBytesUsed); - - _inBytesUsed += 4; - _inBytesPacket -= 4; - - AssertValidState(); - return true; - } - } - - internal bool TryReadSingle(out float value) - { - TdsParser.ReliabilitySection.Assert("unreliable call to ReadSingle"); // you need to setup for a thread abort somewhere before you call this method - Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - if (((_inBytesUsed + 4) > _inBytesRead) || (_inBytesPacket < 4)) - { - // If the float isn't fully in the buffer, or if it isn't fully in the packet, - // then use ReadByteArray since the logic is there to take care of that. - - if (!TryReadByteArray(_bTmp, 4)) - { - value = default; - return false; - } - - AssertValidState(); - value = BitConverter.ToSingle(_bTmp, 0); - return true; - } - else - { - // The entire float is in the packet and in the buffer, so just return it - // and take care of the counters. - - value = BitConverter.ToSingle(_inBuff, _inBytesUsed); - - _inBytesUsed += 4; - _inBytesPacket -= 4; - - AssertValidState(); - return true; - } - } - - internal bool TryReadDouble(out double value) - { - TdsParser.ReliabilitySection.Assert("unreliable call to ReadDouble"); // you need to setup for a thread abort somewhere before you call this method - Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - if (((_inBytesUsed + 8) > _inBytesRead) || (_inBytesPacket < 8)) - { - // If the double isn't fully in the buffer, or if it isn't fully in the packet, - // then use ReadByteArray since the logic is there to take care of that. - - if (!TryReadByteArray(_bTmp, 8)) - { - value = default; - return false; - } - - AssertValidState(); - value = BitConverter.ToDouble(_bTmp, 0); - return true; - } - else - { - // The entire double is in the packet and in the buffer, so just return it - // and take care of the counters. - - value = BitConverter.ToDouble(_inBuff, _inBytesUsed); - - _inBytesUsed += 8; - _inBytesPacket -= 8; - - AssertValidState(); - return true; - } - } - - internal bool TryReadString(int length, out string value) - { - TdsParser.ReliabilitySection.Assert("unreliable call to ReadString"); // you need to setup for a thread abort somewhere before you call this method - Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - int cBytes = length << 1; - byte[] buf; - int offset = 0; - - if (((_inBytesUsed + cBytes) > _inBytesRead) || (_inBytesPacket < cBytes)) - { - if (_bTmp == null || _bTmp.Length < cBytes) - { - _bTmp = new byte[cBytes]; - } - - if (!TryReadByteArray(_bTmp, cBytes)) - { - value = null; - return false; - } - - // assign local to point to parser scratch buffer - buf = _bTmp; - - AssertValidState(); - } - else - { - // assign local to point to _inBuff - buf = _inBuff; - offset = _inBytesUsed; - _inBytesUsed += cBytes; - _inBytesPacket -= cBytes; - - AssertValidState(); - } - - value = System.Text.Encoding.Unicode.GetString(buf, offset, cBytes); - return true; - } - - internal bool TryReadStringWithEncoding(int length, System.Text.Encoding encoding, bool isPlp, out string value) - { - TdsParser.ReliabilitySection.Assert("unreliable call to ReadStringWithEncoding"); // you need to setup for a thread abort somewhere before you call this method - Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - - if (null == encoding) - { - // Bug 462435:CR: TdsParser.DrainData(stateObj) hitting timeout exception after Connection Resiliency change - // http://vstfdevdiv:8080/web/wi.aspx?pcguid=22f9acc9-569a-41ff-b6ac-fac1b6370209&id=462435 - // Need to skip the current column before throwing the error - this ensures that the state shared between this and the data reader is consistent when calling DrainData - if (isPlp) - { - if (!_parser.TrySkipPlpValue((ulong)length, this, out _)) - { - value = null; - return false; - } - } - else - { - if (!TrySkipBytes(length)) - { - value = null; - return false; - } - } - - _parser.ThrowUnsupportedCollationEncountered(this); - } - byte[] buf = null; - int offset = 0; - - if (isPlp) - { - if (!TryReadPlpBytes(ref buf, 0, int.MaxValue, out length)) - { - value = null; - return false; - } - - AssertValidState(); - } - else - { - if (((_inBytesUsed + length) > _inBytesRead) || (_inBytesPacket < length)) - { - if (_bTmp == null || _bTmp.Length < length) - { - _bTmp = new byte[length]; - } - - if (!TryReadByteArray(_bTmp, length)) - { - value = null; - return false; - } - - // assign local to point to parser scratch buffer - buf = _bTmp; - - AssertValidState(); - } - else - { - // assign local to point to _inBuff - buf = _inBuff; - offset = _inBytesUsed; - _inBytesUsed += length; - _inBytesPacket -= length; - - AssertValidState(); - } - } - - // BCL optimizes to not use char[] underneath - value = encoding.GetString(buf, offset, length); - return true; - } - - internal ulong ReadPlpLength(bool returnPlpNullIfNull) - { - ulong value; - Debug.Assert(_syncOverAsync, "Should not attempt pends in a synchronous call"); - bool result = TryReadPlpLength(returnPlpNullIfNull, out value); - if (!result) - { throw SQL.SynchronousCallMayNotPend(); } - return value; - } - - // Reads the length of either the entire data or the length of the next chunk in a - // partially length prefixed data - // After this call, call ReadPlpBytes/ReadPlpUnicodeChars untill the specified length of data - // is consumed. Repeat this until ReadPlpLength returns 0 in order to read the - // entire stream. - // When this function returns 0, it means the data stream is read completely and the - // plp state in the tdsparser is cleaned. - internal bool TryReadPlpLength(bool returnPlpNullIfNull, out ulong lengthLeft) - { - uint chunklen; - // bool firstchunk = false; - bool isNull = false; - - Debug.Assert(_longlenleft == 0, "Out of synch length read request"); - if (_longlen == 0) - { - // First chunk is being read. Find out what type of chunk it is - long value; - if (!TryReadInt64(out value)) - { - lengthLeft = 0; - return false; - } - _longlen = (ulong)value; - // firstchunk = true; - } - - if (_longlen == TdsEnums.SQL_PLP_NULL) - { - _longlen = 0; - _longlenleft = 0; - isNull = true; - } - else - { - // Data is coming in uint chunks, read length of next chunk - if (!TryReadUInt32(out chunklen)) - { - lengthLeft = 0; - return false; - } - if (chunklen == TdsEnums.SQL_PLP_CHUNK_TERMINATOR) - { - _longlenleft = 0; - _longlen = 0; - } - else - { - _longlenleft = chunklen; - } - } - - AssertValidState(); - - if (isNull && returnPlpNullIfNull) - { - lengthLeft = TdsEnums.SQL_PLP_NULL; - return true; - } - - lengthLeft = _longlenleft; - return true; - } - - internal int ReadPlpBytesChunk(byte[] buff, int offset, int len) - { - Debug.Assert(_syncOverAsync, "Should not attempt pends in a synchronous call"); - Debug.Assert(_longlenleft > 0, "Read when no data available"); - - int value; - int bytesToRead = (int)Math.Min(_longlenleft, (ulong)len); - bool result = TryReadByteArray(buff.AsSpan(start: offset), bytesToRead, out value); - _longlenleft -= (ulong)bytesToRead; - if (!result) - { - throw SQL.SynchronousCallMayNotPend(); - } - return value; - } // Reads the requested number of bytes from a plp data stream, or the entire data if // requested length is -1 or larger than the actual length of data. First call to this method diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 86f51fba2a..59f919e597 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -1019,6 +1019,662 @@ internal bool SetPacketSize(int size) return false; } + + /////////////////////////////////////// + // Buffer read methods - data values // + /////////////////////////////////////// + + // look at the next byte without pulling it off the wire, don't just return _inBytesUsed since we may + // have to go to the network to get the next byte. + internal bool TryPeekByte(out byte value) + { + if (!TryReadByte(out value)) + { + return false; + } + + // now do fixup + _inBytesPacket++; + _inBytesUsed--; + + AssertValidState(); + return true; + } + + // Takes a byte array, an offset, and a len and fills the array from the offset to len number of + // bytes from the in buffer. + public bool TryReadByteArray(Span buff, int len) + { + return TryReadByteArray(buff, len, out _); + } + + // NOTE: This method must be retriable WITHOUT replaying a snapshot + // Every time you call this method increment the offset and decrease len by the value of totalRead + public bool TryReadByteArray(Span buff, int len, out int totalRead) + { +#if NETFRAMEWORK + TdsParser.ReliabilitySection.Assert("unreliable call to ReadByteArray"); // you need to setup for a thread abort somewhere before you call this method +#endif + + totalRead = 0; + +#if DEBUG + if (_snapshot != null && _snapshot.DoPend()) + { + _networkPacketTaskSource = new TaskCompletionSource(); + Interlocked.MemoryBarrier(); + + if (s_forcePendingReadsToWaitForUser) + { + _realNetworkPacketTaskSource = new TaskCompletionSource(); + _realNetworkPacketTaskSource.SetResult(null); + } + else + { + _networkPacketTaskSource.TrySetResult(null); + } + return false; + } +#endif + + Debug.Assert(buff == null || buff.Length >= len, "Invalid length sent to ReadByteArray()!"); + + // loop through and read up to array length + while (len > 0) + { + if ((_inBytesPacket == 0) || (_inBytesUsed == _inBytesRead)) + { + if (!TryPrepareBuffer()) + { + return false; + } + } + + int bytesToRead = Math.Min(len, Math.Min(_inBytesPacket, _inBytesRead - _inBytesUsed)); + Debug.Assert(bytesToRead > 0, "0 byte read in TryReadByteArray"); + if (!buff.IsEmpty) + { + ReadOnlySpan copyFrom = new ReadOnlySpan(_inBuff, _inBytesUsed, bytesToRead); + Span copyTo = buff.Slice(totalRead, bytesToRead); + copyFrom.CopyTo(copyTo); + } + + totalRead += bytesToRead; + _inBytesUsed += bytesToRead; + _inBytesPacket -= bytesToRead; + len -= bytesToRead; + + AssertValidState(); + } + + return true; + } + + // Takes no arguments and returns a byte from the buffer. If the buffer is empty, it is filled + // before the byte is returned. + internal bool TryReadByte(out byte value) + { +#if NETFRAMEWORK + TdsParser.ReliabilitySection.Assert("unreliable call to ReadByte"); // you need to setup for a thread abort somewhere before you call this method +#endif + + Debug.Assert(_inBytesUsed >= 0 && _inBytesUsed <= _inBytesRead, "ERROR - TDSParser: _inBytesUsed < 0 or _inBytesUsed > _inBytesRead"); + value = 0; + +#if DEBUG + if (_snapshot != null && _snapshot.DoPend()) + { + _networkPacketTaskSource = new TaskCompletionSource(); + Interlocked.MemoryBarrier(); + + if (s_forcePendingReadsToWaitForUser) + { + _realNetworkPacketTaskSource = new TaskCompletionSource(); + _realNetworkPacketTaskSource.SetResult(null); + } + else + { + _networkPacketTaskSource.TrySetResult(null); + } + return false; + } +#endif + + if ((_inBytesPacket == 0) || (_inBytesUsed == _inBytesRead)) + { + if (!TryPrepareBuffer()) + { + return false; + } + } + + // decrement the number of bytes left in the packet + _inBytesPacket--; + + Debug.Assert(_inBytesPacket >= 0, "ERROR - TDSParser: _inBytesPacket < 0"); + + // return the byte from the buffer and increment the counter for number of bytes used in the in buffer + value = (_inBuff[_inBytesUsed++]); + + AssertValidState(); + return true; + } + + internal bool TryReadChar(out char value) + { + Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); + + Span buffer = stackalloc byte[2]; + if (((_inBytesUsed + 2) > _inBytesRead) || (_inBytesPacket < 2)) + { + // If the char isn't fully in the buffer, or if it isn't fully in the packet, + // then use ReadByteArray since the logic is there to take care of that. + if (!TryReadByteArray(buffer, 2)) + { + value = '\0'; + return false; + } + } + else + { + // The entire char is in the packet and in the buffer, so just return it + // and take care of the counters. + buffer = _inBuff.AsSpan(_inBytesUsed, 2); + _inBytesUsed += 2; + _inBytesPacket -= 2; + } + + AssertValidState(); + value = (char)((buffer[1] << 8) + buffer[0]); + return true; + } + + internal bool TryReadInt16(out short value) + { + Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); + + Span buffer = stackalloc byte[2]; + if (((_inBytesUsed + 2) > _inBytesRead) || (_inBytesPacket < 2)) + { + // If the int16 isn't fully in the buffer, or if it isn't fully in the packet, + // then use ReadByteArray since the logic is there to take care of that. + if (!TryReadByteArray(buffer, 2)) + { + value = default; + return false; + } + } + else + { + // The entire int16 is in the packet and in the buffer, so just return it + // and take care of the counters. + buffer = _inBuff.AsSpan(_inBytesUsed, 2); + _inBytesUsed += 2; + _inBytesPacket -= 2; + } + + AssertValidState(); + value = (short)((buffer[1] << 8) + buffer[0]); + return true; + } + + internal bool TryReadInt32(out int value) + { +#if NETFRAMEWORK + TdsParser.ReliabilitySection.Assert("unreliable call to ReadInt32"); // you need to setup for a thread abort somewhere before you call this method +#endif + + Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); + Span buffer = stackalloc byte[4]; + if (((_inBytesUsed + 4) > _inBytesRead) || (_inBytesPacket < 4)) + { + // If the int isn't fully in the buffer, or if it isn't fully in the packet, + // then use ReadByteArray since the logic is there to take care of that. + if (!TryReadByteArray(buffer, 4)) + { + value = 0; + return false; + } + } + else + { + // The entire int is in the packet and in the buffer, so just return it + // and take care of the counters. + buffer = _inBuff.AsSpan(_inBytesUsed, 4); + _inBytesUsed += 4; + _inBytesPacket -= 4; + } + + 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 + internal bool TryReadInt64(out long value) + { +#if NETFRAMEWORK + TdsParser.ReliabilitySection.Assert("unreliable call to ReadInt64"); // you need to setup for a thread abort somewhere before you call this method +#endif + + if ((_inBytesPacket == 0) || (_inBytesUsed == _inBytesRead)) + { + if (!TryPrepareBuffer()) + { + value = 0; + return false; + } + } + + if ((_bTmpRead > 0) || (((_inBytesUsed + 8) > _inBytesRead) || (_inBytesPacket < 8))) + { + // If the long isn't fully in the buffer, or if it isn't fully in the packet, + // then use ReadByteArray since the logic is there to take care of that. + + int bytesRead = 0; + if (!TryReadByteArray(_bTmp.AsSpan(_bTmpRead), 8 - _bTmpRead, out bytesRead)) + { + Debug.Assert(_bTmpRead + bytesRead <= 8, "Read more data than required"); + _bTmpRead += bytesRead; + value = 0; + return false; + } + else + { + Debug.Assert(_bTmpRead + bytesRead == 8, "TryReadByteArray returned true without reading all data required"); + _bTmpRead = 0; + AssertValidState(); + value = BitConverter.ToInt64(_bTmp, 0); + return true; + } + } + else + { + // The entire long is in the packet and in the buffer, so just return it + // and take care of the counters. + + value = BitConverter.ToInt64(_inBuff, _inBytesUsed); + + _inBytesUsed += 8; + _inBytesPacket -= 8; + + AssertValidState(); + return true; + } + } + + internal bool TryReadUInt16(out ushort value) + { + Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); + + Span buffer = stackalloc byte[2]; + if (((_inBytesUsed + 2) > _inBytesRead) || (_inBytesPacket < 2)) + { + // If the uint16 isn't fully in the buffer, or if it isn't fully in the packet, + // then use ReadByteArray since the logic is there to take care of that. + if (!TryReadByteArray(buffer, 2)) + { + value = default; + return false; + } + } + else + { + // The entire uint16 is in the packet and in the buffer, so just return it + // and take care of the counters. + buffer = _inBuff.AsSpan(_inBytesUsed, 2); + _inBytesUsed += 2; + _inBytesPacket -= 2; + } + + AssertValidState(); + value = (ushort)((buffer[1] << 8) + buffer[0]); + return true; + } + + // This method is safe to call when doing async without replay + internal bool TryReadUInt32(out uint value) + { +#if NETFRAMEWORK + TdsParser.ReliabilitySection.Assert("unreliable call to ReadUInt32"); // you need to setup for a thread abort somewhere before you call this method +#endif + + if ((_inBytesPacket == 0) || (_inBytesUsed == _inBytesRead)) + { + if (!TryPrepareBuffer()) + { + value = 0; + return false; + } + } + + if ((_bTmpRead > 0) || (((_inBytesUsed + 4) > _inBytesRead) || (_inBytesPacket < 4))) + { + // If the int isn't fully in the buffer, or if it isn't fully in the packet, + // then use ReadByteArray since the logic is there to take care of that. + + int bytesRead = 0; + if (!TryReadByteArray(_bTmp.AsSpan(_bTmpRead), 4 - _bTmpRead, out bytesRead)) + { + Debug.Assert(_bTmpRead + bytesRead <= 4, "Read more data than required"); + _bTmpRead += bytesRead; + value = 0; + return false; + } + else + { + Debug.Assert(_bTmpRead + bytesRead == 4, "TryReadByteArray returned true without reading all data required"); + _bTmpRead = 0; + AssertValidState(); + value = BitConverter.ToUInt32(_bTmp, 0); + return true; + } + } + else + { + // The entire int is in the packet and in the buffer, so just return it + // and take care of the counters. + + value = BitConverter.ToUInt32(_inBuff, _inBytesUsed); + + _inBytesUsed += 4; + _inBytesPacket -= 4; + + AssertValidState(); + return true; + } + } + + internal bool TryReadSingle(out float value) + { +#if NETFRAMEWORK + TdsParser.ReliabilitySection.Assert("unreliable call to ReadSingle"); // you need to setup for a thread abort somewhere before you call this method +#endif + + Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); + if (((_inBytesUsed + 4) > _inBytesRead) || (_inBytesPacket < 4)) + { + // If the float isn't fully in the buffer, or if it isn't fully in the packet, + // then use ReadByteArray since the logic is there to take care of that. + + if (!TryReadByteArray(_bTmp, 4)) + { + value = default; + return false; + } + + AssertValidState(); + value = BitConverter.ToSingle(_bTmp, 0); + return true; + } + else + { + // The entire float is in the packet and in the buffer, so just return it + // and take care of the counters. + + value = BitConverter.ToSingle(_inBuff, _inBytesUsed); + + _inBytesUsed += 4; + _inBytesPacket -= 4; + + AssertValidState(); + return true; + } + } + + internal bool TryReadDouble(out double value) + { +#if NETFRAMEWORK + TdsParser.ReliabilitySection.Assert("unreliable call to ReadDouble"); // you need to setup for a thread abort somewhere before you call this method +#endif + + Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); + if (((_inBytesUsed + 8) > _inBytesRead) || (_inBytesPacket < 8)) + { + // If the double isn't fully in the buffer, or if it isn't fully in the packet, + // then use ReadByteArray since the logic is there to take care of that. + + if (!TryReadByteArray(_bTmp, 8)) + { + value = default; + return false; + } + + AssertValidState(); + value = BitConverter.ToDouble(_bTmp, 0); + return true; + } + else + { + // The entire double is in the packet and in the buffer, so just return it + // and take care of the counters. + + value = BitConverter.ToDouble(_inBuff, _inBytesUsed); + + _inBytesUsed += 8; + _inBytesPacket -= 8; + + AssertValidState(); + return true; + } + } + + internal bool TryReadString(int length, out string value) + { +#if NETFRAMEWORK + TdsParser.ReliabilitySection.Assert("unreliable call to ReadString"); // you need to setup for a thread abort somewhere before you call this method +#endif + + Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); + int cBytes = length << 1; + byte[] buf; + int offset = 0; + + if (((_inBytesUsed + cBytes) > _inBytesRead) || (_inBytesPacket < cBytes)) + { + if (_bTmp == null || _bTmp.Length < cBytes) + { + _bTmp = new byte[cBytes]; + } + + if (!TryReadByteArray(_bTmp, cBytes)) + { + value = null; + return false; + } + + // assign local to point to parser scratch buffer + buf = _bTmp; + + AssertValidState(); + } + else + { + // assign local to point to _inBuff + buf = _inBuff; + offset = _inBytesUsed; + _inBytesUsed += cBytes; + _inBytesPacket -= cBytes; + + AssertValidState(); + } + + value = System.Text.Encoding.Unicode.GetString(buf, offset, cBytes); + return true; + } + internal bool TryReadStringWithEncoding(int length, System.Text.Encoding encoding, bool isPlp, out string value) + { +#if NETFRAMEWORK + TdsParser.ReliabilitySection.Assert("unreliable call to ReadStringWithEncoding"); // you need to setup for a thread abort somewhere before you call this method +#endif + + Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); + + if (null == encoding) + { + // Need to skip the current column before throwing the error - this ensures that the state shared between this and the data reader is consistent when calling DrainData + if (isPlp) + { + if (!_parser.TrySkipPlpValue((ulong)length, this, out _)) + { + value = null; + return false; + } + } + else + { + if (!TrySkipBytes(length)) + { + value = null; + return false; + } + } + + _parser.ThrowUnsupportedCollationEncountered(this); + } + byte[] buf = null; + int offset = 0; + + if (isPlp) + { + if (!TryReadPlpBytes(ref buf, 0, int.MaxValue, out length)) + { + value = null; + return false; + } + + AssertValidState(); + } + else + { + if (((_inBytesUsed + length) > _inBytesRead) || (_inBytesPacket < length)) + { + if (_bTmp == null || _bTmp.Length < length) + { + _bTmp = new byte[length]; + } + + if (!TryReadByteArray(_bTmp, length)) + { + value = null; + return false; + } + + // assign local to point to parser scratch buffer + buf = _bTmp; + + AssertValidState(); + } + else + { + // assign local to point to _inBuff + buf = _inBuff; + offset = _inBytesUsed; + _inBytesUsed += length; + _inBytesPacket -= length; + + AssertValidState(); + } + } + + // BCL optimizes to not use char[] underneath + value = encoding.GetString(buf, offset, length); + return true; + } + + internal ulong ReadPlpLength(bool returnPlpNullIfNull) + { + ulong value; + Debug.Assert(_syncOverAsync, "Should not attempt pends in a synchronous call"); + bool result = TryReadPlpLength(returnPlpNullIfNull, out value); + if (!result) + { + throw SQL.SynchronousCallMayNotPend(); + } + return value; + } + + // Reads the length of either the entire data or the length of the next chunk in a + // partially length prefixed data + // After this call, call ReadPlpBytes/ReadPlpUnicodeChars until the specified length of data + // is consumed. Repeat this until ReadPlpLength returns 0 in order to read the + // entire stream. + // When this function returns 0, it means the data stream is read completely and the + // plp state in the tdsparser is cleaned. + internal bool TryReadPlpLength(bool returnPlpNullIfNull, out ulong lengthLeft) + { + uint chunklen; + // bool firstchunk = false; + bool isNull = false; + + Debug.Assert(_longlenleft == 0, "Out of synch length read request"); + if (_longlen == 0) + { + // First chunk is being read. Find out what type of chunk it is + long value; + if (!TryReadInt64(out value)) + { + lengthLeft = 0; + return false; + } + _longlen = (ulong)value; + // firstchunk = true; + } + + if (_longlen == TdsEnums.SQL_PLP_NULL) + { + _longlen = 0; + _longlenleft = 0; + isNull = true; + } + else + { + // Data is coming in uint chunks, read length of next chunk + if (!TryReadUInt32(out chunklen)) + { + lengthLeft = 0; + return false; + } + if (chunklen == TdsEnums.SQL_PLP_CHUNK_TERMINATOR) + { + _longlenleft = 0; + _longlen = 0; + } + else + { + _longlenleft = (ulong)chunklen; + } + } + + AssertValidState(); + + if (isNull && returnPlpNullIfNull) + { + lengthLeft = TdsEnums.SQL_PLP_NULL; + return true; + } + + lengthLeft = _longlenleft; + return true; + } + + internal int ReadPlpBytesChunk(byte[] buff, int offset, int len) + { + Debug.Assert(_syncOverAsync, "Should not attempt pends in a synchronous call"); + Debug.Assert(_longlenleft > 0, "Read when no data available"); + + int value; + int bytesToRead = (int)Math.Min(_longlenleft, (ulong)len); + bool result = TryReadByteArray(buff.AsSpan(offset), bytesToRead, out value); + _longlenleft -= (ulong)bytesToRead; + if (!result) + { + throw SQL.SynchronousCallMayNotPend(); + } + return value; + } + /* // leave this in. comes handy if you have to do Console.WriteLine style debugging ;) From 5429584b884d0947284f115c9c3839fbe2627f41 Mon Sep 17 00:00:00 2001 From: panoskj Date: Sat, 19 Mar 2022 13:12:01 +0200 Subject: [PATCH 02/22] Merging of TdsParserStateObject.TryReadPlpBytes method (note: ports parts of #285 to netfx). --- .../Data/SqlClient/TdsParserStateObject.cs | 128 ----------------- .../Data/SqlClient/TdsParserStateObject.cs | 105 -------------- .../Data/SqlClient/TdsParserStateObject.cs | 129 ++++++++++++++++++ 3 files changed, 129 insertions(+), 233 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 16947160f1..96dc16e46c 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -329,134 +329,6 @@ internal bool HasReceivedColumnMetadata set => SetSnapshottedState(SnapshottedStateFlags.ColMetaDataReceived, value); } - /////////////////////////////////////// - // Buffer read methods - data values // - /////////////////////////////////////// - - // Reads the requested number of bytes from a plp data stream, or the entire data if - // requested length is -1 or larger than the actual length of data. First call to this method - // should be preceeded by a call to ReadPlpLength or ReadDataLength. - // Returns the actual bytes read. - // NOTE: This method must be retriable WITHOUT replaying a snapshot - // Every time you call this method increment the offset and decrease len by the value of totalBytesRead - internal bool TryReadPlpBytes(ref byte[] buff, int offset, int len, out int totalBytesRead) - { - int bytesRead; - int bytesLeft; - byte[] newbuf; - - if (_longlen == 0) - { - Debug.Assert(_longlenleft == 0); - if (buff == null) - { - buff = Array.Empty(); - } - - AssertValidState(); - totalBytesRead = 0; - return true; // No data - } - - Debug.Assert(_longlen != TdsEnums.SQL_PLP_NULL, "Out of sync plp read request"); - Debug.Assert((buff == null && offset == 0) || (buff.Length >= offset + len), "Invalid length sent to ReadPlpBytes()!"); - - bytesLeft = len; - - // If total length is known up front, allocate the whole buffer in one shot instead of realloc'ing and copying over each time - if (buff == null && _longlen != TdsEnums.SQL_PLP_UNKNOWNLEN) - { - if (_snapshot != null) - { - // if there is a snapshot and it contains a stored plp buffer take it - // and try to use it if it is the right length - buff = _snapshot._plpBuffer; - _snapshot._plpBuffer = null; - } - - if ((ulong)(buff?.Length ?? 0) != _longlen) - { - // if the buffer is null or the wrong length create one to use - buff = new byte[(Math.Min((int)_longlen, len))]; - } - } - - if (_longlenleft == 0) - { - if (!TryReadPlpLength(false, out _)) - { - totalBytesRead = 0; - return false; - } - if (_longlenleft == 0) - { // Data read complete - totalBytesRead = 0; - return true; - } - } - - if (buff == null) - { - buff = new byte[_longlenleft]; - } - - totalBytesRead = 0; - - while (bytesLeft > 0) - { - int bytesToRead = (int)Math.Min(_longlenleft, (ulong)bytesLeft); - if (buff.Length < (offset + bytesToRead)) - { - // Grow the array - newbuf = new byte[offset + bytesToRead]; - Buffer.BlockCopy(buff, 0, newbuf, 0, offset); - buff = newbuf; - } - - bool result = TryReadByteArray(buff.AsSpan(offset), bytesToRead, out bytesRead); - Debug.Assert(bytesRead <= bytesLeft, "Read more bytes than we needed"); - Debug.Assert((ulong)bytesRead <= _longlenleft, "Read more bytes than is available"); - - bytesLeft -= bytesRead; - offset += bytesRead; - totalBytesRead += bytesRead; - _longlenleft -= (ulong)bytesRead; - if (!result) - { - if (_snapshot != null) - { - // a partial read has happened so store the target buffer in the snapshot - // so it can be re-used when another packet arrives and we read again - _snapshot._plpBuffer = buff; - } - return false; - } - - if (_longlenleft == 0) - { - // Read the next chunk or cleanup state if hit the end - if (!TryReadPlpLength(false, out _)) - { - if (_snapshot != null) - { - // a partial read has happened so store the target buffer in the snapshot - // so it can be re-used when another packet arrives and we read again - _snapshot._plpBuffer = buff; - } - return false; - } - } - - AssertValidState(); - - // Catch the point where we read the entire plp data stream and clean up state - if (_longlenleft == 0) // Data read complete - break; - } - return true; - } - - ///////////////////////////////////////// // Value Skip Logic // ///////////////////////////////////////// diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index dabad6d98e..c7226b69e4 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -415,111 +415,6 @@ internal void StartSession(int objectID) _allowObjectID = objectID; } - /////////////////////////////////////// - // Buffer read methods - data values // - /////////////////////////////////////// - - - // Reads the requested number of bytes from a plp data stream, or the entire data if - // requested length is -1 or larger than the actual length of data. First call to this method - // should be preceeded by a call to ReadPlpLength or ReadDataLength. - // Returns the actual bytes read. - // NOTE: This method must be retriable WITHOUT replaying a snapshot - // Every time you call this method increment the offst and decrease len by the value of totalBytesRead - internal bool TryReadPlpBytes(ref byte[] buff, int offst, int len, out int totalBytesRead) - { - int bytesRead; - int bytesLeft; - byte[] newbuf; - - if (_longlen == 0) - { - Debug.Assert(_longlenleft == 0); - if (buff == null) - { - buff = new byte[0]; - } - - AssertValidState(); - totalBytesRead = 0; - return true; // No data - } - - Debug.Assert((_longlen != TdsEnums.SQL_PLP_NULL), - "Out of sync plp read request"); - - Debug.Assert((buff == null && offst == 0) || (buff.Length >= offst + len), "Invalid length sent to ReadPlpBytes()!"); - bytesLeft = len; - - // If total length is known up front, allocate the whole buffer in one shot instead of realloc'ing and copying over each time - if (buff == null && _longlen != TdsEnums.SQL_PLP_UNKNOWNLEN) - { - buff = new byte[(Math.Min((int)_longlen, len))]; - } - - if (_longlenleft == 0) - { - if (!TryReadPlpLength(false, out _)) - { - totalBytesRead = 0; - return false; - } - if (_longlenleft == 0) - { // Data read complete - totalBytesRead = 0; - return true; - } - } - - if (buff == null) - { - buff = new byte[_longlenleft]; - } - - totalBytesRead = 0; - - while (bytesLeft > 0) - { - int bytesToRead = (int)Math.Min(_longlenleft, (ulong)bytesLeft); - if (buff.Length < (offst + bytesToRead)) - { - // Grow the array - newbuf = new byte[offst + bytesToRead]; - Buffer.BlockCopy(buff, 0, newbuf, 0, offst); - buff = newbuf; - } - - bool result = TryReadByteArray(buff.AsSpan(start: offst), bytesToRead, out bytesRead); - Debug.Assert(bytesRead <= bytesLeft, "Read more bytes than we needed"); - Debug.Assert((ulong)bytesRead <= _longlenleft, "Read more bytes than is available"); - - bytesLeft -= bytesRead; - offst += bytesRead; - totalBytesRead += bytesRead; - _longlenleft -= (ulong)bytesRead; - if (!result) - { - return false; - } - - if (_longlenleft == 0) - { // Read the next chunk or cleanup state if hit the end - if (!TryReadPlpLength(false, out _)) - { - return false; - } - } - - AssertValidState(); - - // Catch the point where we read the entire plp data stream and clean up state - if (_longlenleft == 0) // Data read complete - break; - } - return true; - } - - ///////////////////////////////////////// // Value Skip Logic // ///////////////////////////////////////// diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 59f919e597..69c1bfa211 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -1675,6 +1675,135 @@ internal int ReadPlpBytesChunk(byte[] buff, int offset, int len) return value; } + // Reads the requested number of bytes from a plp data stream, or the entire data if + // requested length is -1 or larger than the actual length of data. First call to this method + // should be preceeded by a call to ReadPlpLength or ReadDataLength. + // Returns the actual bytes read. + // NOTE: This method must be retriable WITHOUT replaying a snapshot + // Every time you call this method increment the offset and decrease len by the value of totalBytesRead + internal bool TryReadPlpBytes(ref byte[] buff, int offset, int len, out int totalBytesRead) + { + int bytesRead; + int bytesLeft; + byte[] newbuf; + + if (_longlen == 0) + { + Debug.Assert(_longlenleft == 0); + if (buff == null) + { + buff = Array.Empty(); + } + + AssertValidState(); + totalBytesRead = 0; + return true; // No data + } + + Debug.Assert(_longlen != TdsEnums.SQL_PLP_NULL, "Out of sync plp read request"); + Debug.Assert((buff == null && offset == 0) || (buff.Length >= offset + len), "Invalid length sent to ReadPlpBytes()!"); + + bytesLeft = len; + + // If total length is known up front, allocate the whole buffer in one shot instead of realloc'ing and copying over each time + if (buff == null && _longlen != TdsEnums.SQL_PLP_UNKNOWNLEN) + { +#if !NETFRAMEWORK // This condition should be removed when StateSnapshot has been merged. + if (_snapshot != null) + { + // if there is a snapshot and it contains a stored plp buffer take it + // and try to use it if it is the right length + buff = _snapshot._plpBuffer; + _snapshot._plpBuffer = null; + } + + if ((ulong)(buff?.Length ?? 0) != _longlen) +#endif + { + // if the buffer is null or the wrong length create one to use + buff = new byte[(int)Math.Min((int)_longlen, len)]; + } + } + + if (_longlenleft == 0) + { + if (!TryReadPlpLength(false, out _)) + { + totalBytesRead = 0; + return false; + } + if (_longlenleft == 0) + { // Data read complete + totalBytesRead = 0; + return true; + } + } + + if (buff == null) + { + buff = new byte[_longlenleft]; + } + + totalBytesRead = 0; + + while (bytesLeft > 0) + { + int bytesToRead = (int)Math.Min(_longlenleft, (ulong)bytesLeft); + if (buff.Length < (offset + bytesToRead)) + { + // Grow the array + newbuf = new byte[offset + bytesToRead]; + Buffer.BlockCopy(buff, 0, newbuf, 0, offset); + buff = newbuf; + } + + bool result = TryReadByteArray(buff.AsSpan(offset), bytesToRead, out bytesRead); + Debug.Assert(bytesRead <= bytesLeft, "Read more bytes than we needed"); + Debug.Assert((ulong)bytesRead <= _longlenleft, "Read more bytes than is available"); + + bytesLeft -= bytesRead; + offset += bytesRead; + totalBytesRead += bytesRead; + _longlenleft -= (ulong)bytesRead; + if (!result) + { +#if !NETFRAMEWORK // This condition should be removed when StateSnapshot has been merged. + if (_snapshot != null) + { + // a partial read has happened so store the target buffer in the snapshot + // so it can be re-used when another packet arrives and we read again + _snapshot._plpBuffer = buff; + } +#endif + return false; + } + + if (_longlenleft == 0) + { + // Read the next chunk or cleanup state if hit the end + if (!TryReadPlpLength(false, out _)) + { +#if !NETFRAMEWORK // This condition should be removed when StateSnapshot has been merged. + if (_snapshot != null) + { + // a partial read has happened so store the target buffer in the snapshot + // so it can be re-used when another packet arrives and we read again + _snapshot._plpBuffer = buff; + } +#endif + return false; + } + } + + AssertValidState(); + + // Catch the point where we read the entire plp data stream and clean up state + if (_longlenleft == 0) // Data read complete + break; + } + return true; + } + /* // leave this in. comes handy if you have to do Console.WriteLine style debugging ;) From d1127018796ddb9e6820955daf083e3182028d14 Mon Sep 17 00:00:00 2001 From: panoskj Date: Sat, 19 Mar 2022 13:20:17 +0200 Subject: [PATCH 03/22] Merging "Value Skip Logic" methods of TdsParserStateObject. --- .../Data/SqlClient/TdsParserStateObject.cs | 32 ------------------- .../Data/SqlClient/TdsParserStateObject.cs | 32 ------------------- .../Data/SqlClient/TdsParserStateObject.cs | 32 +++++++++++++++++++ 3 files changed, 32 insertions(+), 64 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 96dc16e46c..7581244e29 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -329,38 +329,6 @@ internal bool HasReceivedColumnMetadata set => SetSnapshottedState(SnapshottedStateFlags.ColMetaDataReceived, value); } - ///////////////////////////////////////// - // 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.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.Empty, num); - } - ///////////////////////////////////////// // Network/Packet Reading & Processing // ///////////////////////////////////////// diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index c7226b69e4..877052cafb 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -415,38 +415,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.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.Empty, num); - } - ///////////////////////////////////////// // Network/Packet Reading & Processing // ///////////////////////////////////////// diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 69c1bfa211..259697537f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -1804,6 +1804,38 @@ internal bool TryReadPlpBytes(ref byte[] buff, int offset, int len, out int tota return true; } + ///////////////////////////////////////// + // 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"); + int cbSkip = 0; + + while (num > 0) + { + cbSkip = (int)Math.Min((long)int.MaxValue, num); + if (!TryReadByteArray(Span.Empty, cbSkip)) + { + return false; + } + num -= (long)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.Empty, num); + } + /* // leave this in. comes handy if you have to do Console.WriteLine style debugging ;) From f115d59d64ed5e7004e0b5a3add27e0ad077404e Mon Sep 17 00:00:00 2001 From: panoskj Date: Sat, 19 Mar 2022 13:50:54 +0200 Subject: [PATCH 04/22] Merging some "Network/Packet Reading & Processing" methods of TdsParserStateObject. --- .../Data/SqlClient/TdsParserStateObject.cs | 68 ---------------- .../Data/SqlClient/TdsParserStateObject.cs | 75 +----------------- .../Data/SqlClient/TdsParserStateObject.cs | 79 +++++++++++++++++++ 3 files changed, 81 insertions(+), 141 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 7581244e29..4541a31299 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -361,74 +361,6 @@ internal void ResetSnapshot() _snapshotReplay = false; } -#if DEBUG - private string _lastStack; -#endif - - internal bool TryReadNetworkPacket() - { -#if DEBUG - Debug.Assert(!_shouldHaveEnoughData || _attentionSent, "Caller said there should be enough data, but we are currently reading a packet"); -#endif - - if (_snapshot != null) - { - if (_snapshotReplay) - { - if (_snapshot.Replay()) - { -#if DEBUG - if (s_checkNetworkPacketRetryStacks) - { - _snapshot.CheckStack(Environment.StackTrace); - } -#endif - return true; - } -#if DEBUG - else - { - if (s_checkNetworkPacketRetryStacks) - { - _lastStack = Environment.StackTrace; - } - } -#endif - } - - // previous buffer is in snapshot - _inBuff = new byte[_inBuff.Length]; - } - - if (_syncOverAsync) - { - ReadSniSyncOverAsync(); - return true; - } - - ReadSni(new TaskCompletionSource()); - -#if DEBUG - if (s_failAsyncPends) - { - throw new InvalidOperationException("Attempted to pend a read when _failAsyncPends test hook was enabled"); - } - if (s_forceSyncOverAsyncAfterFirstPend) - { - _syncOverAsync = true; - } -#endif - Debug.Assert((_snapshot != null) ^ _asyncReadWithoutSnapshot, "Must have either _snapshot set up or _asyncReadWithoutSnapshot enabled (but not both) to pend a read"); - - return false; - } - - internal void PrepareReplaySnapshot() - { - _networkPacketTaskSource = null; - _snapshot.PrepareReplay(); - } - internal void ReadSniSyncOverAsync() { if (_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 877052cafb..8152aad4be 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -432,77 +432,6 @@ internal void ResetSnapshot() _snapshotReplay = false; } -#if DEBUG - StackTrace _lastStack; -#endif - - 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 - - if (_snapshot != null) - { - if (_snapshotReplay) - { - if (_snapshot.Replay()) - { -#if DEBUG - if (s_checkNetworkPacketRetryStacks) - { - _snapshot.CheckStack(new StackTrace()); - } -#endif - SqlClientEventSource.Log.TryTraceEvent(" Async packet replay{0}", "INFO"); - return true; - } -#if DEBUG - else - { - if (s_checkNetworkPacketRetryStacks) - { - _lastStack = new StackTrace(); - } - } -#endif - } - - // previous buffer is in snapshot - _inBuff = new byte[_inBuff.Length]; - } - - if (_syncOverAsync) - { - ReadSniSyncOverAsync(); - return true; - } - - ReadSni(new TaskCompletionSource()); - -#if DEBUG - if (s_failAsyncPends) - { - throw new InvalidOperationException("Attempted to pend a read when _failAsyncPends test hook was enabled"); - } - if (s_forceSyncOverAsyncAfterFirstPend) - { - _syncOverAsync = true; - } -#endif - Debug.Assert((_snapshot != null) ^ _asyncReadWithoutSnapshot, "Must have either _snapshot set up or _asyncReadWithoutSnapshot enabled (but not both) to pend a read"); - - return false; - } - - internal void PrepareReplaySnapshot() - { - _networkPacketTaskSource = null; - _snapshot.PrepareReplay(); - } - internal void ReadSniSyncOverAsync() { if (_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed) @@ -2472,7 +2401,7 @@ class PacketData public byte[] Buffer; public int Read; #if DEBUG - public StackTrace Stack; + public string Stack; #endif } @@ -2561,7 +2490,7 @@ internal void AssertCurrent() { Debug.Assert(_snapshotInBuffCurrent == _snapshotInBuffs.Count, "Should not be reading new packets when not replaying last packet"); } - internal void CheckStack(StackTrace trace) + internal void CheckStack(string trace) { PacketData prev = _snapshotInBuffs[_snapshotInBuffCurrent - 1]; if (prev.Stack == null) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 259697537f..bae4a59efb 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -1836,6 +1836,85 @@ internal bool TrySkipBytes(int num) return TryReadByteArray(Span.Empty, num); } + ///////////////////////////////////////// + // Network/Packet Reading & Processing // + ///////////////////////////////////////// + /// +#if DEBUG + private string _lastStack; +#endif + + internal bool TryReadNetworkPacket() + { +#if NETFRAMEWORK + TdsParser.ReliabilitySection.Assert("unreliable call to TryReadNetworkPacket"); // you need to setup for a thread abort somewhere before you call this method +#endif + +#if DEBUG + Debug.Assert(!_shouldHaveEnoughData || _attentionSent, "Caller said there should be enough data, but we are currently reading a packet"); +#endif + + if (_snapshot != null) + { + if (_snapshotReplay) + { + if (_snapshot.Replay()) + { +#if DEBUG + if (s_checkNetworkPacketRetryStacks) + { + _snapshot.CheckStack(Environment.StackTrace); + } +#endif +#if NETFRAMEWORK + SqlClientEventSource.Log.TryTraceEvent(" Async packet replay{0}", "INFO"); +#endif + return true; + } +#if DEBUG + else + { + if (s_checkNetworkPacketRetryStacks) + { + _lastStack = Environment.StackTrace; + } + } +#endif + } + + // previous buffer is in snapshot + _inBuff = new byte[_inBuff.Length]; + } + + if (_syncOverAsync) + { + ReadSniSyncOverAsync(); + return true; + } + + ReadSni(new TaskCompletionSource()); + +#if DEBUG + if (s_failAsyncPends) + { + throw new InvalidOperationException("Attempted to pend a read when _failAsyncPends test hook was enabled"); + } + if (s_forceSyncOverAsyncAfterFirstPend) + { + _syncOverAsync = true; + } +#endif + Debug.Assert((_snapshot != null) ^ _asyncReadWithoutSnapshot, "Must have either _snapshot set up or _asyncReadWithoutSnapshot enabled (but not both) to pend a read"); + + return false; + } + + internal void PrepareReplaySnapshot() + { + _networkPacketTaskSource = null; + _snapshot.PrepareReplay(); + } + /* // leave this in. comes handy if you have to do Console.WriteLine style debugging ;) From 7c3dfe21c7504f0f8b5f2f91fd3644f693528520 Mon Sep 17 00:00:00 2001 From: panoskj Date: Sat, 19 Mar 2022 14:35:58 +0200 Subject: [PATCH 05/22] Merging of TdsParserStateObject.ReadSniSyncOverAsync method. --- .../Data/SqlClient/TdsParserStateObject.cs | 67 ---------------- .../Data/SqlClient/TdsParserStateObject.cs | 72 +++-------------- .../Data/SqlClient/TdsParserStateObject.cs | 77 +++++++++++++++++++ 3 files changed, 86 insertions(+), 130 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 4541a31299..5be3051493 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -361,73 +361,6 @@ internal void ResetSnapshot() _snapshotReplay = false; } - internal void ReadSniSyncOverAsync() - { - if (_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed) - { - throw ADP.ClosedConnectionError(); - } - - PacketHandle readPacket = default; - - uint error; - - bool shouldDecrement = false; - try - { - Interlocked.Increment(ref _readingCount); - shouldDecrement = true; - - readPacket = ReadSyncOverAsync(GetTimeoutRemaining(), out error); - - Interlocked.Decrement(ref _readingCount); - shouldDecrement = false; - - if (_parser.MARSOn) - { // Only take reset lock on MARS and Async. - CheckSetResetConnectionState(error, CallbackType.Read); - } - - if (TdsEnums.SNI_SUCCESS == error) - { // Success - process results! - - Debug.Assert(!IsPacketEmpty(readPacket), "ReadNetworkPacket cannot be null in synchronous operation!"); - - ProcessSniPacket(readPacket, 0); -#if DEBUG - if (s_forcePendingReadsToWaitForUser) - { - _networkPacketTaskSource = new TaskCompletionSource(); - Interlocked.MemoryBarrier(); - _networkPacketTaskSource.Task.Wait(); - _networkPacketTaskSource = null; - } -#endif - } - else - { // Failure! - - Debug.Assert(!IsValidPacket(readPacket), "unexpected readPacket without corresponding SNIPacketRelease"); - - ReadSniError(this, error); - } - } - finally - { - if (shouldDecrement) - { - Interlocked.Decrement(ref _readingCount); - } - - if (!IsPacketEmpty(readPacket)) - { - ReleasePacket(readPacket); - } - - AssertValidState(); - } - } - internal void OnConnectionClosed() { // the stateObj is not null, so the async invocation that registered this callback diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 8152aad4be..7829734994 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -432,77 +432,23 @@ internal void ResetSnapshot() _snapshotReplay = false; } - internal void ReadSniSyncOverAsync() + private IntPtr ReadSyncOverAsync(int timeout, out uint error) { - if (_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed) + SNIHandle handle = Handle; + if (handle == null) { throw ADP.ClosedConnectionError(); } - IntPtr readPacket = IntPtr.Zero; - 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; - - SNIHandle handle = Handle; - if (handle == null) - { - throw ADP.ClosedConnectionError(); - } - - error = SNINativeMethodWrapper.SNIReadSyncOverAsync(handle, ref readPacket, GetTimeoutRemaining()); - - Interlocked.Decrement(ref _readingCount); - shouldDecrement = false; - - if (_parser.MARSOn) - { // Only take reset lock on MARS and Async. - CheckSetResetConnectionState(error, CallbackType.Read); - } + error = SNINativeMethodWrapper.SNIReadSyncOverAsync(handle, ref readPacket, timeout); + return readPacket; + } - if (TdsEnums.SNI_SUCCESS == error) - { // Success - process results! - Debug.Assert(ADP.s_ptrZero != readPacket, "ReadNetworkPacket cannot be null in synchronous operation!"); - ProcessSniPacket(readPacket, 0); -#if DEBUG - if (s_forcePendingReadsToWaitForUser) - { - _networkPacketTaskSource = new TaskCompletionSource(); - Thread.MemoryBarrier(); - _networkPacketTaskSource.Task.Wait(); - _networkPacketTaskSource = null; - } -#endif - } - else - { // Failure! - Debug.Assert(IntPtr.Zero == readPacket, "unexpected readPacket without corresponding SNIPacketRelease"); - ReadSniError(this, error); - } - } - finally - { - if (shouldDecrement) - { - Interlocked.Decrement(ref _readingCount); - } + private bool IsPacketEmpty(IntPtr packet) => packet == IntPtr.Zero; - if (readPacket != IntPtr.Zero) - { - // Be sure to release packet, otherwise it will be leaked by native. - SNINativeMethodWrapper.SNIPacketRelease(readPacket); - } + private bool IsValidPacket(IntPtr packet) => packet != IntPtr.Zero; - AssertValidState(); - } - } + private void ReleasePacket(IntPtr packet) => SNINativeMethodWrapper.SNIPacketRelease(packet); internal void OnConnectionClosed() { diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index bae4a59efb..c1aed6b494 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -13,6 +13,10 @@ namespace Microsoft.Data.SqlClient { +#if NETFRAMEWORK + using PacketHandle = IntPtr; +#endif + sealed internal class LastIOTimer { internal long _value; @@ -1915,6 +1919,79 @@ internal void PrepareReplaySnapshot() _snapshot.PrepareReplay(); } + internal void ReadSniSyncOverAsync() + { + if (_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed) + { + throw ADP.ClosedConnectionError(); + } + + PacketHandle readPacket = default; + + uint error; + +#if NETFRAMEWORK + System.Runtime.CompilerServices.RuntimeHelpers.PrepareConstrainedRegions(); +#endif + bool shouldDecrement = false; + try + { +#if NETFRAMEWORK + TdsParser.ReliabilitySection.Assert("unreliable call to ReadSniSync"); // you need to setup for a thread abort somewhere before you call this method +#endif + Interlocked.Increment(ref _readingCount); + shouldDecrement = true; + + readPacket = ReadSyncOverAsync(GetTimeoutRemaining(), out error); + + Interlocked.Decrement(ref _readingCount); + shouldDecrement = false; + + if (_parser.MARSOn) + { // Only take reset lock on MARS and Async. + CheckSetResetConnectionState(error, CallbackType.Read); + } + + if (TdsEnums.SNI_SUCCESS == error) + { // Success - process results! + + Debug.Assert(!IsPacketEmpty(readPacket), "ReadNetworkPacket cannot be null in synchronous operation!"); + + ProcessSniPacket(readPacket, 0); +#if DEBUG + if (s_forcePendingReadsToWaitForUser) + { + _networkPacketTaskSource = new TaskCompletionSource(); + Interlocked.MemoryBarrier(); + _networkPacketTaskSource.Task.Wait(); + _networkPacketTaskSource = null; + } +#endif + } + else + { // Failure! + + Debug.Assert(!IsValidPacket(readPacket), "unexpected readPacket without corresponding SNIPacketRelease"); + + ReadSniError(this, error); + } + } + finally + { + if (shouldDecrement) + { + Interlocked.Decrement(ref _readingCount); + } + + if (!IsPacketEmpty(readPacket)) + { + ReleasePacket(readPacket); + } + + AssertValidState(); + } + } + /* // leave this in. comes handy if you have to do Console.WriteLine style debugging ;) From 6ae132189402383b637fd33aea03346521a40883 Mon Sep 17 00:00:00 2001 From: panoskj Date: Sat, 19 Mar 2022 14:46:19 +0200 Subject: [PATCH 06/22] Merging some "Network/Packet Reading & Processing" methods of TdsParserStateObject. --- .../Data/SqlClient/TdsParserStateObject.cs | 191 ----------------- .../Data/SqlClient/TdsParserStateObject.cs | 194 ------------------ .../Data/SqlClient/TdsParserStateObject.cs | 194 ++++++++++++++++++ 3 files changed, 194 insertions(+), 385 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 5be3051493..d9fb710a47 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -361,197 +361,6 @@ internal void ResetSnapshot() _snapshotReplay = false; } - internal void OnConnectionClosed() - { - // the stateObj is not null, so the async invocation that registered this callback - // via the SqlReferenceCollection has not yet completed. We will look for a - // _networkPacketTaskSource and mark it faulted. If we don't find it, then - // TdsParserStateObject.ReadSni will abort when it does look to see if the parser - // is open. If we do, then when the call that created it completes and a continuation - // is registered, we will ensure the completion is called. - - // Note, this effort is necessary because when the app domain is being unloaded, - // we don't get callback from SNI. - - // first mark parser broken. This is to ensure that ReadSni will abort if it has - // not yet executed. - Parser.State = TdsParserState.Broken; - Parser.Connection.BreakConnection(); - - // Ensure that changing state occurs before checking _networkPacketTaskSource - Interlocked.MemoryBarrier(); - - // then check for networkPacketTaskSource - TaskCompletionSource taskSource = _networkPacketTaskSource; - if (taskSource != null) - { - taskSource.TrySetException(ADP.ExceptionWithStackTrace(ADP.ClosedConnectionError())); - } - - taskSource = _writeCompletionSource; - if (taskSource != null) - { - taskSource.TrySetException(ADP.ExceptionWithStackTrace(ADP.ClosedConnectionError())); - } - } - - public void SetTimeoutStateStopped() - { - Interlocked.Exchange(ref _timeoutState, TimeoutState.Stopped); - _timeoutIdentityValue = 0; - } - - public bool IsTimeoutStateExpired - { - get - { - int state = _timeoutState; - return state == TimeoutState.ExpiredAsync || state == TimeoutState.ExpiredSync; - } - } - - private void OnTimeoutAsync(object state) - { - if (_enforceTimeoutDelay) - { - Thread.Sleep(_enforcedTimeoutDelayInMilliSeconds); - } - - int currentIdentityValue = _timeoutIdentityValue; - TimeoutState timeoutState = (TimeoutState)state; - if (timeoutState.IdentityValue == _timeoutIdentityValue) - { - // the return value is not useful here because no choice is going to be made using it - // we only want to make this call to set the state knowing that it will be seen later - OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredAsync); - } - else - { - Debug.WriteLine($"OnTimeoutAsync called with identity state={timeoutState.IdentityValue} but current identity is {currentIdentityValue} so it is being ignored"); - } - } - - private bool OnTimeoutSync(bool asyncClose = false) - { - return OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredSync, asyncClose); - } - - /// - /// attempts to change the timout state from the expected state to the target state and if it succeeds - /// will setup the the stateobject into the timeout expired state - /// - /// the state that is the expected current state, state will change only if this is correct - /// the state that will be changed to if the expected state is correct - /// any close action to be taken by an async task to avoid deadlock. - /// boolean value indicating whether the call changed the timeout state - private bool OnTimeoutCore(int expectedState, int targetState, bool asyncClose = false) - { - Debug.Assert(targetState == TimeoutState.ExpiredAsync || targetState == TimeoutState.ExpiredSync, "OnTimeoutCore must have an expiry state as the targetState"); - - bool retval = false; - if (Interlocked.CompareExchange(ref _timeoutState, targetState, expectedState) == expectedState) - { - retval = true; - // lock protects against Close and Cancel - lock (this) - { - if (!_attentionSent) - { - AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, 0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); - - // Grab a reference to the _networkPacketTaskSource in case it becomes null while we are trying to use it - TaskCompletionSource source = _networkPacketTaskSource; - - if (_parser.Connection.IsInPool) - { - // We should never timeout if the connection is currently in the pool: the safest thing to do here is to doom the connection to avoid corruption - Debug.Assert(_parser.Connection.IsConnectionDoomed, "Timeout occurred while the connection is in the pool"); - _parser.State = TdsParserState.Broken; - _parser.Connection.BreakConnection(); - if (source != null) - { - source.TrySetCanceled(); - } - } - else if (_parser.State == TdsParserState.OpenLoggedIn) - { - try - { - SendAttention(mustTakeWriteLock: true, asyncClose); - } - catch (Exception e) - { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - // if unable to send attention, cancel the _networkPacketTaskSource to - // request the parser be broken. SNIWritePacket errors will already - // be in the _errors collection. - if (source != null) - { - source.TrySetCanceled(); - } - } - } - - // If we still haven't received a packet then we don't want to actually close the connection - // from another thread, so complete the pending operation as cancelled, informing them to break it - if (source != null) - { - Task.Delay(AttentionTimeoutSeconds * 1000).ContinueWith(_ => - { - // Only break the connection if the read didn't finish - if (!source.Task.IsCompleted) - { - int pendingCallback = IncrementPendingCallbacks(); - try - { - // If pendingCallback is at 3, then ReadAsyncCallback hasn't been called yet - // So it is safe for us to break the connection and cancel the Task (since we are not sure that ReadAsyncCallback will ever be called) - if ((pendingCallback == 3) && (!source.Task.IsCompleted)) - { - Debug.Assert(source == _networkPacketTaskSource, "_networkPacketTaskSource which is being timed is not the current task source"); - - // Try to throw the timeout exception and store it in the task - bool exceptionStored = false; - try - { - CheckThrowSNIException(); - } - catch (Exception ex) - { - if (source.TrySetException(ex)) - { - exceptionStored = true; - } - } - - // Ensure that the connection is no longer usable - // This is needed since the timeout error added above is non-fatal (and so throwing it won't break the connection) - _parser.State = TdsParserState.Broken; - _parser.Connection.BreakConnection(); - - // If we didn't get an exception (something else observed it?) then ensure that the task is cancelled - if (!exceptionStored) - { - source.TrySetCanceled(); - } - } - } - finally - { - DecrementPendingCallbacks(release: false); - } - } - }); - } - } - } - } - return retval; - } - internal void ReadSni(TaskCompletionSource completion) { Debug.Assert(_networkPacketTaskSource == null || ((_asyncReadWithoutSnapshot) && (_networkPacketTaskSource.Task.IsCompleted)), "Pending async call or failed to replay snapshot when calling ReadSni"); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 7829734994..a15001e153 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -450,200 +450,6 @@ private IntPtr ReadSyncOverAsync(int timeout, out uint error) private void ReleasePacket(IntPtr packet) => SNINativeMethodWrapper.SNIPacketRelease(packet); - internal void OnConnectionClosed() - { - // the stateObj is not null, so the async invocation that registered this callback - // via the SqlReferenceCollection has not yet completed. We will look for a - // _networkPacketTaskSource and mark it faulted. If we don't find it, then - // TdsParserStateObject.ReadSni will abort when it does look to see if the parser - // is open. If we do, then when the call that created it completes and a continuation - // is registered, we will ensure the completion is called. - - // Note, this effort is necessary because when the app domain is being unloaded, - // we don't get callback from SNI. - - // first mark parser broken. This is to ensure that ReadSni will abort if it has - // not yet executed. - Parser.State = TdsParserState.Broken; - Parser.Connection.BreakConnection(); - - // Ensure that changing state occurs before checking _networkPacketTaskSource - Thread.MemoryBarrier(); - - // then check for networkPacketTaskSource - TaskCompletionSource taskSource = _networkPacketTaskSource; - if (taskSource != null) - { - taskSource.TrySetException(ADP.ExceptionWithStackTrace(ADP.ClosedConnectionError())); - } - - taskSource = _writeCompletionSource; - if (taskSource != null) - { - taskSource.TrySetException(ADP.ExceptionWithStackTrace(ADP.ClosedConnectionError())); - } - - } - - public void SetTimeoutStateStopped() - { - Interlocked.Exchange(ref _timeoutState, TimeoutState.Stopped); - _timeoutIdentityValue = 0; - } - - public bool IsTimeoutStateExpired - { - get - { - int state = _timeoutState; - return state == TimeoutState.ExpiredAsync || state == TimeoutState.ExpiredSync; - } - } - - private void OnTimeoutAsync(object state) - { - if (_enforceTimeoutDelay) - { - Thread.Sleep(_enforcedTimeoutDelayInMilliSeconds); - } - - int currentIdentityValue = _timeoutIdentityValue; - TimeoutState timeoutState = (TimeoutState)state; - if (timeoutState.IdentityValue == _timeoutIdentityValue) - { - // the return value is not useful here because no choice is going to be made using it - // we only want to make this call to set the state knowing that it will be seen later - OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredAsync); - } - else - { - Debug.WriteLine($"OnTimeoutAsync called with identity state={timeoutState.IdentityValue} but current identity is {currentIdentityValue} so it is being ignored"); - } - } - - private bool OnTimeoutSync(bool asyncClose = false) - { - return OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredSync, asyncClose); - } - - /// - /// attempts to change the timout state from the expected state to the target state and if it succeeds - /// will setup the the stateobject into the timeout expired state - /// - /// the state that is the expected current state, state will change only if this is correct - /// the state that will be changed to if the expected state is correct - /// any close action to be taken by an async task to avoid deadlock. - /// boolean value indicating whether the call changed the timeout state - private bool OnTimeoutCore(int expectedState, int targetState, bool asyncClose = false) - { - Debug.Assert(targetState == TimeoutState.ExpiredAsync || targetState == TimeoutState.ExpiredSync, "OnTimeoutCore must have an expiry state as the targetState"); - - bool retval = false; - if (Interlocked.CompareExchange(ref _timeoutState, targetState, expectedState) == expectedState) - { - retval = true; - // lock protects against Close and Cancel - lock (this) - { - if (!_attentionSent) - { - AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, 0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); - - // Grab a reference to the _networkPacketTaskSource in case it becomes null while we are trying to use it - TaskCompletionSource source = _networkPacketTaskSource; - - if (_parser.Connection.IsInPool) - { - // Dev11 Bug 390048 : Timing issue between OnTimeout and ReadAsyncCallback results in SqlClient's packet parsing going out of sync - // We should never timeout if the connection is currently in the pool: the safest thing to do here is to doom the connection to avoid corruption - Debug.Assert(_parser.Connection.IsConnectionDoomed, "Timeout occurred while the connection is in the pool"); - _parser.State = TdsParserState.Broken; - _parser.Connection.BreakConnection(); - if (source != null) - { - source.TrySetCanceled(); - } - } - else if (_parser.State == TdsParserState.OpenLoggedIn) - { - try - { - SendAttention(mustTakeWriteLock: true, asyncClose); - } - catch (Exception e) - { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - // if unable to send attention, cancel the _networkPacketTaskSource to - // request the parser be broken. SNIWritePacket errors will already - // be in the _errors collection. - if (source != null) - { - source.TrySetCanceled(); - } - } - } - - // If we still haven't received a packet then we don't want to actually close the connection - // from another thread, so complete the pending operation as cancelled, informing them to break it - if (source != null) - { - Task.Delay(AttentionTimeoutSeconds * 1000).ContinueWith(_ => - { - // Only break the connection if the read didn't finish - if (!source.Task.IsCompleted) - { - int pendingCallback = IncrementPendingCallbacks(); - RuntimeHelpers.PrepareConstrainedRegions(); - try - { - // If pendingCallback is at 3, then ReadAsyncCallback hasn't been called yet - // So it is safe for us to break the connection and cancel the Task (since we are not sure that ReadAsyncCallback will ever be called) - if ((pendingCallback == 3) && (!source.Task.IsCompleted)) - { - Debug.Assert(source == _networkPacketTaskSource, "_networkPacketTaskSource which is being timed is not the current task source"); - - // Try to throw the timeout exception and store it in the task - bool exceptionStored = false; - try - { - CheckThrowSNIException(); - } - catch (Exception ex) - { - if (source.TrySetException(ex)) - { - exceptionStored = true; - } - } - - // Ensure that the connection is no longer usable - // This is needed since the timeout error added above is non-fatal (and so throwing it won't break the connection) - _parser.State = TdsParserState.Broken; - _parser.Connection.BreakConnection(); - - // If we didn't get an exception (something else observed it?) then ensure that the task is cancelled - if (!exceptionStored) - { - source.TrySetCanceled(); - } - } - } - finally - { - DecrementPendingCallbacks(release: false); - } - } - }); - } - } - } - } - return retval; - } - internal void ReadSni(TaskCompletionSource completion) { Debug.Assert(_networkPacketTaskSource == null || ((_asyncReadWithoutSnapshot) && (_networkPacketTaskSource.Task.IsCompleted)), "Pending async call or failed to replay snapshot when calling ReadSni"); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index c1aed6b494..8152b7582d 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -1992,6 +1992,200 @@ internal void ReadSniSyncOverAsync() } } + internal void OnConnectionClosed() + { + // the stateObj is not null, so the async invocation that registered this callback + // via the SqlReferenceCollection has not yet completed. We will look for a + // _networkPacketTaskSource and mark it faulted. If we don't find it, then + // TdsParserStateObject.ReadSni will abort when it does look to see if the parser + // is open. If we do, then when the call that created it completes and a continuation + // is registered, we will ensure the completion is called. + + // Note, this effort is necessary because when the app domain is being unloaded, + // we don't get callback from SNI. + + // first mark parser broken. This is to ensure that ReadSni will abort if it has + // not yet executed. + Parser.State = TdsParserState.Broken; + Parser.Connection.BreakConnection(); + + // Ensure that changing state occurs before checking _networkPacketTaskSource + Interlocked.MemoryBarrier(); + + // then check for networkPacketTaskSource + TaskCompletionSource taskSource = _networkPacketTaskSource; + if (taskSource != null) + { + taskSource.TrySetException(ADP.ExceptionWithStackTrace(ADP.ClosedConnectionError())); + } + + taskSource = _writeCompletionSource; + if (taskSource != null) + { + taskSource.TrySetException(ADP.ExceptionWithStackTrace(ADP.ClosedConnectionError())); + } + } + + public void SetTimeoutStateStopped() + { + Interlocked.Exchange(ref _timeoutState, TimeoutState.Stopped); + _timeoutIdentityValue = 0; + } + + public bool IsTimeoutStateExpired + { + get + { + int state = _timeoutState; + return state == TimeoutState.ExpiredAsync || state == TimeoutState.ExpiredSync; + } + } + + private void OnTimeoutAsync(object state) + { + if (_enforceTimeoutDelay) + { + Thread.Sleep(_enforcedTimeoutDelayInMilliSeconds); + } + + int currentIdentityValue = _timeoutIdentityValue; + TimeoutState timeoutState = (TimeoutState)state; + if (timeoutState.IdentityValue == _timeoutIdentityValue) + { + // the return value is not useful here because no choice is going to be made using it + // we only want to make this call to set the state knowing that it will be seen later + OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredAsync); + } + else + { + Debug.WriteLine($"OnTimeoutAsync called with identity state={timeoutState.IdentityValue} but current identity is {currentIdentityValue} so it is being ignored"); + } + } + + private bool OnTimeoutSync(bool asyncClose = false) + { + return OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredSync, asyncClose); + } + + /// + /// attempts to change the timout state from the expected state to the target state and if it succeeds + /// will setup the the stateobject into the timeout expired state + /// + /// the state that is the expected current state, state will change only if this is correct + /// the state that will be changed to if the expected state is correct + /// any close action to be taken by an async task to avoid deadlock. + /// boolean value indicating whether the call changed the timeout state + private bool OnTimeoutCore(int expectedState, int targetState, bool asyncClose = false) + { + Debug.Assert(targetState == TimeoutState.ExpiredAsync || targetState == TimeoutState.ExpiredSync, "OnTimeoutCore must have an expiry state as the targetState"); + + bool retval = false; + if (Interlocked.CompareExchange(ref _timeoutState, targetState, expectedState) == expectedState) + { + retval = true; + // lock protects against Close and Cancel + lock (this) + { + if (!_attentionSent) + { + AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, (byte)0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); + + // Grab a reference to the _networkPacketTaskSource in case it becomes null while we are trying to use it + TaskCompletionSource source = _networkPacketTaskSource; + + if (_parser.Connection.IsInPool) + { + // We should never timeout if the connection is currently in the pool: the safest thing to do here is to doom the connection to avoid corruption + Debug.Assert(_parser.Connection.IsConnectionDoomed, "Timeout occurred while the connection is in the pool"); + _parser.State = TdsParserState.Broken; + _parser.Connection.BreakConnection(); + if (source != null) + { + source.TrySetCanceled(); + } + } + else if (_parser.State == TdsParserState.OpenLoggedIn) + { + try + { + SendAttention(mustTakeWriteLock: true, asyncClose); + } + catch (Exception e) + { + if (!ADP.IsCatchableExceptionType(e)) + { + throw; + } + // if unable to send attention, cancel the _networkPacketTaskSource to + // request the parser be broken. SNIWritePacket errors will already + // be in the _errors collection. + if (source != null) + { + source.TrySetCanceled(); + } + } + } + + // If we still haven't received a packet then we don't want to actually close the connection + // from another thread, so complete the pending operation as cancelled, informing them to break it + if (source != null) + { + Task.Delay(AttentionTimeoutSeconds * 1000).ContinueWith(_ => + { + // Only break the connection if the read didn't finish + if (!source.Task.IsCompleted) + { + int pendingCallback = IncrementPendingCallbacks(); +#if NETFRAMEWORK + System.Runtime.CompilerServices.RuntimeHelpers.PrepareConstrainedRegions(); +#endif + try + { + // If pendingCallback is at 3, then ReadAsyncCallback hasn't been called yet + // So it is safe for us to break the connection and cancel the Task (since we are not sure that ReadAsyncCallback will ever be called) + if ((pendingCallback == 3) && (!source.Task.IsCompleted)) + { + Debug.Assert(source == _networkPacketTaskSource, "_networkPacketTaskSource which is being timed is not the current task source"); + + // Try to throw the timeout exception and store it in the task + bool exceptionStored = false; + try + { + CheckThrowSNIException(); + } + catch (Exception ex) + { + if (source.TrySetException(ex)) + { + exceptionStored = true; + } + } + + // Ensure that the connection is no longer usable + // This is needed since the timeout error added above is non-fatal (and so throwing it won't break the connection) + _parser.State = TdsParserState.Broken; + _parser.Connection.BreakConnection(); + + // If we didn't get an exception (something else observed it?) then ensure that the task is cancelled + if (!exceptionStored) + { + source.TrySetCanceled(); + } + } + } + finally + { + DecrementPendingCallbacks(release: false); + } + } + }); + } + } + } + } + return retval; + } + /* // leave this in. comes handy if you have to do Console.WriteLine style debugging ;) From e5af755aea00d948e0d0e895f9b24e1dc807b434 Mon Sep 17 00:00:00 2001 From: panoskj Date: Sat, 19 Mar 2022 16:21:26 +0200 Subject: [PATCH 07/22] Merging TdsParserStateObject.ReadSni method. --- .../Data/SqlClient/TdsParserStateObject.cs | 140 +--------------- .../Data/SqlClient/TdsParserStateObject.cs | 148 ++--------------- .../src/Microsoft/Data/Common/AdapterUtil.cs | 46 +++--- .../Data/SqlClient/TdsParserStateObject.cs | 149 ++++++++++++++++++ 4 files changed, 191 insertions(+), 292 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index d9fb710a47..f8475b923a 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -17,6 +17,8 @@ namespace Microsoft.Data.SqlClient { internal abstract partial class TdsParserStateObject { + private static bool UseManagedSNI => TdsParserStateObjectFactory.UseManagedSNI; + private static readonly ContextCallback s_readAsyncCallbackComplete = ReadAsyncCallbackComplete; // Timeout variables @@ -361,144 +363,6 @@ internal void ResetSnapshot() _snapshotReplay = false; } - internal void ReadSni(TaskCompletionSource completion) - { - Debug.Assert(_networkPacketTaskSource == null || ((_asyncReadWithoutSnapshot) && (_networkPacketTaskSource.Task.IsCompleted)), "Pending async call or failed to replay snapshot when calling ReadSni"); - _networkPacketTaskSource = completion; - - // Ensure that setting the completion source is completed before checking the state - Interlocked.MemoryBarrier(); - - // We must check after assigning _networkPacketTaskSource to avoid races with - // SqlCommand.OnConnectionClosed - if (_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed) - { - throw ADP.ClosedConnectionError(); - } - -#if DEBUG - if (s_forcePendingReadsToWaitForUser) - { - _realNetworkPacketTaskSource = new TaskCompletionSource(); - } -#endif - - - PacketHandle readPacket = default; - - uint error = 0; - - try - { - Debug.Assert(completion != null, "Async on but null asyncResult passed"); - - // if the state is currently stopped then change it to running and allocate a new identity value from - // the identity source. The identity value is used to correlate timer callback events to the currently - // running timeout and prevents a late timer callback affecting a result it does not relate to - int previousTimeoutState = Interlocked.CompareExchange(ref _timeoutState, TimeoutState.Running, TimeoutState.Stopped); - Debug.Assert(previousTimeoutState == TimeoutState.Stopped, "previous timeout state was not Stopped"); - if (previousTimeoutState == TimeoutState.Stopped) - { - Debug.Assert(_timeoutIdentityValue == 0, "timer was previously stopped without resetting the _identityValue"); - _timeoutIdentityValue = Interlocked.Increment(ref _timeoutIdentitySource); - } - - _networkPacketTimeout?.Dispose(); - - _networkPacketTimeout = ADP.UnsafeCreateTimer( - _onTimeoutAsync, - new TimeoutState(_timeoutIdentityValue), - Timeout.Infinite, - Timeout.Infinite - ); - - - // -1 == Infinite - // 0 == Already timed out (NOTE: To simulate the same behavior as sync we will only timeout on 0 if we receive an IO Pending from SNI) - // >0 == Actual timeout remaining - int msecsRemaining = GetTimeoutRemaining(); - if (msecsRemaining > 0) - { - ChangeNetworkPacketTimeout(msecsRemaining, Timeout.Infinite); - } - - Interlocked.Increment(ref _readingCount); - - SessionHandle handle = SessionHandle; - if (!handle.IsNull) - { - 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); - - if (handle.IsNull) - { - throw ADP.ClosedConnectionError(); - } - - if (TdsEnums.SNI_SUCCESS == error) - { // Success - process results! - Debug.Assert(IsValidPacket(readPacket), "ReadNetworkPacket should not have been null on this async operation!"); - // Evaluate this condition for MANAGED_SNI. This may not be needed because the network call is happening Async and only the callback can receive a success. - ReadAsyncCallback(IntPtr.Zero, readPacket, 0); - - // Only release packet for Managed SNI as for Native SNI packet is released in finally block. - if (TdsParserStateObjectFactory.UseManagedSNI && !IsPacketEmpty(readPacket)) - { - ReleasePacket(readPacket); - } - } - else if (TdsEnums.SNI_SUCCESS_IO_PENDING != error) - { // FAILURE! - Debug.Assert(IsPacketEmpty(readPacket), "unexpected readPacket without corresponding SNIPacketRelease"); - - ReadSniError(this, error); -#if DEBUG - if ((s_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) - { - _realNetworkPacketTaskSource.TrySetResult(null); - } - else -#endif - { - _networkPacketTaskSource.TrySetResult(null); - } - // Disable timeout timer on error - SetTimeoutStateStopped(); - ChangeNetworkPacketTimeout(Timeout.Infinite, Timeout.Infinite); - } - else if (msecsRemaining == 0) - { - // Got IO Pending, but we have no time left to wait - // disable the timer and set the error state by calling OnTimeoutSync - ChangeNetworkPacketTimeout(Timeout.Infinite, Timeout.Infinite); - OnTimeoutSync(); - } - // DO NOT HANDLE PENDING READ HERE - which is TdsEnums.SNI_SUCCESS_IO_PENDING state. - // That is handled by user who initiated async read, or by ReadNetworkPacket which is sync over async. - } - finally - { - if (!TdsParserStateObjectFactory.UseManagedSNI) - { - if (!IsPacketEmpty(readPacket)) - { - // Be sure to release packet, otherwise it will be leaked by native. - ReleasePacket(readPacket); - } - } - AssertValidState(); - } - } - /// /// Checks to see if the underlying connection is still alive (used by connection pool resiliency) /// NOTE: This is not safe to do on a connection that is currently in use diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index a15001e153..472f00115e 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -16,10 +16,23 @@ namespace Microsoft.Data.SqlClient { + internal readonly ref struct SessionHandle + { + public readonly SNIHandle NativeHandle; + + public SessionHandle(SNIHandle nativeHandle) => NativeHandle = nativeHandle; + + public bool IsNull => NativeHandle is null; + } + internal partial class TdsParserStateObject { + private static bool UseManagedSNI => false; + private SNIHandle _sessionHandle = null; // the SNI handle we're to work on + private SessionHandle SessionHandle => new SessionHandle(_sessionHandle); + internal bool _pendingData = false; internal bool _errorTokenReceived = false; // Keep track of whether an error was received for the result. // This is reset upon each done token - there can be @@ -60,6 +73,7 @@ internal TdsParserStateObject(TdsParser parser, SNIHandle physicalConnection, bo // Construct a MARS session Debug.Assert(null != parser, "no parser?"); _parser = parser; + _onTimeoutAsync = OnTimeoutAsync; SniContext = SniContext.Snix_GetMarsSession; Debug.Assert(null != _parser._physicalStateObj, "no physical session?"); @@ -450,139 +464,11 @@ private IntPtr ReadSyncOverAsync(int timeout, out uint error) private void ReleasePacket(IntPtr packet) => SNINativeMethodWrapper.SNIPacketRelease(packet); - internal void ReadSni(TaskCompletionSource completion) + private IntPtr ReadAsync(SessionHandle handle, out uint error) { - Debug.Assert(_networkPacketTaskSource == null || ((_asyncReadWithoutSnapshot) && (_networkPacketTaskSource.Task.IsCompleted)), "Pending async call or failed to replay snapshot when calling ReadSni"); - _networkPacketTaskSource = completion; - - // Ensure that setting the completion source is completed before checking the state - Thread.MemoryBarrier(); - - // We must check after assigning _networkPacketTaskSource to avoid races with - // SqlCommand.OnConnectionClosed - if (_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed) - { - throw ADP.ClosedConnectionError(); - } - -#if DEBUG - if (s_forcePendingReadsToWaitForUser) - { - _realNetworkPacketTaskSource = new TaskCompletionSource(); - } -#endif - IntPtr readPacket = IntPtr.Zero; - uint error = 0; - - RuntimeHelpers.PrepareConstrainedRegions(); - try - { - Debug.Assert(completion != null, "Async on but null asyncResult passed"); - - // if the state is currently stopped then change it to running and allocate a new identity value from - // the identity source. The identity value is used to correlate timer callback events to the currently - // running timeout and prevents a late timer callback affecting a result it does not relate to - int previousTimeoutState = Interlocked.CompareExchange(ref _timeoutState, TimeoutState.Running, TimeoutState.Stopped); - Debug.Assert(previousTimeoutState == TimeoutState.Stopped, "previous timeout state was not Stopped"); - if (previousTimeoutState == TimeoutState.Stopped) - { - Debug.Assert(_timeoutIdentityValue == 0, "timer was previously stopped without resetting the _identityValue"); - _timeoutIdentityValue = Interlocked.Increment(ref _timeoutIdentitySource); - } - - _networkPacketTimeout?.Dispose(); - - _networkPacketTimeout = new Timer( - new TimerCallback(OnTimeoutAsync), - new TimeoutState(_timeoutIdentityValue), - Timeout.Infinite, - Timeout.Infinite - ); - - // -1 == Infinite - // 0 == Already timed out (NOTE: To simulate the same behavior as sync we will only timeout on 0 if we receive an IO Pending from SNI) - // >0 == Actual timeout remaining - int msecsRemaining = GetTimeoutRemaining(); - if (msecsRemaining > 0) - { - ChangeNetworkPacketTimeout(msecsRemaining, Timeout.Infinite); - } - - SNIHandle handle = null; - - RuntimeHelpers.PrepareConstrainedRegions(); - try - { } - finally - { - Interlocked.Increment(ref _readingCount); - - handle = Handle; - if (handle != null) - { - - IncrementPendingCallbacks(); - - error = SNINativeMethodWrapper.SNIReadAsync(handle, ref readPacket); - - if (!(TdsEnums.SNI_SUCCESS == error || TdsEnums.SNI_SUCCESS_IO_PENDING == error)) - { - DecrementPendingCallbacks(false); // Failure - we won't receive callback! - } - } - - Interlocked.Decrement(ref _readingCount); - } - - if (handle == null) - { - throw ADP.ClosedConnectionError(); - } - - if (TdsEnums.SNI_SUCCESS == error) - { // Success - process results! - Debug.Assert(ADP.s_ptrZero != readPacket, "ReadNetworkPacket should not have been null on this async operation!"); - ReadAsyncCallback(ADP.s_ptrZero, readPacket, 0); - } - else if (TdsEnums.SNI_SUCCESS_IO_PENDING != error) - { // FAILURE! - Debug.Assert(IntPtr.Zero == readPacket, "unexpected readPacket without corresponding SNIPacketRelease"); - ReadSniError(this, error); -#if DEBUG - if ((s_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) - { - _realNetworkPacketTaskSource.TrySetResult(null); - } - else -#endif - { - _networkPacketTaskSource.TrySetResult(null); - } - // Disable timeout timer on error - SetTimeoutStateStopped(); - ChangeNetworkPacketTimeout(Timeout.Infinite, Timeout.Infinite); - } - else if (msecsRemaining == 0) - { - // Got IO Pending, but we have no time left to wait - // disable the timer and set the error state by calling OnTimeoutSync - ChangeNetworkPacketTimeout(Timeout.Infinite, Timeout.Infinite); - OnTimeoutSync(); - } - // DO NOT HANDLE PENDING READ HERE - which is TdsEnums.SNI_SUCCESS_IO_PENDING state. - // That is handled by user who initiated async read, or by ReadNetworkPacket which is sync over async. - } - finally - { - if (readPacket != IntPtr.Zero) - { - // Be sure to release packet, otherwise it will be leaked by native. - SNINativeMethodWrapper.SNIPacketRelease(readPacket); - } - - AssertValidState(); - } + error = SNINativeMethodWrapper.SNIReadAsync(handle.NativeHandle, ref readPacket); + return readPacket; } /// diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs index 8f48131f8d..3ca1c4d59d 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/AdapterUtil.cs @@ -443,6 +443,29 @@ internal static Exception CreateSqlException(MsalException msalException, SqlCon return SqlException.CreateException(sqlErs, "", sender); } + internal static Timer UnsafeCreateTimer(TimerCallback callback, object state, int dueTime, int period) + { + // Don't capture the current ExecutionContext and its AsyncLocals onto + // a global timer causing them to live forever + bool restoreFlow = false; + try + { + if (!ExecutionContext.IsFlowSuppressed()) + { + ExecutionContext.SuppressFlow(); + restoreFlow = true; + } + + return new Timer(callback, state, dueTime, period); + } + finally + { + // Restore the current ExecutionContext + if (restoreFlow) + ExecutionContext.RestoreFlow(); + } + } + #endregion #region CommandBuilder, Command, BulkCopy @@ -1498,29 +1521,6 @@ internal static IntPtr IntPtrOffset(IntPtr pbase, int offset) #endregion #else #region netcore project only - internal static Timer UnsafeCreateTimer(TimerCallback callback, object state, int dueTime, int period) - { - // Don't capture the current ExecutionContext and its AsyncLocals onto - // a global timer causing them to live forever - bool restoreFlow = false; - try - { - if (!ExecutionContext.IsFlowSuppressed()) - { - ExecutionContext.SuppressFlow(); - restoreFlow = true; - } - - return new Timer(callback, state, dueTime, period); - } - finally - { - // Restore the current ExecutionContext - if (restoreFlow) - ExecutionContext.RestoreFlow(); - } - } - // // COM+ exceptions // diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 8152b7582d..56a56f5007 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -2186,6 +2186,155 @@ private bool OnTimeoutCore(int expectedState, int targetState, bool asyncClose = return retval; } + internal void ReadSni(TaskCompletionSource completion) + { + Debug.Assert(_networkPacketTaskSource == null || ((_asyncReadWithoutSnapshot) && (_networkPacketTaskSource.Task.IsCompleted)), "Pending async call or failed to replay snapshot when calling ReadSni"); + _networkPacketTaskSource = completion; + + // Ensure that setting the completion source is completed before checking the state + Interlocked.MemoryBarrier(); + + // We must check after assigning _networkPacketTaskSource to avoid races with + // SqlCommand.OnConnectionClosed + if (_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed) + { + throw ADP.ClosedConnectionError(); + } + +#if DEBUG + if (s_forcePendingReadsToWaitForUser) + { + _realNetworkPacketTaskSource = new TaskCompletionSource(); + } +#endif + + + PacketHandle readPacket = default; + + uint error = 0; +#if NETFRAMEWORK + System.Runtime.CompilerServices.RuntimeHelpers.PrepareConstrainedRegions(); +#endif + try + { + Debug.Assert(completion != null, "Async on but null asyncResult passed"); + + // if the state is currently stopped then change it to running and allocate a new identity value from + // the identity source. The identity value is used to correlate timer callback events to the currently + // running timeout and prevents a late timer callback affecting a result it does not relate to + int previousTimeoutState = Interlocked.CompareExchange(ref _timeoutState, TimeoutState.Running, TimeoutState.Stopped); + Debug.Assert(previousTimeoutState == TimeoutState.Stopped, "previous timeout state was not Stopped"); + if (previousTimeoutState == TimeoutState.Stopped) + { + Debug.Assert(_timeoutIdentityValue == 0, "timer was previously stopped without resetting the _identityValue"); + _timeoutIdentityValue = Interlocked.Increment(ref _timeoutIdentitySource); + } + + _networkPacketTimeout?.Dispose(); + + _networkPacketTimeout = ADP.UnsafeCreateTimer( + _onTimeoutAsync, + new TimeoutState(_timeoutIdentityValue), + Timeout.Infinite, + Timeout.Infinite + ); + + + // -1 == Infinite + // 0 == Already timed out (NOTE: To simulate the same behavior as sync we will only timeout on 0 if we receive an IO Pending from SNI) + // >0 == Actual timeout remaining + int msecsRemaining = GetTimeoutRemaining(); + if (msecsRemaining > 0) + { + ChangeNetworkPacketTimeout(msecsRemaining, Timeout.Infinite); + } + + SessionHandle handle = default; +#if NETFRAMEWORK + System.Runtime.CompilerServices.RuntimeHelpers.PrepareConstrainedRegions(); + try + { } + finally +#endif + { + Interlocked.Increment(ref _readingCount); + + handle = SessionHandle; + if (!handle.IsNull) + { + 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); + } + + if (handle.IsNull) + { + throw ADP.ClosedConnectionError(); + } + + if (TdsEnums.SNI_SUCCESS == error) + { // Success - process results! + Debug.Assert(IsValidPacket(readPacket), "ReadNetworkPacket should not have been null on this async operation!"); + // Evaluate this condition for MANAGED_SNI. This may not be needed because the network call is happening Async and only the callback can receive a success. + ReadAsyncCallback(IntPtr.Zero, readPacket, 0); + + // Only release packet for Managed SNI as for Native SNI packet is released in finally block. + if (UseManagedSNI && !IsPacketEmpty(readPacket)) + { + ReleasePacket(readPacket); + } + } + else if (TdsEnums.SNI_SUCCESS_IO_PENDING != error) + { // FAILURE! + Debug.Assert(IsPacketEmpty(readPacket), "unexpected readPacket without corresponding SNIPacketRelease"); + + ReadSniError(this, error); +#if DEBUG + if ((s_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) + { + _realNetworkPacketTaskSource.TrySetResult(null); + } + else +#endif + { + _networkPacketTaskSource.TrySetResult(null); + } + // Disable timeout timer on error + SetTimeoutStateStopped(); + ChangeNetworkPacketTimeout(Timeout.Infinite, Timeout.Infinite); + } + else if (msecsRemaining == 0) + { + // Got IO Pending, but we have no time left to wait + // disable the timer and set the error state by calling OnTimeoutSync + ChangeNetworkPacketTimeout(Timeout.Infinite, Timeout.Infinite); + OnTimeoutSync(); + } + // DO NOT HANDLE PENDING READ HERE - which is TdsEnums.SNI_SUCCESS_IO_PENDING state. + // That is handled by user who initiated async read, or by ReadNetworkPacket which is sync over async. + } + finally + { + if (!UseManagedSNI) + { + if (!IsPacketEmpty(readPacket)) + { + // Be sure to release packet, otherwise it will be leaked by native. + ReleasePacket(readPacket); + } + } + AssertValidState(); + } + } + /* // leave this in. comes handy if you have to do Console.WriteLine style debugging ;) From 49f716977fabbf4eee009ab1df440f7a6c1edee0 Mon Sep 17 00:00:00 2001 From: panoskj Date: Sat, 19 Mar 2022 16:36:41 +0200 Subject: [PATCH 08/22] Merging TdsParserStateObject.IsConnectionAlive method. --- .../Data/SqlClient/TdsParserStateObject.cs | 56 -------------- .../Data/SqlClient/TdsParserStateObject.cs | 75 +----------------- .../Data/SqlClient/TdsParserStateObject.cs | 76 +++++++++++++++++++ 3 files changed, 77 insertions(+), 130 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index f8475b923a..147891976c 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -363,62 +363,6 @@ internal void ResetSnapshot() _snapshotReplay = false; } - /// - /// Checks to see if the underlying connection is still alive (used by connection pool resiliency) - /// NOTE: This is not safe to do on a connection that is currently in use - /// NOTE: This will mark the connection as broken if it is found to be dead - /// - /// If true then an exception will be thrown if the connection is found to be dead, otherwise no exception will be thrown - /// True if the connection is still alive, otherwise false - internal bool IsConnectionAlive(bool throwOnException) - { - Debug.Assert(_parser.Connection == null || _parser.Connection.Pool != null, "Shouldn't be calling IsConnectionAlive on non-pooled connections"); - bool isAlive = true; - - if (DateTime.UtcNow.Ticks - _lastSuccessfulIOTimer._value > CheckConnectionWindow) - { - if ((_parser == null) || ((_parser.State == TdsParserState.Broken) || (_parser.State == TdsParserState.Closed))) - { - isAlive = false; - if (throwOnException) - { - throw SQL.ConnectionDoomed(); - } - } - else if ((_pendingCallbacks > 1) || ((_parser.Connection != null) && (!_parser.Connection.IsInPool))) - { - // This connection is currently in use, assume that the connection is 'alive' - // NOTE: SNICheckConnection is not currently supported for connections that are in use - Debug.Assert(true, "Call to IsConnectionAlive while connection is in use"); - } - else - { - uint error; - SniContext = SniContext.Snix_Connect; - - error = CheckConnection(); - if ((error != TdsEnums.SNI_SUCCESS) && (error != TdsEnums.SNI_WAIT_TIMEOUT)) - { - // 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; - } - } - } - - return isAlive; - } - /// /// Checks to see if the underlying connection is still valid (used by idle connection resiliency - for active connections) /// NOTE: This is not safe to do on a connection that is currently in use diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 472f00115e..d573075412 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -471,80 +471,7 @@ private IntPtr ReadAsync(SessionHandle handle, out uint error) return readPacket; } - /// - /// Checks to see if the underlying connection is still alive (used by connection pool resilency) - /// NOTE: This is not safe to do on a connection that is currently in use - /// NOTE: This will mark the connection as broken if it is found to be dead - /// - /// If true then an exception will be thrown if the connection is found to be dead, otherwise no exception will be thrown - /// True if the connection is still alive, otherwise false - internal bool IsConnectionAlive(bool throwOnException) - { - Debug.Assert(_parser.Connection == null || _parser.Connection.Pool != null, "Shouldn't be calling IsConnectionAlive on non-pooled connections"); - bool isAlive = true; - - if (DateTime.UtcNow.Ticks - _lastSuccessfulIOTimer._value > CheckConnectionWindow) - { - if ((_parser == null) || ((_parser.State == TdsParserState.Broken) || (_parser.State == TdsParserState.Closed))) - { - isAlive = false; - if (throwOnException) - { - throw SQL.ConnectionDoomed(); - } - } - else if ((_pendingCallbacks > 1) || ((_parser.Connection != null) && (!_parser.Connection.IsInPool))) - { - // This connection is currently in use, assume that the connection is 'alive' - // NOTE: SNICheckConnection is not currently supported for connections that are in use - Debug.Assert(true, "Call to IsConnectionAlive while connection is in use"); - } - else - { - uint error; - IntPtr readPacket = IntPtr.Zero; - - RuntimeHelpers.PrepareConstrainedRegions(); - try - { - 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 = SNINativeMethodWrapper.SNICheckConnection(Handle); - - if ((error != TdsEnums.SNI_SUCCESS) && (error != TdsEnums.SNI_WAIT_TIMEOUT)) - { - // Connection is dead - SqlClientEventSource.Log.TryTraceEvent(" received error {0} on idle connection", (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; - } - } - finally - { - if (readPacket != IntPtr.Zero) - { - // Be sure to release packet, otherwise it will be leaked by native. - SNINativeMethodWrapper.SNIPacketRelease(readPacket); - } - - } - } - } - - return isAlive; - } + private uint CheckConnection() => SNINativeMethodWrapper.SNICheckConnection(Handle); /// /// Checks to see if the underlying connection is still valid (used by idle connection resiliency - for active connections) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 56a56f5007..c42d143ba1 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -2335,6 +2335,82 @@ internal void ReadSni(TaskCompletionSource completion) } } + /// + /// Checks to see if the underlying connection is still alive (used by connection pool resiliency) + /// NOTE: This is not safe to do on a connection that is currently in use + /// NOTE: This will mark the connection as broken if it is found to be dead + /// + /// If true then an exception will be thrown if the connection is found to be dead, otherwise no exception will be thrown + /// True if the connection is still alive, otherwise false + internal bool IsConnectionAlive(bool throwOnException) + { + Debug.Assert(_parser.Connection == null || _parser.Connection.Pool != null, "Shouldn't be calling IsConnectionAlive on non-pooled connections"); + bool isAlive = true; + + if (DateTime.UtcNow.Ticks - _lastSuccessfulIOTimer._value > CheckConnectionWindow) + { + if ((_parser == null) || ((_parser.State == TdsParserState.Broken) || (_parser.State == TdsParserState.Closed))) + { + isAlive = false; + if (throwOnException) + { + throw SQL.ConnectionDoomed(); + } + } + else if ((_pendingCallbacks > 1) || ((_parser.Connection != null) && (!_parser.Connection.IsInPool))) + { + // This connection is currently in use, assume that the connection is 'alive' + // NOTE: SNICheckConnection is not currently supported for connections that are in use + Debug.Assert(true, "Call to IsConnectionAlive while connection is in use"); + } + else + { + uint error; +#if NETFRAMEWORK + System.Runtime.CompilerServices.RuntimeHelpers.PrepareConstrainedRegions(); + try +#endif + { +#if NETFRAMEWORK + TdsParser.ReliabilitySection.Assert("unreliable call to IsConnectionAlive"); // you need to setup for a thread abort somewhere before you call this method + +#endif + SniContext = SniContext.Snix_Connect; + + error = CheckConnection(); + if ((error != TdsEnums.SNI_SUCCESS) && (error != TdsEnums.SNI_WAIT_TIMEOUT)) + { + // Connection is dead +#if NETFRAMEWORK + SqlClientEventSource.Log.TryTraceEvent(" received error {0} on idle connection", (int)error); + +#else + SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObject.IsConnectionAlive | Info | State Object Id {0}, received error {1} on idle connection", _objectID, (int)error); +#endif + 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; + } + } +#if NETFRAMEWORK + finally + { + } +#endif + } + } + + return isAlive; + } + /* // leave this in. comes handy if you have to do Console.WriteLine style debugging ;) From 7073564c5a37d2ecb3dbdf8d59e0e993e8621719 Mon Sep 17 00:00:00 2001 From: panoskj Date: Sat, 19 Mar 2022 16:55:49 +0200 Subject: [PATCH 09/22] Merging TdsParserStateObject.ValidateSNIConnection method (note: changed behavior of CheckConnection for netfx, to make it same as netcore, affecting IsConnectionAlive). --- .../Data/SqlClient/TdsParserStateObject.cs | 32 ---------------- .../Data/SqlClient/TdsParserStateObject.cs | 38 ++----------------- .../Data/SqlClient/TdsParserStateObject.cs | 32 ++++++++++++++++ 3 files changed, 35 insertions(+), 67 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 147891976c..30afe41570 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -363,38 +363,6 @@ internal void ResetSnapshot() _snapshotReplay = false; } - /// - /// Checks to see if the underlying connection is still valid (used by idle connection resiliency - for active connections) - /// NOTE: This is not safe to do on a connection that is currently in use - /// NOTE: This will mark the connection as broken if it is found to be dead - /// - /// True if the connection is still alive, otherwise false - internal bool ValidateSNIConnection() - { - if ((_parser == null) || ((_parser.State == TdsParserState.Broken) || (_parser.State == TdsParserState.Closed))) - { - return false; - } - - if (DateTime.UtcNow.Ticks - _lastSuccessfulIOTimer._value <= CheckConnectionWindow) - { - return true; - } - - uint error = TdsEnums.SNI_SUCCESS; - SniContext = SniContext.Snix_Connect; - try - { - Interlocked.Increment(ref _readingCount); - error = CheckConnection(); - } - finally - { - Interlocked.Decrement(ref _readingCount); - } - return (error == TdsEnums.SNI_SUCCESS) || (error == TdsEnums.SNI_WAIT_TIMEOUT); - } - // This method should only be called by ReadSni! If not - it may have problems with timeouts! private void ReadSniError(TdsParserStateObject stateObj, uint error) { diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index d573075412..48822a8d83 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -471,42 +471,10 @@ private IntPtr ReadAsync(SessionHandle handle, out uint error) return readPacket; } - private uint CheckConnection() => SNINativeMethodWrapper.SNICheckConnection(Handle); - - /// - /// Checks to see if the underlying connection is still valid (used by idle connection resiliency - for active connections) - /// NOTE: This is not safe to do on a connection that is currently in use - /// NOTE: This will mark the connection as broken if it is found to be dead - /// - /// True if the connection is still alive, otherwise false - internal bool ValidateSNIConnection() + private uint CheckConnection() { - if ((_parser == null) || ((_parser.State == TdsParserState.Broken) || (_parser.State == TdsParserState.Closed))) - { - return false; - } - - if (DateTime.UtcNow.Ticks - _lastSuccessfulIOTimer._value <= CheckConnectionWindow) - { - return true; - } - - uint error = TdsEnums.SNI_SUCCESS; - SniContext = SniContext.Snix_Connect; - try - { - Interlocked.Increment(ref _readingCount); - SNIHandle handle = Handle; - if (handle != null) - { - error = SNINativeMethodWrapper.SNICheckConnection(handle); - } - } - finally - { - Interlocked.Decrement(ref _readingCount); - } - return (error == TdsEnums.SNI_SUCCESS) || (error == TdsEnums.SNI_WAIT_TIMEOUT); + SNIHandle handle = Handle; + return handle == null ? TdsEnums.SNI_SUCCESS : SNINativeMethodWrapper.SNICheckConnection(handle); } // This method should only be called by ReadSni! If not - it may have problems with timeouts! diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index c42d143ba1..8b37d4ffb8 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -2411,6 +2411,38 @@ internal bool IsConnectionAlive(bool throwOnException) return isAlive; } + /// + /// Checks to see if the underlying connection is still valid (used by idle connection resiliency - for active connections) + /// NOTE: This is not safe to do on a connection that is currently in use + /// NOTE: This will mark the connection as broken if it is found to be dead + /// + /// True if the connection is still alive, otherwise false + internal bool ValidateSNIConnection() + { + if ((_parser == null) || ((_parser.State == TdsParserState.Broken) || (_parser.State == TdsParserState.Closed))) + { + return false; + } + + if (DateTime.UtcNow.Ticks - _lastSuccessfulIOTimer._value <= CheckConnectionWindow) + { + return true; + } + + uint error = TdsEnums.SNI_SUCCESS; + SniContext = SniContext.Snix_Connect; + try + { + Interlocked.Increment(ref _readingCount); + error = CheckConnection(); + } + finally + { + Interlocked.Decrement(ref _readingCount); + } + return (error == TdsEnums.SNI_SUCCESS) || (error == TdsEnums.SNI_WAIT_TIMEOUT); + } + /* // leave this in. comes handy if you have to do Console.WriteLine style debugging ;) From 1ddaeec317cfb15cace905fa7d4bf91e10a37146 Mon Sep 17 00:00:00 2001 From: panoskj Date: Sat, 19 Mar 2022 17:12:30 +0200 Subject: [PATCH 10/22] Merging TdsParserStateObject.ReadSniError method. --- .../Data/SqlClient/TdsParserStateObject.cs | 98 +--------------- .../Data/SqlClient/TdsParserStateObject.cs | 107 +----------------- .../Data/SqlClient/TdsParserStateObject.cs | 99 ++++++++++++++++ 3 files changed, 103 insertions(+), 201 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 30afe41570..7cda8dd37f 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -82,6 +82,8 @@ internal abstract SessionHandle SessionHandle get; } + private static bool TransparentNetworkIPResolution => false; + private partial struct NullBitmap { internal bool TryInitialize(TdsParserStateObject stateObj, int columnsCount) @@ -363,102 +365,6 @@ internal void ResetSnapshot() _snapshotReplay = false; } - // This method should only be called by ReadSni! If not - it may have problems with timeouts! - private void ReadSniError(TdsParserStateObject stateObj, uint error) - { - if (TdsEnums.SNI_WAIT_TIMEOUT == error) - { - Debug.Assert(_syncOverAsync, "Should never reach here with async on!"); - bool fail = false; - - if (IsTimeoutStateExpired) - { // This is now our second timeout - time to give up. - fail = true; - } - else - { - stateObj.SetTimeoutStateStopped(); - Debug.Assert(_parser.Connection != null, "SqlConnectionInternalTds handler can not be null at this point."); - AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, 0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); - - if (!stateObj._attentionSent) - { - if (stateObj.Parser.State == TdsParserState.OpenLoggedIn) - { - stateObj.SendAttention(mustTakeWriteLock: true); - - PacketHandle syncReadPacket = default; - - bool shouldDecrement = false; - try - { - Interlocked.Increment(ref _readingCount); - shouldDecrement = true; - - syncReadPacket = ReadSyncOverAsync(stateObj.GetTimeoutRemaining(), out error); - - Interlocked.Decrement(ref _readingCount); - shouldDecrement = false; - - if (TdsEnums.SNI_SUCCESS == error) - { - // We will end up letting the run method deal with the expected done:done_attn token stream. - stateObj.ProcessSniPacket(syncReadPacket, 0); - return; - } - else - { - Debug.Assert(!IsValidPacket(syncReadPacket), "unexpected syncReadPacket without corresponding SNIPacketRelease"); - fail = true; // Subsequent read failed, time to give up. - } - } - finally - { - if (shouldDecrement) - { - Interlocked.Decrement(ref _readingCount); - } - - if (!IsPacketEmpty(syncReadPacket)) - { - ReleasePacket(syncReadPacket); - } - } - } - else - { - if (_parser._loginWithFailover) - { - // For DbMirroring Failover during login, never break the connection, just close the TdsParser - _parser.Disconnect(); - } - else if ((_parser.State == TdsParserState.OpenNotLoggedIn) && (_parser.Connection.ConnectionOptions.MultiSubnetFailover)) - { - // For MultiSubnet Failover during login, never break the connection, just close the TdsParser - _parser.Disconnect(); - } - else - fail = true; // We aren't yet logged in - just fail. - } - } - } - - if (fail) - { - _parser.State = TdsParserState.Broken; // We failed subsequent read, we have to quit! - _parser.Connection.BreakConnection(); - } - } - else - { - // Caution: ProcessSNIError always returns a fatal error! - AddError(_parser.ProcessSNIError(stateObj)); - } - ThrowExceptionAndWarning(); - - AssertValidState(); - } - public void ProcessSniPacket(PacketHandle packet, uint error) { if (error != 0) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 48822a8d83..59fa665c90 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -33,6 +33,8 @@ internal partial class TdsParserStateObject private SessionHandle SessionHandle => new SessionHandle(_sessionHandle); + private bool TransparentNetworkIPResolution => _parser.Connection.ConnectionOptions.TransparentNetworkIPResolution; + internal bool _pendingData = false; internal bool _errorTokenReceived = false; // Keep track of whether an error was received for the result. // This is reset upon each done token - there can be @@ -477,111 +479,6 @@ private uint CheckConnection() return handle == null ? TdsEnums.SNI_SUCCESS : SNINativeMethodWrapper.SNICheckConnection(handle); } - // 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!"); - bool fail = false; - - if (IsTimeoutStateExpired) - { // This is now our second timeout - time to give up. - fail = true; - } - else - { - stateObj.SetTimeoutStateStopped(); - Debug.Assert(_parser.Connection != null, "SqlConnectionInternalTds handler can not be null at this point."); - AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, 0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); - - if (!stateObj._attentionSent) - { - if (stateObj.Parser.State == TdsParserState.OpenLoggedIn) - { - stateObj.SendAttention(mustTakeWriteLock: true); - - IntPtr syncReadPacket = IntPtr.Zero; - RuntimeHelpers.PrepareConstrainedRegions(); - bool shouldDecrement = false; - try - { - Interlocked.Increment(ref _readingCount); - shouldDecrement = true; - - SNIHandle handle = Handle; - if (handle == null) - { - throw ADP.ClosedConnectionError(); - } - - error = SNINativeMethodWrapper.SNIReadSyncOverAsync(handle, ref syncReadPacket, stateObj.GetTimeoutRemaining()); - - Interlocked.Decrement(ref _readingCount); - shouldDecrement = false; - - if (TdsEnums.SNI_SUCCESS == error) - { - // We will end up letting the run method deal with the expected done:done_attn token stream. - stateObj.ProcessSniPacket(syncReadPacket, 0); - return; - } - else - { - Debug.Assert(IntPtr.Zero == syncReadPacket, "unexpected syncReadPacket without corresponding SNIPacketRelease"); - fail = true; // Subsequent read failed, time to give up. - } - } - finally - { - if (shouldDecrement) - { - Interlocked.Decrement(ref _readingCount); - } - - if (syncReadPacket != IntPtr.Zero) - { - // Be sure to release packet, otherwise it will be leaked by native. - SNINativeMethodWrapper.SNIPacketRelease(syncReadPacket); - } - } - } - else - { - if (_parser._loginWithFailover) - { - // For DB Mirroring Failover during login, never break the connection, just close the TdsParser (Devdiv 846298) - _parser.Disconnect(); - } - else if ((_parser.State == TdsParserState.OpenNotLoggedIn) && (_parser.Connection.ConnectionOptions.MultiSubnetFailover || _parser.Connection.ConnectionOptions.TransparentNetworkIPResolution)) - { - // For MultiSubnet Failover during login, never break the connection, just close the TdsParser - _parser.Disconnect(); - } - else - fail = true; // We aren't yet logged in - just fail. - } - } - } - - if (fail) - { - _parser.State = TdsParserState.Broken; // We failed subsequent read, we have to quit! - _parser.Connection.BreakConnection(); - } - } - else - { - // Caution: ProcessSNIError always returns a fatal error! - AddError(_parser.ProcessSNIError(stateObj)); - } - ThrowExceptionAndWarning(); - - AssertValidState(); - } - // TODO: - does this need to be MUSTRUN??? public void ProcessSniPacket(IntPtr packet, uint error) { diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 8b37d4ffb8..f2c52592b7 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -2443,6 +2443,105 @@ internal bool ValidateSNIConnection() return (error == TdsEnums.SNI_SUCCESS) || (error == TdsEnums.SNI_WAIT_TIMEOUT); } + // This method should only be called by ReadSni! If not - it may have problems with timeouts! + private void ReadSniError(TdsParserStateObject stateObj, uint error) + { + if (TdsEnums.SNI_WAIT_TIMEOUT == error) + { + Debug.Assert(_syncOverAsync, "Should never reach here with async on!"); + bool fail = false; + + if (IsTimeoutStateExpired) + { // This is now our second timeout - time to give up. + fail = true; + } + else + { + stateObj.SetTimeoutStateStopped(); + Debug.Assert(_parser.Connection != null, "SqlConnectionInternalTds handler can not be null at this point."); + AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, (byte)0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); + + if (!stateObj._attentionSent) + { + if (stateObj.Parser.State == TdsParserState.OpenLoggedIn) + { + stateObj.SendAttention(mustTakeWriteLock: true); + + PacketHandle syncReadPacket = default; + +#if NETFRAMEWORK + System.Runtime.CompilerServices.RuntimeHelpers.PrepareConstrainedRegions(); +#endif + bool shouldDecrement = false; + try + { + Interlocked.Increment(ref _readingCount); + shouldDecrement = true; + + syncReadPacket = ReadSyncOverAsync(stateObj.GetTimeoutRemaining(), out error); + + Interlocked.Decrement(ref _readingCount); + shouldDecrement = false; + + if (TdsEnums.SNI_SUCCESS == error) + { + // We will end up letting the run method deal with the expected done:done_attn token stream. + stateObj.ProcessSniPacket(syncReadPacket, 0); + return; + } + else + { + Debug.Assert(!IsValidPacket(syncReadPacket), "unexpected syncReadPacket without corresponding SNIPacketRelease"); + fail = true; // Subsequent read failed, time to give up. + } + } + finally + { + if (shouldDecrement) + { + Interlocked.Decrement(ref _readingCount); + } + + if (!IsPacketEmpty(syncReadPacket)) + { + ReleasePacket(syncReadPacket); + } + } + } + else + { + if (_parser._loginWithFailover) + { + // For DbMirroring Failover during login, never break the connection, just close the TdsParser + _parser.Disconnect(); + } + else if ((_parser.State == TdsParserState.OpenNotLoggedIn) && (_parser.Connection.ConnectionOptions.MultiSubnetFailover || TransparentNetworkIPResolution)) + { + // For MultiSubnet Failover during login, never break the connection, just close the TdsParser + _parser.Disconnect(); + } + else + fail = true; // We aren't yet logged in - just fail. + } + } + } + + if (fail) + { + _parser.State = TdsParserState.Broken; // We failed subsequent read, we have to quit! + _parser.Connection.BreakConnection(); + } + } + else + { + // Caution: ProcessSNIError always returns a fatal error! + AddError(_parser.ProcessSNIError(stateObj)); + } + ThrowExceptionAndWarning(); + + AssertValidState(); + } + /* // leave this in. comes handy if you have to do Console.WriteLine style debugging ;) From 32b2b2f7c76c412685245f6f135ba3ad8b00a4f8 Mon Sep 17 00:00:00 2001 From: panoskj Date: Sat, 19 Mar 2022 18:32:55 +0200 Subject: [PATCH 11/22] Merging of TdsParserStateObject.ProcessSniPacket method. --- .../Data/SqlClient/TdsParserStateObject.cs | 73 ----------------- .../Data/SqlClient/TdsParserStateObject.cs | 72 +---------------- .../Data/SqlClient/TdsParserStateObject.cs | 80 +++++++++++++++++++ 3 files changed, 83 insertions(+), 142 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 7cda8dd37f..b13042a535 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -365,79 +365,6 @@ internal void ResetSnapshot() _snapshotReplay = false; } - public void ProcessSniPacket(PacketHandle packet, uint error) - { - if (error != 0) - { - if ((_parser.State == TdsParserState.Closed) || (_parser.State == TdsParserState.Broken)) - { - // Do nothing with callback if closed or broken and error not 0 - callback can occur - // after connection has been closed. PROBLEM IN NETLIB - DESIGN FLAW. - return; - } - - AddError(_parser.ProcessSNIError(this)); - AssertValidState(); - } - else - { - uint dataSize = 0; - - uint getDataError = SNIPacketGetData(packet, _inBuff, ref dataSize); - - if (getDataError == TdsEnums.SNI_SUCCESS) - { - if (_inBuff.Length < dataSize) - { - Debug.Assert(true, "Unexpected dataSize on Read"); - throw SQL.InvalidInternalPacketSize(StringsHelper.GetString(Strings.SqlMisc_InvalidArraySizeMessage)); - } - - _lastSuccessfulIOTimer._value = DateTime.UtcNow.Ticks; - _inBytesRead = (int)dataSize; - _inBytesUsed = 0; - - if (_snapshot != null) - { - _snapshot.PushBuffer(_inBuff, _inBytesRead); - if (_snapshotReplay) - { - _snapshot.Replay(); -#if DEBUG - _snapshot.AssertCurrent(); -#endif - } - } - - SniReadStatisticsAndTracing(); - - - AssertValidState(); - } - else - { - throw SQL.ParsingError(); - } - } - } - - private void ChangeNetworkPacketTimeout(int dueTime, int period) - { - Timer networkPacketTimeout = _networkPacketTimeout; - if (networkPacketTimeout != null) - { - try - { - networkPacketTimeout.Change(dueTime, period); - } - catch (ObjectDisposedException) - { - // _networkPacketTimeout is set to null before Disposing, but there is still a slight chance - // that object was disposed after we took a copy - } - } - } - private void SetBufferSecureStrings() { if (_securePasswords != null) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 59fa665c90..b3e8ac2f64 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -479,75 +479,9 @@ private uint CheckConnection() return handle == null ? TdsEnums.SNI_SUCCESS : SNINativeMethodWrapper.SNICheckConnection(handle); } - // TODO: - does this need to be MUSTRUN??? - public void ProcessSniPacket(IntPtr packet, uint error) - { - if (error != 0) - { - if ((_parser.State == TdsParserState.Closed) || (_parser.State == TdsParserState.Broken)) - { - // Do nothing with callback if closed or broken and error not 0 - callback can occur - // after connection has been closed. PROBLEM IN NETLIB - DESIGN FLAW. - return; - } - - AddError(_parser.ProcessSNIError(this)); - AssertValidState(); - } - else - { - uint dataSize = 0; - uint getDataError = SNINativeMethodWrapper.SNIPacketGetData(packet, _inBuff, ref dataSize); - - if (getDataError == TdsEnums.SNI_SUCCESS) - { - if (_inBuff.Length < dataSize) - { - Debug.Assert(true, "Unexpected dataSize on Read"); - throw SQL.InvalidInternalPacketSize(StringsHelper.GetString(Strings.SqlMisc_InvalidArraySizeMessage)); - } - - _lastSuccessfulIOTimer._value = DateTime.UtcNow.Ticks; - _inBytesRead = (int)dataSize; - _inBytesUsed = 0; - - if (_snapshot != null) - { - _snapshot.PushBuffer(_inBuff, _inBytesRead); - if (_snapshotReplay) - { - _snapshot.Replay(); -#if DEBUG - _snapshot.AssertCurrent(); -#endif - } - } - 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(ParsingErrorState.ProcessSniPacketFailed); - } - } - } - - private void ChangeNetworkPacketTimeout(int dueTime, int period) - { - Timer networkPacketTimeout = _networkPacketTimeout; - if (networkPacketTimeout != null) - { - try - { - networkPacketTimeout.Change(dueTime, period); - } - catch (ObjectDisposedException) - { - // _networkPacketTimeout is set to null before Disposing, but there is still a slight chance - // that object was disposed after we took a copy - } - } + private uint SNIPacketGetData(IntPtr packet, byte[] _inBuff, ref uint dataSize) + { + return SNINativeMethodWrapper.SNIPacketGetData(packet, _inBuff, ref dataSize); } public void ReadAsyncCallback(IntPtr key, IntPtr packet, uint error) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index f2c52592b7..afb5e02f0c 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -2542,6 +2542,86 @@ private void ReadSniError(TdsParserStateObject stateObj, uint error) AssertValidState(); } + public void ProcessSniPacket(PacketHandle packet, uint error) + { + if (error != 0) + { + if ((_parser.State == TdsParserState.Closed) || (_parser.State == TdsParserState.Broken)) + { + // Do nothing with callback if closed or broken and error not 0 - callback can occur + // after connection has been closed. PROBLEM IN NETLIB - DESIGN FLAW. + return; + } + + AddError(_parser.ProcessSNIError(this)); + AssertValidState(); + } + else + { + uint dataSize = 0; + + uint getDataError = SNIPacketGetData(packet, _inBuff, ref dataSize); + + if (getDataError == TdsEnums.SNI_SUCCESS) + { + if (_inBuff.Length < dataSize) + { + Debug.Assert(true, "Unexpected dataSize on Read"); + throw SQL.InvalidInternalPacketSize(StringsHelper.GetString(Strings.SqlMisc_InvalidArraySizeMessage)); + } + + _lastSuccessfulIOTimer._value = DateTime.UtcNow.Ticks; + _inBytesRead = (int)dataSize; + _inBytesUsed = 0; + + if (_snapshot != null) + { + _snapshot.PushBuffer(_inBuff, _inBytesRead); + if (_snapshotReplay) + { + _snapshot.Replay(); +#if DEBUG + _snapshot.AssertCurrent(); +#endif + } + } + + SniReadStatisticsAndTracing(); + +#if NETFRAMEWORK + SqlClientEventSource.Log.TryAdvancedTraceBinEvent("TdsParser.ReadNetworkPacketAsyncCallback | INFO | ADV | State Object Id {0}, Packet read. In Buffer {1}, In Bytes Read: {2}", ObjectID, _inBuff, (ushort)_inBytesRead); +#endif + + AssertValidState(); + } + else + { +#if NETFRAMEWORK + throw SQL.ParsingError(ParsingErrorState.ProcessSniPacketFailed); +#else + throw SQL.ParsingError(); +#endif + } + } + } + + private void ChangeNetworkPacketTimeout(int dueTime, int period) + { + Timer networkPacketTimeout = _networkPacketTimeout; + if (networkPacketTimeout != null) + { + try + { + networkPacketTimeout.Change(dueTime, period); + } + catch (ObjectDisposedException) + { + // _networkPacketTimeout is set to null before Disposing, but there is still a slight chance + // that object was disposed after we took a copy + } + } + } + /* // leave this in. comes handy if you have to do Console.WriteLine style debugging ;) From 81f549eedfbd8b94b94495c19152ef7f046f4a98 Mon Sep 17 00:00:00 2001 From: panoskj Date: Sat, 19 Mar 2022 18:57:30 +0200 Subject: [PATCH 12/22] Merging of TdsParserStateObject.ReadAsyncCallback (note: ports parts of #378 and #528 to netfx). --- .../Data/SqlClient/TdsParserStateObject.cs | 151 ----------------- .../Data/SqlClient/TdsParserStateObject.cs | 142 +--------------- .../Data/SqlClient/TdsParserStateObject.cs | 156 ++++++++++++++++++ 3 files changed, 158 insertions(+), 291 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index b13042a535..21f36c4774 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -19,8 +19,6 @@ internal abstract partial class TdsParserStateObject { private static bool UseManagedSNI => TdsParserStateObjectFactory.UseManagedSNI; - private static readonly ContextCallback s_readAsyncCallbackComplete = ReadAsyncCallbackComplete; - // Timeout variables private readonly WeakReference _cancellationOwner = new WeakReference(null); @@ -394,157 +392,8 @@ private void SetBufferSecureStrings() public void ReadAsyncCallback(PacketHandle packet, uint error) => ReadAsyncCallback(IntPtr.Zero, packet, error); - public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error) - { - // Key never used. - // Note - it's possible that when native calls managed that an asynchronous exception - // could occur in the native->managed transition, which would - // have two impacts: - // 1) user event not called - // 2) DecrementPendingCallbacks not called, which would mean this object would be leaked due - // 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 source = _networkPacketTaskSource; -#if DEBUG - if ((s_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) - { - source = _realNetworkPacketTaskSource; - } -#endif - - // The mars physical connection can get a callback - // with a packet but no result after the connection is closed. - if (source == null && _parser._pMarsPhysicalConObj == this) - { - return; - } - - bool processFinallyBlock = true; - try - { - 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); - } - - ChangeNetworkPacketTimeout(Timeout.Infinite, Timeout.Infinite); - - // 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. - if (TimeoutHasExpired) - { - OnTimeoutSync(true); - } - - // try to change to the stopped state but only do so if currently in the running state - // and use cmpexch so that all changes out of the running state are atomic - int previousState = Interlocked.CompareExchange(ref _timeoutState, TimeoutState.Stopped, TimeoutState.Running); - - // if the state is anything other than running then this query has reached an end so - // set the correlation _timeoutIdentityValue to 0 to prevent late callbacks executing - if (_timeoutState != TimeoutState.Running) - { - _timeoutIdentityValue = 0; - } - - ProcessSniPacket(packet, error); - } - catch (Exception e) - { - processFinallyBlock = ADP.IsCatchableExceptionType(e); - throw; - } - finally - { - // pendingCallbacks may be 2 after decrementing, this indicates that a fatal timeout is occurring, and therefore we shouldn't complete the task - int pendingCallbacks = DecrementPendingCallbacks(false); // may dispose of GC handle. - if ((processFinallyBlock) && (source != null) && (pendingCallbacks < 2)) - { - if (error == 0) - { - if (_executionContext != null) - { - ExecutionContext.Run(_executionContext, s_readAsyncCallbackComplete, source); - } - else - { - source.TrySetResult(null); - } - } - else - { - if (_executionContext != null) - { - ExecutionContext.Run(_executionContext, state => ReadAsyncCallbackCaptureException((TaskCompletionSource)state), source); - } - else - { - ReadAsyncCallbackCaptureException(source); - } - } - } - - AssertValidState(); - } - } - - private static void ReadAsyncCallbackComplete(object state) - { - TaskCompletionSource source = (TaskCompletionSource)state; - source.TrySetResult(null); - } - protected abstract bool CheckPacket(PacketHandle packet, TaskCompletionSource source); - private void ReadAsyncCallbackCaptureException(TaskCompletionSource source) - { - bool captureSuccess = false; - try - { - if (_hasErrorOrWarning) - { - // Do the close on another thread, since we don't want to block the callback thread - ThrowExceptionAndWarning(asyncClose: true); - } - else if ((_parser.State == TdsParserState.Closed) || (_parser.State == TdsParserState.Broken)) - { - // Connection was closed by another thread before we parsed the packet, so no error was added to the collection - throw ADP.ClosedConnectionError(); - } - } - catch (Exception ex) - { - if (source.TrySetException(ex)) - { - // There was an exception, and it was successfully stored in the task - captureSuccess = true; - } - } - - if (!captureSuccess) - { - // Either there was no exception, or the task was already completed - // This is unusual, but possible if a fatal timeout occurred on another thread (which should mean that the connection is now broken) - Debug.Assert(_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed || _parser.Connection.IsConnectionDoomed, "Failed to capture exception while the connection was still healthy"); - - // The safest thing to do is to ensure that the connection is broken and attempt to cancel the task - // This must be done from another thread to not block the callback thread - Task.Factory.StartNew(() => - { - _parser.State = TdsParserState.Broken; - _parser.Connection.BreakConnection(); - source.TrySetCanceled(); - }); - } - } - public void WriteAsyncCallback(PacketHandle packet, uint sniError) => WriteAsyncCallback(IntPtr.Zero, packet, sniError); diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index b3e8ac2f64..ad5a38ee9d 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -464,6 +464,8 @@ private IntPtr ReadSyncOverAsync(int timeout, out uint error) private bool IsValidPacket(IntPtr packet) => packet != IntPtr.Zero; + private bool CheckPacket(IntPtr packet, TaskCompletionSource source) => IntPtr.Zero == packet || IntPtr.Zero != packet && source != null; + private void ReleasePacket(IntPtr packet) => SNINativeMethodWrapper.SNIPacketRelease(packet); private IntPtr ReadAsync(SessionHandle handle, out uint error) @@ -484,146 +486,6 @@ private uint SNIPacketGetData(IntPtr packet, byte[] _inBuff, ref uint dataSize) return SNINativeMethodWrapper.SNIPacketGetData(packet, _inBuff, ref dataSize); } - public void ReadAsyncCallback(IntPtr key, IntPtr packet, uint error) - { // Key never used. - // Note - it's possible that when native calls managed that an asynchronous exception - // could occur in the native->managed transition, which would - // have two impacts: - // 1) user event not called - // 2) DecrementPendingCallbacks not called, which would mean this object would be leaked due - // 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. - - TaskCompletionSource source = _networkPacketTaskSource; -#if DEBUG - if ((s_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) - { - source = _realNetworkPacketTaskSource; - } -#endif - - // The mars physical connection can get a callback - // with a packet but no result after the connection is closed. - if (source == null && _parser._pMarsPhysicalConObj == this) - { - return; - } - - RuntimeHelpers.PrepareConstrainedRegions(); - bool processFinallyBlock = true; - try - { - Debug.Assert(IntPtr.Zero == packet || IntPtr.Zero != packet && source != null, "AsyncResult null on callback"); - if (_parser.MARSOn) - { // Only take reset lock on MARS and Async. - CheckSetResetConnectionState(error, CallbackType.Read); - } - - ChangeNetworkPacketTimeout(Timeout.Infinite, Timeout.Infinite); - - // 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. - if (TimeoutHasExpired) - { - OnTimeoutSync(asyncClose: true); - } - - // try to change to the stopped state but only do so if currently in the running state - // and use cmpexch so that all changes out of the running state are atomic - int previousState = Interlocked.CompareExchange(ref _timeoutState, TimeoutState.Stopped, TimeoutState.Running); - - // if the state is anything other than running then this query has reached an end so - // set the correlation _timeoutIdentityValue to 0 to prevent late callbacks executing - if (_timeoutState != TimeoutState.Running) - { - _timeoutIdentityValue = 0; - } - - ProcessSniPacket(packet, error); - } - catch (Exception e) - { - processFinallyBlock = ADP.IsCatchableExceptionType(e); - throw; - } - finally - { - // pendingCallbacks may be 2 after decrementing, this indicates that a fatal timeout is occuring, and therefore we shouldn't complete the task - int pendingCallbacks = DecrementPendingCallbacks(false); // may dispose of GC handle. - if ((processFinallyBlock) && (source != null) && (pendingCallbacks < 2)) - { - if (error == 0) - { - if (_executionContext != null) - { - ExecutionContext.Run(_executionContext, (state) => source.TrySetResult(null), null); - } - else - { - source.TrySetResult(null); - } - } - else - { - if (_executionContext != null) - { - ExecutionContext.Run(_executionContext, (state) => ReadAsyncCallbackCaptureException(source), null); - } - else - { - ReadAsyncCallbackCaptureException(source); - } - } - } - - AssertValidState(); - } - } - - private void ReadAsyncCallbackCaptureException(TaskCompletionSource source) - { - bool captureSuccess = false; - try - { - if (_hasErrorOrWarning) - { - // Do the close on another thread, since we don't want to block the callback thread - ThrowExceptionAndWarning(asyncClose: true); - } - else if ((_parser.State == TdsParserState.Closed) || (_parser.State == TdsParserState.Broken)) - { - // Connection was closed by another thread before we parsed the packet, so no error was added to the collection - throw ADP.ClosedConnectionError(); - } - } - catch (Exception ex) - { - if (source.TrySetException(ex)) - { - // There was an exception, and it was successfully stored in the task - captureSuccess = true; - } - } - - if (!captureSuccess) - { - // Either there was no exception, or the task was already completed - // This is unusual, but possible if a fatal timeout occurred on another thread (which should mean that the connection is now broken) - Debug.Assert(_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed || _parser.Connection.IsConnectionDoomed, "Failed to capture exception while the connection was still healthy"); - - // The safest thing to do is to ensure that the connection is broken and attempt to cancel the task - // This must be done from another thread to not block the callback thread - Task.Factory.StartNew(() => - { - _parser.State = TdsParserState.Broken; - _parser.Connection.BreakConnection(); - source.TrySetCanceled(); - }); - } - } - #pragma warning disable 420 // a reference to a volatile field will not be treated as volatile public void WriteAsyncCallback(IntPtr key, IntPtr packet, uint sniError) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index afb5e02f0c..626654e141 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -24,6 +24,8 @@ sealed internal class LastIOTimer partial class TdsParserStateObject { + private static readonly ContextCallback s_readAsyncCallbackComplete = ReadAsyncCallbackComplete; + private static int s_objectTypeCount; // EventSource counter internal readonly int _objectID = Interlocked.Increment(ref s_objectTypeCount); @@ -2622,6 +2624,160 @@ private void ChangeNetworkPacketTimeout(int dueTime, int period) } } + public void ReadAsyncCallback(IntPtr key, PacketHandle packet, uint error) + { + // Key never used. + // Note - it's possible that when native calls managed that an asynchronous exception + // could occur in the native->managed transition, which would + // have two impacts: + // 1) user event not called + // 2) DecrementPendingCallbacks not called, which would mean this object would be leaked due + // 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. +#if !NETFRAMEWORK + SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObject.ReadAsyncCallback | Info | State Object Id {0}, received error {1} on idle connection", _objectID, (int)error); +#endif + + TaskCompletionSource source = _networkPacketTaskSource; +#if DEBUG + if ((s_forcePendingReadsToWaitForUser) && (_realNetworkPacketTaskSource != null)) + { + source = _realNetworkPacketTaskSource; + } +#endif + + // The mars physical connection can get a callback + // with a packet but no result after the connection is closed. + if (source == null && _parser._pMarsPhysicalConObj == this) + { + return; + } + +#if NETFRAMEWORK + System.Runtime.CompilerServices.RuntimeHelpers.PrepareConstrainedRegions(); +#endif + bool processFinallyBlock = true; + try + { + 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); + } + + ChangeNetworkPacketTimeout(Timeout.Infinite, Timeout.Infinite); + + // 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. + if (TimeoutHasExpired) + { + OnTimeoutSync(true); + } + + // try to change to the stopped state but only do so if currently in the running state + // and use cmpexch so that all changes out of the running state are atomic + int previousState = Interlocked.CompareExchange(ref _timeoutState, TimeoutState.Stopped, TimeoutState.Running); + + // if the state is anything other than running then this query has reached an end so + // set the correlation _timeoutIdentityValue to 0 to prevent late callbacks executing + if (_timeoutState != TimeoutState.Running) + { + _timeoutIdentityValue = 0; + } + + ProcessSniPacket(packet, error); + } + catch (Exception e) + { + processFinallyBlock = ADP.IsCatchableExceptionType(e); + throw; + } + finally + { + // pendingCallbacks may be 2 after decrementing, this indicates that a fatal timeout is occurring, and therefore we shouldn't complete the task + int pendingCallbacks = DecrementPendingCallbacks(false); // may dispose of GC handle. + if ((processFinallyBlock) && (source != null) && (pendingCallbacks < 2)) + { + if (error == 0) + { + if (_executionContext != null) + { + ExecutionContext.Run(_executionContext, s_readAsyncCallbackComplete, source); + } + else + { + source.TrySetResult(null); + } + } + else + { + if (_executionContext != null) + { + ExecutionContext.Run(_executionContext, state => ReadAsyncCallbackCaptureException((TaskCompletionSource)state), source); + } + else + { + ReadAsyncCallbackCaptureException(source); + } + } + } + + AssertValidState(); + } + } + + private static void ReadAsyncCallbackComplete(object state) + { + TaskCompletionSource source = (TaskCompletionSource)state; + source.TrySetResult(null); + } + + private void ReadAsyncCallbackCaptureException(TaskCompletionSource source) + { + bool captureSuccess = false; + try + { + if (_hasErrorOrWarning) + { + // Do the close on another thread, since we don't want to block the callback thread + ThrowExceptionAndWarning(asyncClose: true); + } + else if ((_parser.State == TdsParserState.Closed) || (_parser.State == TdsParserState.Broken)) + { + // Connection was closed by another thread before we parsed the packet, so no error was added to the collection + throw ADP.ClosedConnectionError(); + } + } + catch (Exception ex) + { + if (source.TrySetException(ex)) + { + // There was an exception, and it was successfully stored in the task + captureSuccess = true; + } + } + + if (!captureSuccess) + { + // Either there was no exception, or the task was already completed + // This is unusual, but possible if a fatal timeout occurred on another thread (which should mean that the connection is now broken) + Debug.Assert(_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed || _parser.Connection.IsConnectionDoomed, "Failed to capture exception while the connection was still healthy"); + + // The safest thing to do is to ensure that the connection is broken and attempt to cancel the task + // This must be done from another thread to not block the callback thread + Task.Factory.StartNew(() => + { + _parser.State = TdsParserState.Broken; + _parser.Connection.BreakConnection(); + source.TrySetCanceled(); + }); + } + } + /* // leave this in. comes handy if you have to do Console.WriteLine style debugging ;) From ff3d22ec4fa2b606e9eefe87e0f06e0634f04042 Mon Sep 17 00:00:00 2001 From: panoskj Date: Sat, 19 Mar 2022 19:15:31 +0200 Subject: [PATCH 13/22] Merging TdsParserStateObject.WriteAsyncCallback method. --- .../Data/SqlClient/TdsParserStateObject.cs | 83 ------------------ .../Data/SqlClient/TdsParserStateObject.cs | 87 ------------------- .../Data/SqlClient/TdsParserStateObject.cs | 87 +++++++++++++++++++ 3 files changed, 87 insertions(+), 170 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 21f36c4774..5370cadc98 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -397,89 +397,6 @@ public void ReadAsyncCallback(PacketHandle packet, uint error) => public void WriteAsyncCallback(PacketHandle packet, uint sniError) => WriteAsyncCallback(IntPtr.Zero, packet, sniError); - public void WriteAsyncCallback(IntPtr key, PacketHandle packet, uint sniError) - { // Key never used. - RemovePacketFromPendingList(packet); - try - { - if (sniError != TdsEnums.SNI_SUCCESS) - { - SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObject.WriteAsyncCallback | Info | State Object Id {0}, Write async returned error code {1}", _objectID, (int)sniError); - try - { - AddError(_parser.ProcessSNIError(this)); - ThrowExceptionAndWarning(asyncClose: true); - } - catch (Exception e) - { - TaskCompletionSource writeCompletionSource = _writeCompletionSource; - if (writeCompletionSource != null) - { - writeCompletionSource.TrySetException(e); - } - else - { - _delayedWriteAsyncCallbackException = e; - - // Ensure that _delayedWriteAsyncCallbackException is set before checking _writeCompletionSource - Interlocked.MemoryBarrier(); - - // Double check that _writeCompletionSource hasn't been created in the meantime - writeCompletionSource = _writeCompletionSource; - if (writeCompletionSource != null) - { - Exception delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); - if (delayedException != null) - { - writeCompletionSource.TrySetException(delayedException); - } - } - } - - return; - } - } - else - { - _lastSuccessfulIOTimer._value = DateTime.UtcNow.Ticks; - } - } - finally - { -#if DEBUG - if (SqlCommand.DebugForceAsyncWriteDelay > 0) - { - new Timer(obj => - { - Interlocked.Decrement(ref _asyncWriteCount); - TaskCompletionSource writeCompletionSource = _writeCompletionSource; - if (_asyncWriteCount == 0 && writeCompletionSource != null) - { - writeCompletionSource.TrySetResult(null); - } - }, null, SqlCommand.DebugForceAsyncWriteDelay, Timeout.Infinite); - } - else - { -#else - { -#endif - Interlocked.Decrement(ref _asyncWriteCount); - } - } -#if DEBUG - if (SqlCommand.DebugForceAsyncWriteDelay > 0) - { - return; - } -#endif - TaskCompletionSource completionSource = _writeCompletionSource; - if (_asyncWriteCount == 0 && completionSource != null) - { - completionSource.TrySetResult(null); - } - } - ///////////////////////////////////////// // Network/Packet Writing & Processing // ///////////////////////////////////////// diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index ad5a38ee9d..a1d9bdfe2c 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -486,93 +486,6 @@ private uint SNIPacketGetData(IntPtr packet, byte[] _inBuff, ref uint dataSize) return SNINativeMethodWrapper.SNIPacketGetData(packet, _inBuff, ref dataSize); } -#pragma warning disable 420 // a reference to a volatile field will not be treated as volatile - - public void WriteAsyncCallback(IntPtr key, IntPtr packet, uint sniError) - { // Key never used. - RemovePacketFromPendingList(packet); - try - { - if (sniError != TdsEnums.SNI_SUCCESS) - { - SqlClientEventSource.Log.TryTraceEvent(" write async returned error code {0}", (int)sniError); - try - { - AddError(_parser.ProcessSNIError(this)); - ThrowExceptionAndWarning(asyncClose: true); - } - catch (Exception e) - { - TaskCompletionSource writeCompletionSource = _writeCompletionSource; - if (writeCompletionSource != null) - { - writeCompletionSource.TrySetException(e); - } - else - { - _delayedWriteAsyncCallbackException = e; - - // Ensure that _delayedWriteAsyncCallbackException is set before checking _writeCompletionSource - Thread.MemoryBarrier(); - - // Double check that _writeCompletionSource hasn't been created in the meantime - writeCompletionSource = _writeCompletionSource; - if (writeCompletionSource != null) - { - Exception delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); - if (delayedException != null) - { - writeCompletionSource.TrySetException(delayedException); - } - } - } - - return; - } - } - else - { - _lastSuccessfulIOTimer._value = DateTime.UtcNow.Ticks; - } - } - finally - { -#if DEBUG - if (SqlCommand.DebugForceAsyncWriteDelay > 0) - { - new Timer(obj => - { - Interlocked.Decrement(ref _asyncWriteCount); - TaskCompletionSource writeCompletionSource = _writeCompletionSource; - if (_asyncWriteCount == 0 && writeCompletionSource != null) - { - writeCompletionSource.TrySetResult(null); - } - }, null, SqlCommand.DebugForceAsyncWriteDelay, Timeout.Infinite); - } - else - { -#else - { -#endif - Interlocked.Decrement(ref _asyncWriteCount); - } - } -#if DEBUG - if (SqlCommand.DebugForceAsyncWriteDelay > 0) - { - return; - } -#endif - TaskCompletionSource completionSource = _writeCompletionSource; - if (_asyncWriteCount == 0 && completionSource != null) - { - completionSource.TrySetResult(null); - } - } - -#pragma warning restore 420 - ///////////////////////////////////////// // Network/Packet Writing & Processing // ///////////////////////////////////////// diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 626654e141..4bc6315956 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -2778,6 +2778,93 @@ private void ReadAsyncCallbackCaptureException(TaskCompletionSource sour } } + public void WriteAsyncCallback(IntPtr key, PacketHandle packet, uint sniError) + { // Key never used. + RemovePacketFromPendingList(packet); + try + { + if (sniError != TdsEnums.SNI_SUCCESS) + { +#if NETFRAMEWORK + SqlClientEventSource.Log.TryTraceEvent(" write async returned error code {0}", (int)sniError); +#else + SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObject.WriteAsyncCallback | Info | State Object Id {0}, Write async returned error code {1}", _objectID, (int)sniError); +#endif + try + { + AddError(_parser.ProcessSNIError(this)); + ThrowExceptionAndWarning(asyncClose: true); + } + catch (Exception e) + { + TaskCompletionSource writeCompletionSource = _writeCompletionSource; + if (writeCompletionSource != null) + { + writeCompletionSource.TrySetException(e); + } + else + { + _delayedWriteAsyncCallbackException = e; + + // Ensure that _delayedWriteAsyncCallbackException is set before checking _writeCompletionSource + Interlocked.MemoryBarrier(); + + // Double check that _writeCompletionSource hasn't been created in the meantime + writeCompletionSource = _writeCompletionSource; + if (writeCompletionSource != null) + { + Exception delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); + if (delayedException != null) + { + writeCompletionSource.TrySetException(delayedException); + } + } + } + + return; + } + } + else + { + _lastSuccessfulIOTimer._value = DateTime.UtcNow.Ticks; + } + } + finally + { +#if DEBUG + if (SqlCommand.DebugForceAsyncWriteDelay > 0) + { + new Timer(obj => + { + Interlocked.Decrement(ref _asyncWriteCount); + TaskCompletionSource writeCompletionSource = _writeCompletionSource; + if (_asyncWriteCount == 0 && writeCompletionSource != null) + { + writeCompletionSource.TrySetResult(null); + } + }, null, SqlCommand.DebugForceAsyncWriteDelay, Timeout.Infinite); + } + else + { +#else + { +#endif + Interlocked.Decrement(ref _asyncWriteCount); + } + } +#if DEBUG + if (SqlCommand.DebugForceAsyncWriteDelay > 0) + { + return; + } +#endif + TaskCompletionSource completionSource = _writeCompletionSource; + if (_asyncWriteCount == 0 && completionSource != null) + { + completionSource.TrySetResult(null); + } + } + /* // leave this in. comes handy if you have to do Console.WriteLine style debugging ;) From 567d0e6ae646039db713e4bb3d65accb42b9cc47 Mon Sep 17 00:00:00 2001 From: panoskj Date: Sat, 19 Mar 2022 19:23:12 +0200 Subject: [PATCH 14/22] Merging some "Network/Packet Writing & Processing" methods of TdsParserStateObject. --- .../Data/SqlClient/TdsParserStateObject.cs | 86 --------------- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 2 +- .../Data/SqlClient/TdsParserStateObject.cs | 102 ------------------ .../Data/SqlClient/TdsParserStateObject.cs | 98 +++++++++++++++++ 4 files changed, 99 insertions(+), 189 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 5370cadc98..031d8506aa 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -400,92 +400,6 @@ public void WriteAsyncCallback(PacketHandle packet, uint sniError) => ///////////////////////////////////////// // Network/Packet Writing & Processing // ///////////////////////////////////////// - internal void WriteSecureString(SecureString secureString) - { - Debug.Assert(_securePasswords[0] == null || _securePasswords[1] == null, "There are more than two secure passwords"); - - int index = _securePasswords[0] != null ? 1 : 0; - - _securePasswords[index] = secureString; - _securePasswordOffsetsInBuffer[index] = _outBytesUsed; - - // loop through and write the entire array - int lengthInBytes = secureString.Length * 2; - - // It is guaranteed both secure password and secure change password should fit into the first packet - // Given current TDS format and implementation it is not possible that one of secure string is the last item and exactly fill up the output buffer - // if this ever happens and it is correct situation, the packet needs to be written out after _outBytesUsed is update - Debug.Assert((_outBytesUsed + lengthInBytes) < _outBuff.Length, "Passwords cannot be split into two different packet or the last item which fully fill up _outBuff!!!"); - - _outBytesUsed += lengthInBytes; - } - - internal void ResetSecurePasswordsInformation() - { - for (int i = 0; i < _securePasswords.Length; ++i) - { - _securePasswords[i] = null; - _securePasswordOffsetsInBuffer[i] = 0; - } - } - - internal Task WaitForAccumulatedWrites() - { - // Checked for stored exceptions - Exception delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); - if (delayedException != null) - { - throw delayedException; - } -#pragma warning restore 420 - - if (_asyncWriteCount == 0) - { - return null; - } - - _writeCompletionSource = new TaskCompletionSource(); - Task task = _writeCompletionSource.Task; - - // Ensure that _writeCompletionSource is set before checking state - Interlocked.MemoryBarrier(); - - // Now that we have set _writeCompletionSource, check if parser is closed or broken - if ((_parser.State == TdsParserState.Closed) || (_parser.State == TdsParserState.Broken)) - { - throw ADP.ClosedConnectionError(); - } - - // Check for stored exceptions - delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); - if (delayedException != null) - { - throw delayedException; - } - - // If there are no outstanding writes, see if we can shortcut and return null - if ((_asyncWriteCount == 0) && ((!task.IsCompleted) || (task.Exception == null))) - { - task = null; - } - - return task; - } - - // Takes in a single byte and writes it to the buffer. If the buffer is full, it is flushed - // and then the buffer is re-initialized in flush() and then the byte is put in the buffer. - internal void WriteByte(byte b) - { - 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 - if (_outBytesUsed == _outBuff.Length) - { - WritePacket(TdsEnums.SOFTFLUSH, canAccumulate: true); - } - // set byte in buffer and increment the counter for number of bytes used in the out buffer - _outBuff[_outBytesUsed++] = b; - } internal Task WriteByteSpan(ReadOnlySpan span, bool canAccumulate = true, TaskCompletionSource completion = null) { diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index 70673ef343..43eadfe39b 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -9340,7 +9340,7 @@ internal void TdsLogin(SqlLogin rec, } _physicalStateObj.WritePacket(TdsEnums.HARDFLUSH); - _physicalStateObj.ResetSecurePasswordsInfomation(); // Password information is needed only from Login process; done with writing login packet and should clear information + _physicalStateObj.ResetSecurePasswordsInformation(); // Password information is needed only from Login process; done with writing login packet and should clear information _physicalStateObj._pendingData = true; _physicalStateObj._messageStatus = 0; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index a1d9bdfe2c..78be9cacb4 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -490,108 +490,6 @@ private uint SNIPacketGetData(IntPtr packet, byte[] _inBuff, ref uint dataSize) // 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) - { - 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; - - _securePasswords[index] = secureString; - _securePasswordOffsetsInBuffer[index] = _outBytesUsed; - - // loop through and write the entire array - int lengthInBytes = secureString.Length * 2; - - // It is guaranteed both secure password and secure change password should fit into the first packet - // Given current TDS format and implementation it is not possible that one of secure string is the last item and exactly fill up the output buffer - // if this ever happens and it is correct situation, the packet needs to be written out after _outBytesUsed is update - Debug.Assert((_outBytesUsed + lengthInBytes) < _outBuff.Length, "Passwords cannot be split into two different packet or the last item which fully fill up _outBuff!!!"); - - _outBytesUsed += lengthInBytes; - } - - // ResetSecurePasswordsInformation: clears information regarding secure passwords when login is done; called from TdsParser.TdsLogin - internal void ResetSecurePasswordsInfomation() - { - for (int i = 0; i < _securePasswords.Length; ++i) - { - _securePasswords[i] = null; - _securePasswordOffsetsInBuffer[i] = 0; - } - } - - internal Task WaitForAccumulatedWrites() - { - // Checked for stored exceptions -#pragma warning disable 420 // A reference to a volatile field will not be treated as volatile - Disabling since the Interlocked APIs are volatile aware - Exception delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); - if (delayedException != null) - { - throw delayedException; - } -#pragma warning restore 420 - - if (_asyncWriteCount == 0) - { - return null; - } - - _writeCompletionSource = new TaskCompletionSource(); - Task task = _writeCompletionSource.Task; - - // Ensure that _writeCompletionSource is set before checking state - Thread.MemoryBarrier(); - - // Now that we have set _writeCompletionSource, check if parser is closed or broken - if ((_parser.State == TdsParserState.Closed) || (_parser.State == TdsParserState.Broken)) - { - throw ADP.ClosedConnectionError(); - } - - // Check for stored exceptions -#pragma warning disable 420 // A reference to a volatile field will not be treated as volatile - Disabling since the Interlocked APIs are volatile aware - delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); - if (delayedException != null) - { - throw delayedException; - } -#pragma warning restore 420 - - // If there are no outstanding writes, see if we can shortcut and return null - if ((_asyncWriteCount == 0) && ((!task.IsCompleted) || (task.Exception == null))) - { - task = null; - } - - return task; - } - - // Takes in a single byte and writes it to the buffer. If the buffer is full, it is flushed - // 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 - if (_outBytesUsed == _outBuff.Length) - { - WritePacket(TdsEnums.SOFTFLUSH, canAccumulate: true); - } - // set byte in buffer and increment the counter for number of bytes used in the out buffer - _outBuff[_outBytesUsed++] = b; - } - // // Takes a byte array and writes it to the buffer. // diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 4bc6315956..edc619c960 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -2865,6 +2865,104 @@ public void WriteAsyncCallback(IntPtr key, PacketHandle packet, uint sniError) } } + + ///////////////////////////////////////// + // Network/Packet Writing & Processing // + ///////////////////////////////////////// + internal void WriteSecureString(SecureString secureString) + { +#if NETFRAMEWORK + TdsParser.ReliabilitySection.Assert("unreliable call to WriteSecureString"); // you need to setup for a thread abort somewhere before you call this method +#endif + + Debug.Assert(_securePasswords[0] == null || _securePasswords[1] == null, "There are more than two secure passwords"); + + int index = _securePasswords[0] != null ? 1 : 0; + + _securePasswords[index] = secureString; + _securePasswordOffsetsInBuffer[index] = _outBytesUsed; + + // loop through and write the entire array + int lengthInBytes = secureString.Length * 2; + + // It is guaranteed both secure password and secure change password should fit into the first packet + // Given current TDS format and implementation it is not possible that one of secure string is the last item and exactly fill up the output buffer + // if this ever happens and it is correct situation, the packet needs to be written out after _outBytesUsed is update + Debug.Assert((_outBytesUsed + lengthInBytes) < _outBuff.Length, "Passwords cannot be split into two different packet or the last item which fully fill up _outBuff!!!"); + + _outBytesUsed += lengthInBytes; + } + + internal void ResetSecurePasswordsInformation() + { + for (int i = 0; i < _securePasswords.Length; ++i) + { + _securePasswords[i] = null; + _securePasswordOffsetsInBuffer[i] = 0; + } + } + + internal Task WaitForAccumulatedWrites() + { + // Checked for stored exceptions + Exception delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); + if (delayedException != null) + { + throw delayedException; + } + + if (_asyncWriteCount == 0) + { + return null; + } + + _writeCompletionSource = new TaskCompletionSource(); + Task task = _writeCompletionSource.Task; + + // Ensure that _writeCompletionSource is set before checking state + Interlocked.MemoryBarrier(); + + // Now that we have set _writeCompletionSource, check if parser is closed or broken + if ((_parser.State == TdsParserState.Closed) || (_parser.State == TdsParserState.Broken)) + { + throw ADP.ClosedConnectionError(); + } + + // Check for stored exceptions + delayedException = Interlocked.Exchange(ref _delayedWriteAsyncCallbackException, null); + if (delayedException != null) + { + throw delayedException; + } + + // If there are no outstanding writes, see if we can shortcut and return null + if ((_asyncWriteCount == 0) && ((!task.IsCompleted) || (task.Exception == null))) + { + task = null; + } + + return task; + } + + // Takes in a single byte and writes it to the buffer. If the buffer is full, it is flushed + // and then the buffer is re-initialized in flush() and then the byte is put in the buffer. + internal void WriteByte(byte b) + { +#if NETFRAMEWORK + TdsParser.ReliabilitySection.Assert("unreliable call to WriteByte"); // you need to setup for a thread abort somewhere before you call this method +#endif + + 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 + if (_outBytesUsed == _outBuff.Length) + { + WritePacket(TdsEnums.SOFTFLUSH, canAccumulate: true); + } + // set byte in buffer and increment the counter for number of bytes used in the out buffer + _outBuff[_outBytesUsed++] = b; + } + /* // leave this in. comes handy if you have to do Console.WriteLine style debugging ;) From 96c2e6863c7e00d71cb84e27469443a7e4107e25 Mon Sep 17 00:00:00 2001 From: panoskj Date: Sun, 20 Mar 2022 11:45:05 +0200 Subject: [PATCH 15/22] Merging constructor of TdsParserStateObject. --- .../Data/SqlClient/TdsParserStateObject.cs | 36 ------------- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 2 +- .../Data/SqlClient/TdsParserStateObject.cs | 52 +++++-------------- .../Data/SqlClient/TdsParserStateObject.cs | 32 ++++++++++++ 4 files changed, 45 insertions(+), 77 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 031d8506aa..d3a87474c8 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -26,42 +26,6 @@ internal abstract partial class TdsParserStateObject private StateSnapshot _cachedSnapshot; private SnapshottedStateFlags _snapshottedState; - ////////////////// - // Constructors // - ////////////////// - - internal TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalConnection, bool async) - { - // Construct a MARS session - Debug.Assert(null != parser, "no parser?"); - _parser = parser; - _onTimeoutAsync = OnTimeoutAsync; - SniContext = SniContext.Snix_GetMarsSession; - - Debug.Assert(null != _parser._physicalStateObj, "no physical session?"); - Debug.Assert(null != _parser._physicalStateObj._inBuff, "no in buffer?"); - Debug.Assert(null != _parser._physicalStateObj._outBuff, "no out buffer?"); - Debug.Assert(_parser._physicalStateObj._outBuff.Length == - _parser._physicalStateObj._inBuff.Length, "Unexpected unequal buffers."); - - // Determine packet size based on physical connection buffer lengths. - SetPacketSize(_parser._physicalStateObj._outBuff.Length); - - CreateSessionHandle(physicalConnection, async); - - if (IsFailedHandle()) - { - AddError(parser.ProcessSNIError(this)); - ThrowExceptionAndWarning(); - } - - // we post a callback that represents the call to dispose; once the - // object is disposed, the next callback will cause the GC Handle to - // be released. - IncrementPendingCallbacks(); - _lastSuccessfulIOTimer = parser._physicalStateObj._lastSuccessfulIOTimer; - } - //////////////// // Properties // //////////////// diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index 43eadfe39b..ca8e2418d4 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -949,7 +949,7 @@ internal void EnableMars() internal TdsParserStateObject CreateSession() { - TdsParserStateObject session = new TdsParserStateObject(this, (SNIHandle)_pMarsPhysicalConObj.Handle, true); + TdsParserStateObject session = new TdsParserStateObject(this, _pMarsPhysicalConObj, true); SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0} created session {1}", ObjectID, session.ObjectID); return session; } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 78be9cacb4..a6e7fce81f 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -66,46 +66,6 @@ internal partial class TdsParserStateObject internal bool _receivedColMetaData; // Used to keep track of when to fire StatementCompleted event. - ////////////////// - // Constructors // - ////////////////// - - internal TdsParserStateObject(TdsParser parser, SNIHandle physicalConnection, bool async) - { - // Construct a MARS session - Debug.Assert(null != parser, "no parser?"); - _parser = parser; - _onTimeoutAsync = OnTimeoutAsync; - SniContext = SniContext.Snix_GetMarsSession; - - Debug.Assert(null != _parser._physicalStateObj, "no physical session?"); - Debug.Assert(null != _parser._physicalStateObj._inBuff, "no in buffer?"); - Debug.Assert(null != _parser._physicalStateObj._outBuff, "no out buffer?"); - Debug.Assert(_parser._physicalStateObj._outBuff.Length == - _parser._physicalStateObj._inBuff.Length, "Unexpected unequal buffers."); - - // Determine packet size based on physical connection buffer lengths. - SetPacketSize(_parser._physicalStateObj._outBuff.Length); - - SNINativeMethodWrapper.ConsumerInfo myInfo = CreateConsumerInfo(async); - SQLDNSInfo cachedDNSInfo; - - SQLFallbackDNSCache.Instance.GetDNSInfo(_parser.FQDNforDNSCache, out cachedDNSInfo); - - _sessionHandle = new SNIHandle(myInfo, physicalConnection, _parser.Connection.ConnectionOptions.IPAddressPreference, cachedDNSInfo); - if (_sessionHandle.Status != TdsEnums.SNI_SUCCESS) - { - AddError(parser.ProcessSNIError(this)); - ThrowExceptionAndWarning(); - } - - // we post a callback that represents the call to dispose; once the - // object is disposed, the next callback will cause the GC Handle to - // be released. - IncrementPendingCallbacks(); - _lastSuccessfulIOTimer = parser._physicalStateObj._lastSuccessfulIOTimer; - } - //////////////// // Properties // //////////////// @@ -460,6 +420,18 @@ private IntPtr ReadSyncOverAsync(int timeout, out uint error) return readPacket; } + private void CreateSessionHandle(TdsParserStateObject physicalConnection, bool async) + { + SNINativeMethodWrapper.ConsumerInfo myInfo = CreateConsumerInfo(async); + + SQLDNSInfo cachedDNSInfo; + SQLFallbackDNSCache.Instance.GetDNSInfo(_parser.FQDNforDNSCache, out cachedDNSInfo); + + _sessionHandle = new SNIHandle(myInfo, physicalConnection.Handle, _parser.Connection.ConnectionOptions.IPAddressPreference, cachedDNSInfo); + } + + private bool IsFailedHandle() => _sessionHandle.Status != TdsEnums.SNI_SUCCESS; + private bool IsPacketEmpty(IntPtr packet) => packet == IntPtr.Zero; private bool IsValidPacket(IntPtr packet) => packet != IntPtr.Zero; diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index edc619c960..3845afacb1 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -303,6 +303,38 @@ internal TdsParserStateObject(TdsParser parser) _lastSuccessfulIOTimer = new LastIOTimer(); } + internal TdsParserStateObject(TdsParser parser, TdsParserStateObject physicalConnection, bool async) + { + // Construct a MARS session + Debug.Assert(null != parser, "no parser?"); + _parser = parser; + _onTimeoutAsync = OnTimeoutAsync; + SniContext = SniContext.Snix_GetMarsSession; + + Debug.Assert(null != _parser._physicalStateObj, "no physical session?"); + Debug.Assert(null != _parser._physicalStateObj._inBuff, "no in buffer?"); + Debug.Assert(null != _parser._physicalStateObj._outBuff, "no out buffer?"); + Debug.Assert(_parser._physicalStateObj._outBuff.Length == + _parser._physicalStateObj._inBuff.Length, "Unexpected unequal buffers."); + + // Determine packet size based on physical connection buffer lengths. + SetPacketSize(_parser._physicalStateObj._outBuff.Length); + + CreateSessionHandle(physicalConnection, async); + + if (IsFailedHandle()) + { + AddError(parser.ProcessSNIError(this)); + ThrowExceptionAndWarning(); + } + + // we post a callback that represents the call to dispose; once the + // object is disposed, the next callback will cause the GC Handle to + // be released. + IncrementPendingCallbacks(); + _lastSuccessfulIOTimer = parser._physicalStateObj._lastSuccessfulIOTimer; + } + //////////////// // Properties // //////////////// From 32b269986e4b018a25f6fb4f2a7e23c02dd8ede1 Mon Sep 17 00:00:00 2001 From: panoskj Date: Sun, 20 Mar 2022 11:54:57 +0200 Subject: [PATCH 16/22] Merging common type NullBitmap of TdsParserStateObject. --- .../Data/SqlClient/TdsParserStateObject.cs | 27 ---------------- .../Data/SqlClient/TdsParserStateObject.cs | 27 ---------------- .../Data/SqlClient/TdsParserStateObject.cs | 31 ++++++++++++++++++- 3 files changed, 30 insertions(+), 55 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index d3a87474c8..7b08a7310b 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -46,33 +46,6 @@ internal abstract SessionHandle SessionHandle private static bool TransparentNetworkIPResolution => false; - 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}, Null Bitmap length {1}, NBCROW bitmap data: {2}", stateObj._objectID, (ushort)_nullBitmap.Length, _nullBitmap); - return true; - } - } - ///////////////////// // General methods // ///////////////////// diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index a6e7fce81f..3e3c1e8a89 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -104,33 +104,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 // ///////////////////// diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 3845afacb1..333b37d59f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -504,11 +504,40 @@ internal bool IsNullCompressionBitSet(int columnOrdinal) return _nullBitmapInfo.IsGuaranteedNull(columnOrdinal); } - private partial struct NullBitmap + private struct NullBitmap { private byte[] _nullBitmap; private int _columnsCount; // set to 0 if not used or > 0 for NBC rows + 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); +#if NETFRAMEWORK + 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); +#else + 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); +#endif + + return true; + } + internal bool ReferenceEquals(NullBitmap obj) { return object.ReferenceEquals(_nullBitmap, obj._nullBitmap); From c910c0665aa459d4c79b0e263a7425282bb1b49f Mon Sep 17 00:00:00 2001 From: panoskj Date: Sun, 20 Mar 2022 12:45:19 +0200 Subject: [PATCH 17/22] Merging StartSession, Cancel and ResetCancelAndProcessAttention methods of TdsParserStateObject. --- .../Data/SqlClient/TdsParserStateObject.cs | 95 --------------- .../Microsoft/Data/SqlClient/SqlBulkCopy.cs | 2 +- .../Microsoft/Data/SqlClient/SqlCommand.cs | 6 +- .../Microsoft/Data/SqlClient/SqlDataReader.cs | 5 +- .../Data/SqlClient/TdsParserStateObject.cs | 113 ------------------ .../Data/SqlClient/TdsParserStateObject.cs | 108 +++++++++++++++++ 6 files changed, 115 insertions(+), 214 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 7b08a7310b..fb4349fc09 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -19,9 +19,6 @@ internal abstract partial class TdsParserStateObject { private static bool UseManagedSNI => TdsParserStateObjectFactory.UseManagedSNI; - // Timeout variables - private readonly WeakReference _cancellationOwner = new WeakReference(null); - // Async private StateSnapshot _cachedSnapshot; private SnapshottedStateFlags _snapshottedState; @@ -50,93 +47,6 @@ internal abstract SessionHandle SessionHandle // General methods // ///////////////////// - // This method is only called by the command or datareader as a result of a user initiated - // cancel request. - internal void Cancel(object caller) - { - Debug.Assert(caller != null, "Null caller for Cancel!"); - Debug.Assert(caller is SqlCommand || caller is SqlDataReader, "Calling API with invalid caller type: " + caller.GetType()); - - bool hasLock = false; - try - { - // Keep looping until we either grabbed the lock (and therefore sent attention) or the connection closes\breaks - while ((!hasLock) && (_parser.State != TdsParserState.Closed) && (_parser.State != TdsParserState.Broken)) - { - Monitor.TryEnter(this, WaitForCancellationLockPollTimeout, ref hasLock); - if (hasLock) - { // Lock for the time being - since we need to synchronize the attention send. - // This lock is also protecting against concurrent close and async continuations - - // Ensure that, once we have the lock, that we are still the owner - if ((!_cancelled) && (_cancellationOwner.Target == caller)) - { - _cancelled = true; - - if (HasPendingData && !_attentionSent) - { - bool hasParserLock = false; - // Keep looping until we have the parser lock (and so are allowed to write), or the connection closes\breaks - while ((!hasParserLock) && (_parser.State != TdsParserState.Closed) && (_parser.State != TdsParserState.Broken)) - { - try - { - _parser.Connection._parserLock.Wait(canReleaseFromAnyThread: false, timeout: WaitForCancellationLockPollTimeout, lockTaken: ref hasParserLock); - if (hasParserLock) - { - _parser.Connection.ThreadHasParserLockForClose = true; - SendAttention(); - } - } - finally - { - if (hasParserLock) - { - if (_parser.Connection.ThreadHasParserLockForClose) - { - _parser.Connection.ThreadHasParserLockForClose = false; - } - _parser.Connection._parserLock.Release(); - } - } - } - } - } - } - } - } - finally - { - if (hasLock) - { - Monitor.Exit(this); - } - } - } - - private void ResetCancelAndProcessAttention() - { - // This method is shared by CloseSession initiated by DataReader.Close or completed - // command execution, as well as the session reclamation code for cases where the - // DataReader is opened and then GC'ed. - lock (this) - { - // Reset cancel state. - _cancelled = false; - _cancellationOwner.Target = null; - - if (_attentionSent) - { - // Make sure we're cleaning up the AttentionAck if Cancel happened before taking the lock. - // We serialize Cancel/CloseSession to prevent a race condition between these two states. - // The problem is that both sending and receiving attentions are time taking - // operations. - Parser.ProcessPendingAck(this); - } - SetTimeoutStateStopped(); - } - } - internal abstract void CreatePhysicalSNIHandle( string serverName, bool ignoreSniOpenTimeout, @@ -216,11 +126,6 @@ internal int IncrementPendingCallbacks() return remaining; } - internal void StartSession(object cancellationOwner) - { - _cancellationOwner.Target = cancellationOwner; - } - private void SetSnapshottedState(SnapshottedStateFlags flag, bool value) { if (value) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs index 81ccfb570a..51ebef2948 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs @@ -3088,7 +3088,7 @@ private void WriteToServerInternalRestAsync(CancellationToken cts, TaskCompletio { _stateObj = _parser.GetSession(this); _stateObj._bulkCopyOpperationInProgress = true; - _stateObj.StartSession(ObjectID); + _stateObj.StartSession(this); } finally { diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs index fdfa0772ea..6297090df5 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -1283,14 +1283,14 @@ public override void Cancel() TdsParserStateObject stateObj = _stateObj; if (null != stateObj) { - stateObj.Cancel(ObjectID); + stateObj.Cancel(this); } else { SqlDataReader reader = connection.FindLiveReader(this); if (reader != null) { - reader.Cancel(ObjectID); + reader.Cancel(this); } } } @@ -5986,7 +5986,7 @@ private void GetStateObject(TdsParser parser = null) } TdsParserStateObject stateObj = parser.GetSession(this); - stateObj.StartSession(ObjectID); + stateObj.StartSession(this); _stateObj = stateObj; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs index fd7306a8f7..c404f3da8d 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs @@ -797,12 +797,13 @@ internal DataTable BuildSchemaTable() return schemaTable; } - internal void Cancel(int objectID) + internal void Cancel(SqlCommand command) { + Debug.Assert(command == _command, "Calling command from an object that isn't this reader's command"); TdsParserStateObject stateObj = _stateObj; if (null != stateObj) { - stateObj.Cancel(objectID); + stateObj.Cancel(command); } } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 3e3c1e8a89..d4c696b5eb 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -50,12 +50,6 @@ internal partial class TdsParserStateObject // Timeout variables internal bool _attentionReceived = false; // NOTE: Received is not volatile as it is only ever accessed\modified by TryRun its callees (i.e. single threaded access) - // This variable is used to prevent sending an attention by another thread that is not the - // current owner of the stateObj. I currently do not know how this can happen. Mark added - // the code but does not remember either. At some point, we need to research killing this - // logic. - private volatile int _allowObjectID; - internal bool _hasOpenResult = false; // Used for blanking out password in trace. @@ -108,108 +102,6 @@ internal uint Status // General methods // ///////////////////// - // This method is only called by the command or datareader as a result of a user initiated - // cancel request. - internal void Cancel(int objectID) - { - bool hasLock = false; - try - { - // Keep looping until we either grabbed the lock (and therefore sent attention) or the connection closes\breaks - while ((!hasLock) && (_parser.State != TdsParserState.Closed) && (_parser.State != TdsParserState.Broken)) - { - - Monitor.TryEnter(this, WaitForCancellationLockPollTimeout, ref hasLock); - if (hasLock) - { // Lock for the time being - since we need to synchronize the attention send. - // At some point in the future, I hope to remove this. - // This lock is also protecting against concurrent close and async continuations - - // don't allow objectID -1 since it is reserved for 'not associated with a command' - // yes, the 2^32-1 comand won't cancel - but it also won't cancel when we don't want it - if ((!_cancelled) && (objectID == _allowObjectID) && (objectID != -1)) - { - _cancelled = true; - - if (_pendingData && !_attentionSent) - { - bool hasParserLock = false; - // Keep looping until we have the parser lock (and so are allowed to write), or the conneciton closes\breaks - while ((!hasParserLock) && (_parser.State != TdsParserState.Closed) && (_parser.State != TdsParserState.Broken)) - { - try - { - _parser.Connection._parserLock.Wait(canReleaseFromAnyThread: false, timeout: WaitForCancellationLockPollTimeout, lockTaken: ref hasParserLock); - if (hasParserLock) - { - _parser.Connection.ThreadHasParserLockForClose = true; - SendAttention(); - } - } - finally - { - if (hasParserLock) - { - if (_parser.Connection.ThreadHasParserLockForClose) - { - _parser.Connection.ThreadHasParserLockForClose = false; - } - _parser.Connection._parserLock.Release(); - } - } - } - } - } - } - } - } - finally - { - if (hasLock) - { - Monitor.Exit(this); - } - } - } - - private void ResetCancelAndProcessAttention() - { - // This method is shared by CloseSession initiated by DataReader.Close or completed - // command execution, as well as the session reclaimation code for cases where the - // DataReader is opened and then GC'ed. - lock (this) - { - // Reset cancel state. - _cancelled = false; - _allowObjectID = -1; - - if (_attentionSent) - { - // Make sure we're cleaning up the AttentionAck if Cancel happened before taking the lock. - // We serialize Cancel/CloseSession to prevent a race condition between these two states. - // The problem is that both sending and receiving attentions are time taking - // operations. -#if DEBUG - TdsParser.ReliabilitySection tdsReliabilitySection = new TdsParser.ReliabilitySection(); - - RuntimeHelpers.PrepareConstrainedRegions(); - try - { - tdsReliabilitySection.Start(); -#endif //DEBUG - Parser.ProcessPendingAck(this); -#if DEBUG - } - finally - { - tdsReliabilitySection.Stop(); - } -#endif //DEBUG - } - SetTimeoutStateStopped(); - } - } - private SNINativeMethodWrapper.ConsumerInfo CreateConsumerInfo(bool async) { SNINativeMethodWrapper.ConsumerInfo myInfo = new SNINativeMethodWrapper.ConsumerInfo(); @@ -359,11 +251,6 @@ internal int IncrementPendingCallbacks() return remaining; } - internal void StartSession(int objectID) - { - _allowObjectID = objectID; - } - ///////////////////////////////////////// // Network/Packet Reading & Processing // ///////////////////////////////////////// diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 333b37d59f..c1bfddf69f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -166,6 +166,7 @@ public TimeoutState(int value) // 3) post session close - no attention is allowed private bool _cancelled; private const int WaitForCancellationLockPollTimeout = 100; + private WeakReference _cancellationOwner = new WeakReference(null); // Cache the transaction for which this command was executed so upon completion we can // decrement the appropriate result count. @@ -593,6 +594,70 @@ internal void Activate(object owner) Debug.Assert(result == 1, "invalid deactivate count"); } + // This method is only called by the command or datareader as a result of a user initiated + // cancel request. + internal void Cancel(object caller) + { + Debug.Assert(caller != null, "Null caller for Cancel!"); + Debug.Assert(caller is SqlCommand || caller is SqlDataReader, "Calling API with invalid caller type: " + caller.GetType()); + + bool hasLock = false; + try + { + // Keep looping until we either grabbed the lock (and therefore sent attention) or the connection closes\breaks + while ((!hasLock) && (_parser.State != TdsParserState.Closed) && (_parser.State != TdsParserState.Broken)) + { + Monitor.TryEnter(this, WaitForCancellationLockPollTimeout, ref hasLock); + if (hasLock) + { // Lock for the time being - since we need to synchronize the attention send. + // This lock is also protecting against concurrent close and async continuations + + // Ensure that, once we have the lock, that we are still the owner + if ((!_cancelled) && (_cancellationOwner.Target == caller)) + { + _cancelled = true; + + if (HasPendingData && !_attentionSent) + { + bool hasParserLock = false; + // Keep looping until we have the parser lock (and so are allowed to write), or the connection closes\breaks + while ((!hasParserLock) && (_parser.State != TdsParserState.Closed) && (_parser.State != TdsParserState.Broken)) + { + try + { + _parser.Connection._parserLock.Wait(canReleaseFromAnyThread: false, timeout: WaitForCancellationLockPollTimeout, lockTaken: ref hasParserLock); + if (hasParserLock) + { + _parser.Connection.ThreadHasParserLockForClose = true; + SendAttention(); + } + } + finally + { + if (hasParserLock) + { + if (_parser.Connection.ThreadHasParserLockForClose) + { + _parser.Connection.ThreadHasParserLockForClose = false; + } + _parser.Connection._parserLock.Release(); + } + } + } + } + } + } + } + } + finally + { + if (hasLock) + { + Monitor.Exit(this); + } + } + } + // CancelRequest - use to cancel while writing a request to the server // // o none of the request might have been sent to the server, simply reset the buffer, @@ -669,6 +734,44 @@ internal void CloseSession() Parser.PutSession(this); } + private void ResetCancelAndProcessAttention() + { + // This method is shared by CloseSession initiated by DataReader.Close or completed + // command execution, as well as the session reclamation code for cases where the + // DataReader is opened and then GC'ed. + lock (this) + { + // Reset cancel state. + _cancelled = false; + _cancellationOwner.Target = null; + + if (_attentionSent) + { + // Make sure we're cleaning up the AttentionAck if Cancel happened before taking the lock. + // We serialize Cancel/CloseSession to prevent a race condition between these two states. + // The problem is that both sending and receiving attentions are time taking + // operations. +#if NETFRAMEWORK && DEBUG + TdsParser.ReliabilitySection tdsReliabilitySection = new TdsParser.ReliabilitySection(); + + System.Runtime.CompilerServices.RuntimeHelpers.PrepareConstrainedRegions(); + try + { + tdsReliabilitySection.Start(); +#endif + Parser.ProcessPendingAck(this); +#if NETFRAMEWORK && DEBUG + } + finally + { + tdsReliabilitySection.Stop(); + } +#endif + } + SetTimeoutStateStopped(); + } + } + internal bool Deactivate() { bool goodForReuse = false; @@ -796,6 +899,11 @@ internal void SetTimeoutMilliseconds(long timeout) } } + internal void StartSession(object cancellationOwner) + { + _cancellationOwner.Target = cancellationOwner; + } + internal void ThrowExceptionAndWarning(bool callerHasConnectionLock = false, bool asyncClose = false) { _parser.ThrowExceptionAndWarning(this, callerHasConnectionLock, asyncClose); From a720efae173ceb064e39fbbcc30463de3b022b08 Mon Sep 17 00:00:00 2001 From: panoskj Date: Sun, 20 Mar 2022 13:01:22 +0200 Subject: [PATCH 18/22] Merging DecrementPendingCallbacks and incrementPendingCallbacks methods of TdsParserStateObject. --- .../Data/SqlClient/TdsParserStateObject.cs | 19 ---------- .../Data/SqlClient/TdsParserStateObject.cs | 37 +++++-------------- .../Data/SqlClient/TdsParserStateObject.cs | 37 +++++++++++++++++++ 3 files changed, 46 insertions(+), 47 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index fb4349fc09..93172d19a4 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -107,25 +107,6 @@ internal abstract void CreatePhysicalSNIHandle( internal abstract uint GenerateSspiClientContext(byte[] receivedBuff, uint receivedLength, ref byte[] sendBuff, ref uint sendLength, byte[][] _sniSpnBuffer); - internal int DecrementPendingCallbacks(bool release) - { - int remaining = Interlocked.Decrement(ref _pendingCallbacks); - SqlClientEventSource.Log.TryAdvancedTraceEvent("TdsParserStateObject.DecrementPendingCallbacks | ADV | State Object Id {0}, after decrementing _pendingCallbacks: {1}", _objectID, _pendingCallbacks); - FreeGcHandle(remaining, release); - // NOTE: TdsParserSessionPool may call DecrementPendingCallbacks on a TdsParserStateObject which is already disposed - // This is not dangerous (since the stateObj is no longer in use), but we need to add a workaround in the assert for it - Debug.Assert((remaining == -1 && SessionHandle.IsNull) || (0 <= remaining && remaining < 3), $"_pendingCallbacks values is invalid after decrementing: {remaining}"); - return remaining; - } - - internal int IncrementPendingCallbacks() - { - int remaining = Interlocked.Increment(ref _pendingCallbacks); - SqlClientEventSource.Log.TryAdvancedTraceEvent("TdsParserStateObject.IncrementPendingCallbacks | ADV | State Object Id {0}, after incrementing _pendingCallbacks: {1}", _objectID, _pendingCallbacks); - Debug.Assert(0 < remaining && remaining <= 3, $"_pendingCallbacks values is invalid after incrementing: {remaining}"); - return remaining; - } - private void SetSnapshottedState(SnapshottedStateFlags flag, bool value) { if (value) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index d4c696b5eb..7768712265 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -166,24 +166,6 @@ internal void CreatePhysicalSNIHandle( ipPreference, cachedDNSInfo, hostNameInCertificate); } - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] - internal int DecrementPendingCallbacks(bool release) - { - int remaining = Interlocked.Decrement(ref _pendingCallbacks); - SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}, after decrementing _pendingCallbacks: {1}", ObjectID, _pendingCallbacks); - - if ((0 == remaining || release) && _gcHandle.IsAllocated) - { - SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}, FREEING HANDLE!", ObjectID); - _gcHandle.Free(); - } - - // NOTE: TdsParserSessionPool may call DecrementPendingCallbacks on a TdsParserStateObject which is already disposed - // This is not dangerous (since the stateObj is no longer in use), but we need to add a workaround in the assert for it - Debug.Assert((remaining == -1 && _sessionHandle == null) || (0 <= remaining && remaining < 3), $"_pendingCallbacks values is invalid after decrementing: {remaining}"); - return remaining; - } - internal void Dispose() { @@ -241,16 +223,6 @@ internal void Dispose() } } - [ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] - internal int IncrementPendingCallbacks() - { - int remaining = Interlocked.Increment(ref _pendingCallbacks); - - SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}, after incrementing _pendingCallbacks: {1}", ObjectID, _pendingCallbacks); - Debug.Assert(0 < remaining && remaining <= 3, $"_pendingCallbacks values is invalid after incrementing: {remaining}"); - return remaining; - } - ///////////////////////////////////////// // Network/Packet Reading & Processing // ///////////////////////////////////////// @@ -318,6 +290,15 @@ private uint SNIPacketGetData(IntPtr packet, byte[] _inBuff, ref uint dataSize) return SNINativeMethodWrapper.SNIPacketGetData(packet, _inBuff, ref dataSize); } + private void FreeGcHandle(int remaining, bool release) + { + if ((0 == remaining || release) && _gcHandle.IsAllocated) + { + SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}, FREEING HANDLE!", ObjectID); + _gcHandle.Free(); + } + } + ///////////////////////////////////////// // Network/Packet Writing & Processing // ///////////////////////////////////////// diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index c1bfddf69f..e914bf30c3 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -837,6 +837,26 @@ internal void DecrementOpenResultCount() HasOpenResult = false; } +#if NETFRAMEWORK + [System.Runtime.ConstrainedExecution.ReliabilityContract( + System.Runtime.ConstrainedExecution.Consistency.WillNotCorruptState, + System.Runtime.ConstrainedExecution.Cer.Success)] +#endif + internal int DecrementPendingCallbacks(bool release) + { + int remaining = Interlocked.Decrement(ref _pendingCallbacks); +#if NETFRAMEWORK + SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}, after decrementing _pendingCallbacks: {1}", ObjectID, _pendingCallbacks); +#else + SqlClientEventSource.Log.TryAdvancedTraceEvent("TdsParserStateObject.DecrementPendingCallbacks | ADV | State Object Id {0}, after decrementing _pendingCallbacks: {1}", _objectID, _pendingCallbacks); +#endif + FreeGcHandle(remaining, release); + // NOTE: TdsParserSessionPool may call DecrementPendingCallbacks on a TdsParserStateObject which is already disposed + // This is not dangerous (since the stateObj is no longer in use), but we need to add a workaround in the assert for it + Debug.Assert((remaining == -1 && SessionHandle.IsNull) || (0 <= remaining && remaining < 3), $"_pendingCallbacks values is invalid after decrementing: {remaining}"); + return remaining; + } + internal void DisposeCounters() { Timer networkPacketTimeout = _networkPacketTimeout; @@ -860,6 +880,23 @@ internal void DisposeCounters() } } +#if NETFRAMEWORK + [System.Runtime.ConstrainedExecution.ReliabilityContract( + System.Runtime.ConstrainedExecution.Consistency.WillNotCorruptState, + System.Runtime.ConstrainedExecution.Cer.Success)] +#endif + internal int IncrementPendingCallbacks() + { + int remaining = Interlocked.Increment(ref _pendingCallbacks); +#if NETFRAMEWORK + SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0}, after incrementing _pendingCallbacks: {1}", ObjectID, _pendingCallbacks); +#else + SqlClientEventSource.Log.TryAdvancedTraceEvent("TdsParserStateObject.IncrementPendingCallbacks | ADV | State Object Id {0}, after incrementing _pendingCallbacks: {1}", _objectID, _pendingCallbacks); +#endif + Debug.Assert(0 < remaining && remaining <= 3, $"_pendingCallbacks values is invalid after incrementing: {remaining}"); + return remaining; + } + internal int IncrementAndObtainOpenResultCount(SqlInternalTransaction transaction) { HasOpenResult = true; From 6a89090dc55a777204c3441d2ddd1e10e6f14b5d Mon Sep 17 00:00:00 2001 From: panoskj Date: Wed, 23 Nov 2022 22:32:07 +0200 Subject: [PATCH 19/22] TdsParserStateObject: Removed unnecessary "#pragma warning disable". Removed unnecessary casts. Removed unnecessary assignment. Added readonly specifier where possible. --- .../Data/SqlClient/TdsParserStateObject.cs | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index e914bf30c3..1c617b3076 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -166,7 +166,7 @@ public TimeoutState(int value) // 3) post session close - no attention is allowed private bool _cancelled; private const int WaitForCancellationLockPollTimeout = 100; - private WeakReference _cancellationOwner = new WeakReference(null); + private readonly WeakReference _cancellationOwner = new WeakReference(null); // Cache the transaction for which this command was executed so upon completion we can // decrement the appropriate result count. @@ -276,11 +276,8 @@ public TimeoutState(int value) internal static bool s_forcePendingReadsToWaitForUser = false; internal TaskCompletionSource _realNetworkPacketTaskSource; - // Field is never assigned to, and will always have its default value -#pragma warning disable 0649 // Set to true to enable checking the call stacks match when packet retry occurs. internal static bool s_checkNetworkPacketRetryStacks = false; -#pragma warning restore 0649 #endif ////////////////// @@ -439,7 +436,7 @@ internal int GetTimeoutRemaining() int remaining; if (0 != _timeoutMilliseconds) { - remaining = (int)Math.Min((long)int.MaxValue, _timeoutMilliseconds); + remaining = (int)Math.Min(int.MaxValue, _timeoutMilliseconds); _timeoutTime = TdsParserStaticMethods.GetTimeout(_timeoutMilliseconds); _timeoutMilliseconds = 0; } @@ -918,7 +915,7 @@ internal int IncrementAndObtainOpenResultCount(SqlInternalTransaction transactio internal void SetTimeoutSeconds(int timeout) { - SetTimeoutMilliseconds((long)timeout * 1000L); + SetTimeoutMilliseconds(timeout * 1000L); } internal void SetTimeoutMilliseconds(long timeout) @@ -1011,8 +1008,8 @@ internal bool TryProcessHeader() { // All read _partialHeaderBytesRead = 0; - _inBytesPacket = ((int)_partialHeaderBuffer[TdsEnums.HEADER_LEN_FIELD_OFFSET] << 8 | - (int)_partialHeaderBuffer[TdsEnums.HEADER_LEN_FIELD_OFFSET + 1]) - _inputHeaderLen; + _inBytesPacket = (_partialHeaderBuffer[TdsEnums.HEADER_LEN_FIELD_OFFSET] << 8 | + _partialHeaderBuffer[TdsEnums.HEADER_LEN_FIELD_OFFSET + 1]) - _inputHeaderLen; _messageStatus = _partialHeaderBuffer[1]; _spid = _partialHeaderBuffer[TdsEnums.SPID_OFFSET] << 8 | @@ -1855,7 +1852,7 @@ internal bool TryReadPlpLength(bool returnPlpNullIfNull, out ulong lengthLeft) } else { - _longlenleft = (ulong)chunklen; + _longlenleft = chunklen; } } @@ -1933,7 +1930,7 @@ internal bool TryReadPlpBytes(ref byte[] buff, int offset, int len, out int tota #endif { // if the buffer is null or the wrong length create one to use - buff = new byte[(int)Math.Min((int)_longlen, len)]; + buff = new byte[(Math.Min((int)_longlen, len))]; } } @@ -2026,16 +2023,15 @@ internal bool TryReadPlpBytes(ref byte[] buff, int offset, int len, out int tota internal bool TrySkipLongBytes(long num) { Debug.Assert(_syncOverAsync || !_asyncReadWithoutSnapshot, "This method is not safe to call when doing sync over async"); - int cbSkip = 0; - + while (num > 0) { - cbSkip = (int)Math.Min((long)int.MaxValue, num); + int cbSkip = (int)Math.Min(int.MaxValue, num); if (!TryReadByteArray(Span.Empty, cbSkip)) { return false; } - num -= (long)cbSkip; + num -= cbSkip; } return true; @@ -2296,7 +2292,7 @@ private bool OnTimeoutCore(int expectedState, int targetState, bool asyncClose = { if (!_attentionSent) { - AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, (byte)0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); + AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, 0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); // Grab a reference to the _networkPacketTaskSource in case it becomes null while we are trying to use it TaskCompletionSource source = _networkPacketTaskSource; @@ -2667,7 +2663,7 @@ private void ReadSniError(TdsParserStateObject stateObj, uint error) { stateObj.SetTimeoutStateStopped(); Debug.Assert(_parser.Connection != null, "SqlConnectionInternalTds handler can not be null at this point."); - AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, (byte)0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); + AddError(new SqlError(TdsEnums.TIMEOUT_EXPIRED, 0x00, TdsEnums.MIN_ERROR_CLASS, _parser.Server, _parser.Connection.TimeoutErrorInternal.GetErrorMessage(), "", 0, TdsEnums.SNI_WAIT_TIMEOUT)); if (!stateObj._attentionSent) { From aadf8e38b7f56ef479117bed93c8f17c7bae22e4 Mon Sep 17 00:00:00 2001 From: panoskj Date: Wed, 23 Nov 2022 23:42:13 +0200 Subject: [PATCH 20/22] TdsParserStateObject: replaced some netfx fields with properties, to expose the same interface as netcore version. This helps merging other files, such as TdsParser. --- .../Microsoft/Data/SqlClient/SqlDataReader.cs | 20 +++---- .../SqlClient/SqlInternalConnectionTds.cs | 6 +- .../src/Microsoft/Data/SqlClient/TdsParser.cs | 46 +++++++-------- .../Data/SqlClient/TdsParserStateObject.cs | 56 ++++++++----------- .../Data/SqlClient/TdsParserSessionPool.cs | 6 +- 5 files changed, 60 insertions(+), 74 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs index c404f3da8d..bd3f711d8b 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs @@ -861,7 +861,7 @@ private bool TryCleanPartialRead() } #if DEBUG - if (_stateObj._pendingData) + if (_stateObj.HasPendingData) { byte token; if (!_stateObj.TryPeekByte(out token)) @@ -1068,7 +1068,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 @@ -1334,7 +1334,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) { @@ -3474,7 +3474,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)) @@ -3564,7 +3564,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 @@ -3621,7 +3621,7 @@ private bool TryHasMoreRows(out bool moreRows) moreRows = false; return false; } - if (_stateObj._pendingData) + if (_stateObj.HasPendingData) { if (!_stateObj.TryPeekByte(out b)) { @@ -3965,7 +3965,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) { @@ -3997,7 +3997,7 @@ private bool TryReadInternal(bool setTimeout, out bool more) } } - if (!_stateObj._pendingData) + if (!_stateObj.HasPendingData) { if (!TryCloseInternal(false /*closeReader*/)) { @@ -4021,7 +4021,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)) { @@ -4062,7 +4062,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)) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs index ee8d211e7b..d844075902 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlInternalConnectionTds.cs @@ -910,11 +910,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(); } @@ -1073,7 +1073,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) { diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs index ca8e2418d4..e9bd78bd00 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -964,7 +964,7 @@ internal TdsParserStateObject GetSession(object owner) { session = _sessionPool.GetSession(owner); - Debug.Assert(!session._pendingData, "pending data on a pooled MARS session"); + Debug.Assert(!session.HasPendingData, "pending data on a pooled MARS session"); SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0} getting session {1} from pool", ObjectID, session.ObjectID); } else @@ -1598,7 +1598,7 @@ internal void Deactivate(bool connectionIsDoomed) if (!connectionIsDoomed && null != _physicalStateObj) { - if (_physicalStateObj._pendingData) + if (_physicalStateObj.HasPendingData) { DrainData(_physicalStateObj); } @@ -2479,7 +2479,7 @@ internal bool TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataRead { if (token == TdsEnums.SQLERROR) { - stateObj._errorTokenReceived = true; // Keep track of the fact error token was received - for Done processing. + stateObj.HasReceivedError = true; // Keep track of the fact error token was received - for Done processing. } SqlError error; @@ -3009,18 +3009,18 @@ internal bool TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataRead break; } - Debug.Assert(stateObj._pendingData || !dataReady, "dataReady is set, but there is no pending data"); + Debug.Assert(stateObj.HasPendingData || !dataReady, "dataReady is set, but there is no pending data"); } // Loop while data pending & runbehavior not return immediately, OR // if in attention case, loop while no more pending data & attention has not yet been // received. - while ((stateObj._pendingData && + while ((stateObj.HasPendingData && (RunBehavior.ReturnImmediately != (RunBehavior.ReturnImmediately & runBehavior))) || - (!stateObj._pendingData && stateObj._attentionSent && !stateObj._attentionReceived)); + (!stateObj.HasPendingData && stateObj._attentionSent && !stateObj.HasReceivedAttention)); #if DEBUG - if ((stateObj._pendingData) && (!dataReady)) + if ((stateObj.HasPendingData) && (!dataReady)) { byte token; if (!stateObj.TryPeekByte(out token)) @@ -3031,7 +3031,7 @@ internal bool TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataRead } #endif - if (!stateObj._pendingData) + if (!stateObj.HasPendingData) { if (null != CurrentTransaction) { @@ -3041,7 +3041,7 @@ internal bool TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataRead // if we recieved an attention (but this thread didn't send it) then // we throw an Operation Cancelled error - if (stateObj._attentionReceived) + if (stateObj.HasReceivedAttention) { // Dev11 #344723: SqlClient stress test suspends System_Data!Tcp::ReadSync via a call to SqlDataReader::Close // Spin until SendAttention has cleared _attentionSending, this prevents a race condition between receiving the attention ACK and setting _attentionSent @@ -3052,7 +3052,7 @@ internal bool TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataRead { // Reset attention state. stateObj._attentionSent = false; - stateObj._attentionReceived = false; + stateObj.HasReceivedAttention = false; if (RunBehavior.Clean != (RunBehavior.Clean & runBehavior) && !stateObj.IsTimeoutStateExpired) { @@ -3518,7 +3518,7 @@ private bool TryProcessDone(SqlCommand cmd, SqlDataReader reader, ref RunBehavio { Debug.Assert(TdsEnums.DONE_MORE != (status & TdsEnums.DONE_MORE), "Not expecting DONE_MORE when receiving DONE_ATTN"); Debug.Assert(stateObj._attentionSent, "Received attention done without sending one!"); - stateObj._attentionReceived = true; + stateObj.HasReceivedAttention = true; Debug.Assert(stateObj._inBytesUsed == stateObj._inBytesRead && stateObj._inBytesPacket == 0, "DONE_ATTN received with more data left on wire"); } if ((null != cmd) && (TdsEnums.DONE_COUNT == (status & TdsEnums.DONE_COUNT))) @@ -3537,13 +3537,13 @@ private bool TryProcessDone(SqlCommand cmd, SqlDataReader reader, ref RunBehavio } // Skip the bogus DONE counts sent by the server - if (stateObj._receivedColMetaData || (curCmd != TdsEnums.SELECT)) + if (stateObj.HasReceivedColumnMetadata || (curCmd != TdsEnums.SELECT)) { cmd.OnStatementCompleted(count); } } - stateObj._receivedColMetaData = false; + stateObj.HasReceivedColumnMetadata = false; // Surface exception for DONE_ERROR in the case we did not receive an error token // in the stream, but an error occurred. In these cases, we throw a general server error. The @@ -3552,7 +3552,7 @@ private bool TryProcessDone(SqlCommand cmd, SqlDataReader reader, ref RunBehavio // the server has reached its max connection limit. Bottom line, we need to throw general // error in the cases where we did not receive a error token along with the DONE_ERROR. if ((TdsEnums.DONE_ERROR == (TdsEnums.DONE_ERROR & status)) && stateObj.ErrorCount == 0 && - stateObj._errorTokenReceived == false && (RunBehavior.Clean != (RunBehavior.Clean & run))) + stateObj.HasReceivedError == false && (RunBehavior.Clean != (RunBehavior.Clean & run))) { stateObj.AddError(new SqlError(0, 0, TdsEnums.MIN_ERROR_CLASS, _server, SQLMessage.SevereError(), "", 0)); @@ -3586,17 +3586,17 @@ private bool TryProcessDone(SqlCommand cmd, SqlDataReader reader, ref RunBehavio // stop if the DONE_MORE bit isn't set (see above for attention handling) if (TdsEnums.DONE_MORE != (status & TdsEnums.DONE_MORE)) { - stateObj._errorTokenReceived = false; + stateObj.HasReceivedError = false; if (stateObj._inBytesUsed >= stateObj._inBytesRead) { - stateObj._pendingData = false; + stateObj.HasPendingData = false; } } // _pendingData set by e.g. 'TdsExecuteSQLBatch' // _hasOpenResult always set to true by 'WriteMarsHeader' // - if (!stateObj._pendingData && stateObj._hasOpenResult) + if (!stateObj.HasPendingData && stateObj.HasOpenResult) { /* Debug.Assert(!((sqlTransaction != null && _distributedTransaction != null) || @@ -5140,7 +5140,7 @@ internal void ThrowUnsupportedCollationEncountered(TdsParserStateObject stateObj { DrainData(stateObj); - stateObj._pendingData = false; + stateObj.HasPendingData = false; } ThrowExceptionAndWarning(stateObj); @@ -5742,7 +5742,7 @@ private bool TryCommonProcessMetaData(TdsParserStateObject stateObj, _SqlMetaDat // We get too many DONE COUNTs from the server, causing too meany StatementCompleted event firings. // We only need to fire this event when we actually have a meta data stream with 0 or more rows. - stateObj._receivedColMetaData = true; + stateObj.HasReceivedColumnMetadata = true; return true; } @@ -9341,7 +9341,7 @@ internal void TdsLogin(SqlLogin rec, _physicalStateObj.WritePacket(TdsEnums.HARDFLUSH); _physicalStateObj.ResetSecurePasswordsInformation(); // Password information is needed only from Login process; done with writing login packet and should clear information - _physicalStateObj._pendingData = true; + _physicalStateObj.HasPendingData = true; _physicalStateObj._messageStatus = 0; // Remvove CTAIP Provider after login record is sent. @@ -9389,7 +9389,7 @@ internal void SendFedAuthToken(SqlFedAuthToken fedAuthToken) _physicalStateObj.WriteByteArray(accessToken, accessToken.Length, 0); _physicalStateObj.WritePacket(TdsEnums.HARDFLUSH); - _physicalStateObj._pendingData = true; + _physicalStateObj.HasPendingData = true; _physicalStateObj._messageStatus = 0; _connHandler._federatedAuthenticationRequested = true; @@ -9701,7 +9701,7 @@ internal SqlDataReader TdsExecuteTransactionManagerRequest( Task writeTask = stateObj.WritePacket(TdsEnums.HARDFLUSH); Debug.Assert(writeTask == null, "Writes should not pend when writing sync"); - stateObj._pendingData = true; + stateObj.HasPendingData = true; stateObj._messageStatus = 0; SqlDataReader dtcReader = null; @@ -11223,7 +11223,7 @@ internal Task WriteBulkCopyDone(TdsParserStateObject stateObj) WriteShort(0, stateObj); WriteInt(0, stateObj); - stateObj._pendingData = true; + stateObj.HasPendingData = true; stateObj._messageStatus = 0; return stateObj.WritePacket(TdsEnums.HARDFLUSH); } diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 7768712265..218b226ddc 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -35,10 +35,7 @@ internal partial class TdsParserStateObject private bool TransparentNetworkIPResolution => _parser.Connection.ConnectionOptions.TransparentNetworkIPResolution; - internal bool _pendingData = false; - internal bool _errorTokenReceived = false; // Keep track of whether an error was received for the result. - // This is reset upon each done token - there can be - // SNI variables // multiple resultsets in one batch. + // SNI variables private SNIPacket _sniPacket = null; // Will have to re-vamp this for MARS internal SNIPacket _sniAsyncAttnPacket = null; // Packet to use to send Attn private readonly WritePacketCache _writePacketCache = new WritePacketCache(); // Store write packets that are ready to be re-used @@ -47,19 +44,12 @@ internal partial class TdsParserStateObject // Async variables private GCHandle _gcHandle; // keeps this object alive until we're closed. - // Timeout variables - internal bool _attentionReceived = false; // NOTE: Received is not volatile as it is only ever accessed\modified by TryRun its callees (i.e. single threaded access) - - internal bool _hasOpenResult = false; - // Used for blanking out password in trace. internal int _tracePasswordOffset = 0; internal int _tracePasswordLength = 0; internal int _traceChangePasswordOffset = 0; internal int _traceChangePasswordLength = 0; - internal bool _receivedColMetaData; // Used to keep track of when to fire StatementCompleted event. - //////////////// // Properties // //////////////// @@ -71,18 +61,16 @@ internal SNIHandle Handle } } - internal bool HasOpenResult - { - get => _hasOpenResult; - set => _hasOpenResult = value; - } - - internal bool HasPendingData - { - get => _pendingData; - set => _pendingData = value; - } + internal bool HasOpenResult { get; set; } + internal bool HasPendingData { get; set; } + internal bool HasReceivedError { get; set; } // Keep track of whether an error was received for the result. + // This is reset upon each done token - there can be + // multiple resultsets in one batch. + internal bool HasReceivedAttention { get; set; } // NOTE: Received is not volatile as it is only ever accessed\modified by TryRun its callees (i.e. single threaded access) + + internal bool HasReceivedColumnMetadata { get; set; } // Used to keep track of when to fire StatementCompleted event. + internal uint Status { get @@ -1160,7 +1148,7 @@ internal void AssertStateIsClean() Debug.Assert(_asyncWriteCount == 0, "StateObj still has outstanding async writes"); Debug.Assert(_delayedWriteAsyncCallbackException == null, "StateObj has an unobserved exceptions from an async write"); // Attention\Cancellation\Timeouts - Debug.Assert(!_attentionReceived && !_attentionSent && !_attentionSending, $"StateObj is still dealing with attention: Sent: {_attentionSent}, Received: {_attentionReceived}, Sending: {_attentionSending}"); + Debug.Assert(!HasReceivedAttention && !_attentionSent && !_attentionSending, $"StateObj is still dealing with attention: Sent: {_attentionSent}, Received: {HasReceivedAttention}, Sending: {_attentionSending}"); Debug.Assert(!_cancelled, "StateObj still has cancellation set"); Debug.Assert(_timeoutState == TimeoutState.Stopped, "StateObj still has internal timeout set"); // Errors and Warnings @@ -1375,8 +1363,8 @@ internal void Snap() _snapshotInBuffCurrent = 0; _snapshotInBytesUsed = _stateObj._inBytesUsed; _snapshotInBytesPacket = _stateObj._inBytesPacket; - _snapshotPendingData = _stateObj._pendingData; - _snapshotErrorTokenReceived = _stateObj._errorTokenReceived; + _snapshotPendingData = _stateObj.HasPendingData; + _snapshotErrorTokenReceived = _stateObj.HasReceivedError; _snapshotMessageStatus = _stateObj._messageStatus; // _nullBitmapInfo must be cloned before it is updated _snapshotNullBitmapInfo = _stateObj._nullBitmapInfo; @@ -1385,9 +1373,9 @@ internal void Snap() _snapshotCleanupMetaData = _stateObj._cleanupMetaData; // _cleanupAltMetaDataSetArray must be cloned bofore it is updated _snapshotCleanupAltMetaDataSetArray = _stateObj._cleanupAltMetaDataSetArray; - _snapshotHasOpenResult = _stateObj._hasOpenResult; - _snapshotReceivedColumnMetadata = _stateObj._receivedColMetaData; - _snapshotAttentionReceived = _stateObj._attentionReceived; + _snapshotHasOpenResult = _stateObj.HasOpenResult; + _snapshotReceivedColumnMetadata = _stateObj.HasReceivedColumnMetadata; + _snapshotAttentionReceived = _stateObj.HasReceivedAttention; #if DEBUG _rollingPend = 0; _rollingPendCount = 0; @@ -1408,26 +1396,26 @@ internal void ResetSnapshotState() _stateObj._inBytesUsed = _snapshotInBytesUsed; _stateObj._inBytesPacket = _snapshotInBytesPacket; - _stateObj._pendingData = _snapshotPendingData; - _stateObj._errorTokenReceived = _snapshotErrorTokenReceived; + _stateObj.HasPendingData = _snapshotPendingData; + _stateObj.HasReceivedError = _snapshotErrorTokenReceived; _stateObj._messageStatus = _snapshotMessageStatus; _stateObj._nullBitmapInfo = _snapshotNullBitmapInfo; _stateObj._cleanupMetaData = _snapshotCleanupMetaData; _stateObj._cleanupAltMetaDataSetArray = _snapshotCleanupAltMetaDataSetArray; // Make sure to go through the appropriate increment/decrement methods if changing HasOpenResult - if (!_stateObj._hasOpenResult && _snapshotHasOpenResult) + if (!_stateObj.HasOpenResult && _snapshotHasOpenResult) { _stateObj.IncrementAndObtainOpenResultCount(_stateObj._executedUnderTransaction); } - else if (_stateObj._hasOpenResult && !_snapshotHasOpenResult) + else if (_stateObj.HasOpenResult && !_snapshotHasOpenResult) { _stateObj.DecrementOpenResultCount(); } //else _stateObj._hasOpenResult is already == _snapshotHasOpenResult - _stateObj._receivedColMetaData = _snapshotReceivedColumnMetadata; - _stateObj._attentionReceived = _snapshotAttentionReceived; + _stateObj.HasReceivedColumnMetadata = _snapshotReceivedColumnMetadata; + _stateObj.HasReceivedAttention = _snapshotAttentionReceived; // Reset partially read state (these only need to be maintained if doing async without snapshot) _stateObj._bTmpRead = 0; diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserSessionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserSessionPool.cs index ed53a831c4..2bf48344a4 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserSessionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserSessionPool.cs @@ -181,11 +181,9 @@ internal void PutSession(TdsParserStateObject session) { // Session is good to re-use and our cache has space SqlClientEventSource.Log.TryAdvancedTraceEvent(" {0} keeping session {1} cachedCount={2}", ObjectID, session.ObjectID, _cachedCount); -#if NETFRAMEWORK - Debug.Assert(!session._pendingData, "pending data on a pooled session?"); -#else + Debug.Assert(!session.HasPendingData, "pending data on a pooled session?"); -#endif + _freeStateObjects[_freeStateObjectCount] = session; _freeStateObjectCount++; } From bc0ceb2c4ed38b371ae5e341c174aa4bc2c679ae Mon Sep 17 00:00:00 2001 From: panoskj Date: Wed, 23 Nov 2022 23:53:00 +0200 Subject: [PATCH 21/22] TdsParserStateObject: removed consecutive empty lines. --- .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 2 -- .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 1 - .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 4 ---- 3 files changed, 7 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 93172d19a4..08a0087aec 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -28,7 +28,6 @@ internal abstract partial class TdsParserStateObject //////////////// internal abstract uint DisableSsl(); - internal abstract uint EnableMars(ref uint info); internal abstract uint Status @@ -776,7 +775,6 @@ private void AssertValidState() Debug.Assert(_inBytesPacket >= 0, "Packet must not be negative"); } - ////////////////////////////////////////////// // Errors and Warnings // ////////////////////////////////////////////// diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 218b226ddc..f06d6f2433 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -903,7 +903,6 @@ void AssertValidState() Debug.Assert(_inBytesPacket >= 0, "Packet must not be negative"); } - ////////////////////////////////////////////// // Errors and Warnings // ////////////////////////////////////////////// diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 1c617b3076..2d8643202d 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -577,7 +577,6 @@ internal bool IsGuaranteedNull(int columnOrdinal) } } - ///////////////////// // General methods // ///////////////////// @@ -2412,7 +2411,6 @@ internal void ReadSni(TaskCompletionSource completion) } #endif - PacketHandle readPacket = default; uint error = 0; @@ -2443,7 +2441,6 @@ internal void ReadSni(TaskCompletionSource completion) Timeout.Infinite ); - // -1 == Infinite // 0 == Already timed out (NOTE: To simulate the same behavior as sync we will only timeout on 0 if we receive an IO Pending from SNI) // >0 == Actual timeout remaining @@ -3067,7 +3064,6 @@ public void WriteAsyncCallback(IntPtr key, PacketHandle packet, uint sniError) } } - ///////////////////////////////////////// // Network/Packet Writing & Processing // ///////////////////////////////////////// From 98bd515e20c33de8833b1d542501859be90d440a Mon Sep 17 00:00:00 2001 From: panoskj Date: Thu, 24 Nov 2022 00:17:05 +0200 Subject: [PATCH 22/22] TdsParserStateObject: Removed trailing whitespaces. Removed unnecessary assignments. Used discard to ignore out parameters, instead of an unused variable. --- .../Data/SqlClient/TdsParserStateObject.cs | 3 +-- .../Data/SqlClient/TdsParserStateObject.cs | 12 ++++++------ .../Data/SqlClient/TdsParserStateObject.cs | 14 +++++++------- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 08a0087aec..bfe1eea700 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -636,9 +636,8 @@ internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = fa return; } - uint sniError; _parser._asyncWrite = false; // stop async write - SNIWritePacket(attnPacket, out sniError, canAccumulate: false, callerHasConnectionLock: false, asyncClose); + SNIWritePacket(attnPacket, out _, canAccumulate: false, callerHasConnectionLock: false, asyncClose); SqlClientEventSource.Log.TryTraceEvent("TdsParserStateObject.SendAttention | Info | State Object Id {0}, Sent Attention.", _objectID); } finally diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index f06d6f2433..86e8c73db7 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -35,7 +35,7 @@ internal partial class TdsParserStateObject private bool TransparentNetworkIPResolution => _parser.Connection.ConnectionOptions.TransparentNetworkIPResolution; - // SNI variables + // SNI variables private SNIPacket _sniPacket = null; // Will have to re-vamp this for MARS internal SNIPacket _sniAsyncAttnPacket = null; // Packet to use to send Attn private readonly WritePacketCache _writePacketCache = new WritePacketCache(); // Store write packets that are ready to be re-used @@ -274,7 +274,7 @@ private uint CheckConnection() } private uint SNIPacketGetData(IntPtr packet, byte[] _inBuff, ref uint dataSize) - { + { return SNINativeMethodWrapper.SNIPacketGetData(packet, _inBuff, ref dataSize); } @@ -562,7 +562,7 @@ private Task SNIWritePacket(SNIHandle handle, SNIPacket packet, out uint sniErro #if DEBUG else if (!sync && !canAccumulate && SqlCommand.DebugForceAsyncWriteDelay > 0) { - // Executed synchronously - callback will not be called + // Executed synchronously - callback will not be called TaskCompletionSource completion = new TaskCompletionSource(); uint error = sniError; new Timer(obj => @@ -623,7 +623,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) @@ -672,7 +672,7 @@ internal void SendAttention(bool mustTakeWriteLock = false, bool asyncClose = fa } uint sniError; - _parser._asyncWrite = false; // stop async write + _parser._asyncWrite = false; // stop async write SNIWritePacket(Handle, attnPacket, out sniError, canAccumulate: false, callerHasConnectionLock: false, asyncClose); SqlClientEventSource.Log.TryTraceEvent(" Send Attention ASync.", "Info"); } @@ -1339,7 +1339,7 @@ internal void CheckStack(string trace) Debug.Assert(_stateObj._permitReplayStackTraceToDiffer || prev.Stack.ToString() == trace.ToString(), "The stack trace on subsequent replays should be the same"); } } -#endif +#endif internal bool Replay() { diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 2d8643202d..441b9bda11 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -139,7 +139,7 @@ public TimeoutState(int value) internal volatile bool _attentionSending; private readonly TimerCallback _onTimeoutAsync; - // Below 2 properties are used to enforce timeout delays in code to + // Below 2 properties are used to enforce timeout delays in code to // reproduce issues related to theadpool starvation and timeout delay. // It should always be set to false by default, and only be enabled during testing. internal bool _enforceTimeoutDelay = false; @@ -1480,7 +1480,7 @@ internal bool TryReadInt64(out long value) // If the long isn't fully in the buffer, or if it isn't fully in the packet, // then use ReadByteArray since the logic is there to take care of that. - int bytesRead = 0; + int bytesRead; if (!TryReadByteArray(_bTmp.AsSpan(_bTmpRead), 8 - _bTmpRead, out bytesRead)) { Debug.Assert(_bTmpRead + bytesRead <= 8, "Read more data than required"); @@ -1562,7 +1562,7 @@ internal bool TryReadUInt32(out uint value) // If the int isn't fully in the buffer, or if it isn't fully in the packet, // then use ReadByteArray since the logic is there to take care of that. - int bytesRead = 0; + int bytesRead; if (!TryReadByteArray(_bTmp.AsSpan(_bTmpRead), 4 - _bTmpRead, out bytesRead)) { Debug.Assert(_bTmpRead + bytesRead <= 4, "Read more data than required"); @@ -2255,7 +2255,7 @@ private void OnTimeoutAsync(object state) TimeoutState timeoutState = (TimeoutState)state; if (timeoutState.IdentityValue == _timeoutIdentityValue) { - // the return value is not useful here because no choice is going to be made using it + // the return value is not useful here because no choice is going to be made using it // we only want to make this call to set the state knowing that it will be seen later OnTimeoutCore(TimeoutState.Running, TimeoutState.ExpiredAsync); } @@ -2421,7 +2421,7 @@ internal void ReadSni(TaskCompletionSource completion) { Debug.Assert(completion != null, "Async on but null asyncResult passed"); - // if the state is currently stopped then change it to running and allocate a new identity value from + // if the state is currently stopped then change it to running and allocate a new identity value from // the identity source. The identity value is used to correlate timer callback events to the currently // running timeout and prevents a late timer callback affecting a result it does not relate to int previousTimeoutState = Interlocked.CompareExchange(ref _timeoutState, TimeoutState.Running, TimeoutState.Stopped); @@ -2871,7 +2871,7 @@ 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); @@ -2967,7 +2967,7 @@ private void ReadAsyncCallbackCaptureException(TaskCompletionSource sour Debug.Assert(_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed || _parser.Connection.IsConnectionDoomed, "Failed to capture exception while the connection was still healthy"); // The safest thing to do is to ensure that the connection is broken and attempt to cancel the task - // This must be done from another thread to not block the callback thread + // This must be done from another thread to not block the callback thread Task.Factory.StartNew(() => { _parser.State = TdsParserState.Broken;