From 82b72bd0c36d18c533551ed85039b9958baab85d Mon Sep 17 00:00:00 2001 From: panoskj Date: Wed, 17 Jan 2024 22:28:13 +0200 Subject: [PATCH 1/3] ValidateSNIConnection now shares the same code in netfx and netcore. --- .../Data/SqlClient/TdsParserStateObject.netfx.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 5d4a332665..2a53273599 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -293,7 +293,12 @@ internal PacketHandle ReadAsync(SessionHandle handle, out uint error) return readPacket; } - internal uint CheckConnection() => SNINativeMethodWrapper.SNICheckConnection(Handle); + /// + /// Note is not supposed to be null when this method is called and the native implementation doesn't expect null values here. + /// The handle will not be null unless this instance has been disposed. + /// We could probably throw an exception instead of returning success (like managed netcore version does). + /// + internal uint CheckConnection() => Handle == null ? TdsEnums.SNI_SUCCESS : SNINativeMethodWrapper.SNICheckConnection(Handle); internal void ReleasePacket(PacketHandle syncReadPacket) => SNINativeMethodWrapper.SNIPacketRelease(syncReadPacket); @@ -410,11 +415,7 @@ internal bool ValidateSNIConnection() try { Interlocked.Increment(ref _readingCount); - SNIHandle handle = Handle; - if (handle != null) - { - error = SNINativeMethodWrapper.SNICheckConnection(handle); - } + error = CheckConnection(); } finally { From 09d29f8c178ec6a337832482ec06cfe444fa1ab3 Mon Sep 17 00:00:00 2001 From: panoskj Date: Wed, 17 Jan 2024 22:32:42 +0200 Subject: [PATCH 2/3] Merge ValidateSNIConnection. --- .../SqlClient/TdsParserStateObject.netcore.cs | 32 ------------------- .../SqlClient/TdsParserStateObject.netfx.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.netcore.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs index b1bb4065c8..28b159c5e7 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserStateObject.netcore.cs @@ -262,38 +262,6 @@ internal void StartSession(object cancellationOwner) _cancellationOwner.Target = cancellationOwner; } - /// - /// 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.netfx.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs index 2a53273599..e7e1ac9453 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParserStateObject.netfx.cs @@ -392,38 +392,6 @@ internal void StartSession(int objectID) _allowObjectID = objectID; } - /// - /// 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/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index 17729a4cc9..ba18456b6a 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -2477,6 +2477,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 3354c9ca4283bbb97a36c8a1d429fc3c2e51fe5b Mon Sep 17 00:00:00 2001 From: panoskj Date: Thu, 18 Jan 2024 09:02:46 +0200 Subject: [PATCH 3/3] Refactoring of ValidateSNIConnection. Makes it clear that in case of an exception: 1) there is no returned value and 2) the "error" variable is not used - there is no need to initialize it with SNI_SUCCESS. --- .../src/Microsoft/Data/SqlClient/TdsParserStateObject.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 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 ba18456b6a..2d7bfed3a9 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -2495,18 +2495,17 @@ internal bool ValidateSNIConnection() return true; } - uint error = TdsEnums.SNI_SUCCESS; SniContext = SniContext.Snix_Connect; try { Interlocked.Increment(ref _readingCount); - error = CheckConnection(); + uint error = CheckConnection(); + return (error == TdsEnums.SNI_SUCCESS) || (error == TdsEnums.SNI_WAIT_TIMEOUT); } finally { Interlocked.Decrement(ref _readingCount); } - return (error == TdsEnums.SNI_SUCCESS) || (error == TdsEnums.SNI_WAIT_TIMEOUT); } /*