From 62273ed204279e554413217e94fa72d87f27c7cc Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Fri, 17 Nov 2023 21:59:57 +0000 Subject: [PATCH 1/5] cache all appcontext switches --- .../Data/SqlClient/LocalAppContextSwitches.cs | 42 ++++++++++++++++--- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs index 3872b215ae..d8c51f8a1c 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs @@ -15,8 +15,8 @@ internal static partial class LocalAppContextSwitches private static bool? s_legacyRowVersionNullBehavior; private static bool? s_suppressInsecureTLSWarning; - private static bool s_makeReadAsyncBlocking; - private static bool s_useMinimumLoginTimeout; + private static bool? s_makeReadAsyncBlocking; + private static bool? s_useMinimumLoginTimeout; #if !NETFRAMEWORK static LocalAppContextSwitches() @@ -36,7 +36,7 @@ static LocalAppContextSwitches() #if NETFRAMEWORK internal const string DisableTNIRByDefaultString = @"Switch.Microsoft.Data.SqlClient.DisableTNIRByDefaultInConnectionString"; - private static bool s_disableTNIRByDefault; + private static bool? s_disableTNIRByDefault; /// /// Transparent Network IP Resolution (TNIR) is a revision of the existing MultiSubnetFailover feature. @@ -54,7 +54,17 @@ static LocalAppContextSwitches() /// This app context switch defaults to 'false'. /// public static bool DisableTNIRByDefault - => AppContext.TryGetSwitch(DisableTNIRByDefaultString, out s_disableTNIRByDefault) && s_disableTNIRByDefault; + { + get + { + if (s_disableTNIRByDefault is null) + { + bool result = AppContext.TryGetSwitch(DisableTNIRByDefaultString, out bool returnedValue) && returnedValue; + s_disableTNIRByDefault = result; + } + return s_disableTNIRByDefault.Value; + } + } #endif /// @@ -101,7 +111,17 @@ public static bool LegacyRowVersionNullBehavior /// This app context switch defaults to 'false'. /// public static bool MakeReadAsyncBlocking - => AppContext.TryGetSwitch(MakeReadAsyncBlockingString, out s_makeReadAsyncBlocking) && s_makeReadAsyncBlocking; + { + get + { + if (s_makeReadAsyncBlocking is null) + { + bool result = AppContext.TryGetSwitch(MakeReadAsyncBlockingString, out bool returnedValue) && returnedValue; + s_makeReadAsyncBlocking = result; + } + return s_makeReadAsyncBlocking.Value; + } + } /// /// Specifies minimum login timeout to be set to 1 second instead of 0 seconds, @@ -109,6 +129,16 @@ public static bool MakeReadAsyncBlocking /// This app context switch defaults to 'true'. /// public static bool UseMinimumLoginTimeout - => !AppContext.TryGetSwitch(UseMinimumLoginTimeoutString, out s_useMinimumLoginTimeout) || s_useMinimumLoginTimeout; + { + get + { + if (s_useMinimumLoginTimeout is null) + { + bool result = AppContext.TryGetSwitch(UseMinimumLoginTimeoutString, out bool returnedValue) && returnedValue; + s_useMinimumLoginTimeout = result; + } + return s_useMinimumLoginTimeout.Value; + } + } } } From a8a0ec35ae8e81d9b63764186cce7835abf53d9d Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Sat, 18 Nov 2023 00:05:47 +0000 Subject: [PATCH 2/5] use atomically updated byte enum instead of Nullable --- .../Data/SqlClient/LocalAppContextSwitches.cs | 89 +++++++++++++------ 1 file changed, 62 insertions(+), 27 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs index d8c51f8a1c..fb7c31798a 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs @@ -8,15 +8,22 @@ namespace Microsoft.Data.SqlClient { internal static partial class LocalAppContextSwitches { + private enum Tristate : byte + { + NotInitialized = 0, + False = 1, + True = 2 + } + internal const string MakeReadAsyncBlockingString = @"Switch.Microsoft.Data.SqlClient.MakeReadAsyncBlocking"; internal const string LegacyRowVersionNullString = @"Switch.Microsoft.Data.SqlClient.LegacyRowVersionNullBehavior"; internal const string SuppressInsecureTLSWarningString = @"Switch.Microsoft.Data.SqlClient.SuppressInsecureTLSWarning"; internal const string UseMinimumLoginTimeoutString = @"Switch.Microsoft.Data.SqlClient.UseOneSecFloorInTimeoutCalculationDuringLogin"; - private static bool? s_legacyRowVersionNullBehavior; - private static bool? s_suppressInsecureTLSWarning; - private static bool? s_makeReadAsyncBlocking; - private static bool? s_useMinimumLoginTimeout; + private static Tristate s_legacyRowVersionNullBehavior; + private static Tristate s_suppressInsecureTLSWarning; + private static Tristate s_makeReadAsyncBlocking; + private static Tristate s_useMinimumLoginTimeout; #if !NETFRAMEWORK static LocalAppContextSwitches() @@ -36,7 +43,7 @@ static LocalAppContextSwitches() #if NETFRAMEWORK internal const string DisableTNIRByDefaultString = @"Switch.Microsoft.Data.SqlClient.DisableTNIRByDefaultInConnectionString"; - private static bool? s_disableTNIRByDefault; + private static Tristate s_disableTNIRByDefault; /// /// Transparent Network IP Resolution (TNIR) is a revision of the existing MultiSubnetFailover feature. @@ -57,12 +64,18 @@ public static bool DisableTNIRByDefault { get { - if (s_disableTNIRByDefault is null) + if (s_disableTNIRByDefault == Tristate.NotInitialized) { - bool result = AppContext.TryGetSwitch(DisableTNIRByDefaultString, out bool returnedValue) && returnedValue; - s_disableTNIRByDefault = result; + if (AppContext.TryGetSwitch(DisableTNIRByDefaultString, out bool returnedValue) && returnedValue) + { + s_disableTNIRByDefault = Tristate.True; + } + else + { + s_disableTNIRByDefault = Tristate.False; + } } - return s_disableTNIRByDefault.Value; + return s_disableTNIRByDefault == Tristate.True; } } #endif @@ -76,13 +89,18 @@ public static bool SuppressInsecureTLSWarning { get { - if (s_suppressInsecureTLSWarning is null) + if (s_suppressInsecureTLSWarning == Tristate.NotInitialized) { - bool result; - result = AppContext.TryGetSwitch(SuppressInsecureTLSWarningString, out result) && result; - s_suppressInsecureTLSWarning = result; + if (AppContext.TryGetSwitch(SuppressInsecureTLSWarningString, out bool returnedValue) && returnedValue) + { + s_suppressInsecureTLSWarning = Tristate.True; + } + else + { + s_suppressInsecureTLSWarning = Tristate.False; + } } - return s_suppressInsecureTLSWarning.Value; + return s_suppressInsecureTLSWarning == Tristate.True; } } @@ -96,13 +114,18 @@ public static bool LegacyRowVersionNullBehavior { get { - if (s_legacyRowVersionNullBehavior is null) + if (s_legacyRowVersionNullBehavior == Tristate.NotInitialized) { - bool result; - result = AppContext.TryGetSwitch(LegacyRowVersionNullString, out result) && result; - s_legacyRowVersionNullBehavior = result; + if (AppContext.TryGetSwitch(LegacyRowVersionNullString, out bool returnedValue) && returnedValue) + { + s_legacyRowVersionNullBehavior = Tristate.True; + } + else + { + s_legacyRowVersionNullBehavior = Tristate.False; + } } - return s_legacyRowVersionNullBehavior.Value; + return s_legacyRowVersionNullBehavior == Tristate.True; } } @@ -114,12 +137,18 @@ public static bool MakeReadAsyncBlocking { get { - if (s_makeReadAsyncBlocking is null) + if (s_makeReadAsyncBlocking == Tristate.NotInitialized) { - bool result = AppContext.TryGetSwitch(MakeReadAsyncBlockingString, out bool returnedValue) && returnedValue; - s_makeReadAsyncBlocking = result; + if (AppContext.TryGetSwitch(MakeReadAsyncBlockingString, out bool returnedValue) && returnedValue) + { + s_makeReadAsyncBlocking = Tristate.True; + } + else + { + s_makeReadAsyncBlocking = Tristate.False; + } } - return s_makeReadAsyncBlocking.Value; + return s_makeReadAsyncBlocking == Tristate.True; } } @@ -132,12 +161,18 @@ public static bool UseMinimumLoginTimeout { get { - if (s_useMinimumLoginTimeout is null) + if (s_useMinimumLoginTimeout == Tristate.NotInitialized) { - bool result = AppContext.TryGetSwitch(UseMinimumLoginTimeoutString, out bool returnedValue) && returnedValue; - s_useMinimumLoginTimeout = result; + if (AppContext.TryGetSwitch(UseMinimumLoginTimeoutString, out bool returnedValue) && returnedValue) + { + s_useMinimumLoginTimeout = Tristate.True; + } + else + { + s_useMinimumLoginTimeout = Tristate.False; + } } - return s_useMinimumLoginTimeout.Value; + return s_useMinimumLoginTimeout == Tristate.True; } } } From fd626946c58e780360f41460d6ec9e49054e0548 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Sat, 18 Nov 2023 01:37:08 +0000 Subject: [PATCH 3/5] fix tests and add note about reflection --- .../Data/SqlClient/LocalAppContextSwitches.cs | 1 + .../SQL/DataReaderTest/DataReaderTest.cs | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs index fb7c31798a..8491b343a5 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs @@ -20,6 +20,7 @@ private enum Tristate : byte internal const string SuppressInsecureTLSWarningString = @"Switch.Microsoft.Data.SqlClient.SuppressInsecureTLSWarning"; internal const string UseMinimumLoginTimeoutString = @"Switch.Microsoft.Data.SqlClient.UseOneSecFloorInTimeoutCalculationDuringLogin"; + // this field is accesses through reflection in tests and should be be renamed or have the type changed without refactoring NullRow related tests private static Tristate s_legacyRowVersionNullBehavior; private static Tristate s_suppressInsecureTLSWarning; private static Tristate s_makeReadAsyncBlocking; diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs index 6929d35ee7..d00ea1d226 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs @@ -17,6 +17,14 @@ public static class DataReaderTest { private static readonly object s_rowVersionLock = new(); + // this enum must mirror the definition in LocalAppContextSwitches + private enum Tristate : byte + { + NotInitialized = 0, + False = 1, + True = 2 + } + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] public static void LoadReaderIntoDataTableToTestGetSchemaTable() { @@ -261,7 +269,7 @@ public static void CheckNullRowVersionIsBDNull() { lock (s_rowVersionLock) { - bool? originalValue = SetLegacyRowVersionNullBehavior(false); + Tristate originalValue = SetLegacyRowVersionNullBehavior(Tristate.False); try { using SqlConnection con = new(DataTestUtility.TCPConnectionString); @@ -298,7 +306,7 @@ public static void CheckLegacyNullRowVersionIsEmptyArray() { lock (s_rowVersionLock) { - bool? originalValue = SetLegacyRowVersionNullBehavior(true); + Tristate originalValue = SetLegacyRowVersionNullBehavior(Tristate.True); try { using SqlConnection con = new(DataTestUtility.TCPConnectionString); @@ -323,11 +331,11 @@ public static void CheckLegacyNullRowVersionIsEmptyArray() } } - private static bool? SetLegacyRowVersionNullBehavior(bool? value) + private static Tristate SetLegacyRowVersionNullBehavior(Tristate value) { Type switchesType = typeof(SqlCommand).Assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches"); FieldInfo switchField = switchesType.GetField("s_legacyRowVersionNullBehavior", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static); - bool? originalValue = (bool?)switchField.GetValue(null); + Tristate originalValue = (Tristate)switchField.GetValue(null); switchField.SetValue(null, value); return originalValue; } From b776663b7f8114057b6bbf9b1c1516d0bdf17e5f Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Sat, 18 Nov 2023 01:37:08 +0000 Subject: [PATCH 4/5] fix tests and add note about reflection --- .../Data/SqlClient/LocalAppContextSwitches.cs | 1 + .../SQL/DataReaderTest/DataReaderTest.cs | 16 ++++++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs index fb7c31798a..792ed84517 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs @@ -20,6 +20,7 @@ private enum Tristate : byte internal const string SuppressInsecureTLSWarningString = @"Switch.Microsoft.Data.SqlClient.SuppressInsecureTLSWarning"; internal const string UseMinimumLoginTimeoutString = @"Switch.Microsoft.Data.SqlClient.UseOneSecFloorInTimeoutCalculationDuringLogin"; + // this field is accessed through reflection in tests and should be be renamed or have the type changed without refactoring NullRow related tests private static Tristate s_legacyRowVersionNullBehavior; private static Tristate s_suppressInsecureTLSWarning; private static Tristate s_makeReadAsyncBlocking; diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs index 6929d35ee7..d00ea1d226 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs @@ -17,6 +17,14 @@ public static class DataReaderTest { private static readonly object s_rowVersionLock = new(); + // this enum must mirror the definition in LocalAppContextSwitches + private enum Tristate : byte + { + NotInitialized = 0, + False = 1, + True = 2 + } + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] public static void LoadReaderIntoDataTableToTestGetSchemaTable() { @@ -261,7 +269,7 @@ public static void CheckNullRowVersionIsBDNull() { lock (s_rowVersionLock) { - bool? originalValue = SetLegacyRowVersionNullBehavior(false); + Tristate originalValue = SetLegacyRowVersionNullBehavior(Tristate.False); try { using SqlConnection con = new(DataTestUtility.TCPConnectionString); @@ -298,7 +306,7 @@ public static void CheckLegacyNullRowVersionIsEmptyArray() { lock (s_rowVersionLock) { - bool? originalValue = SetLegacyRowVersionNullBehavior(true); + Tristate originalValue = SetLegacyRowVersionNullBehavior(Tristate.True); try { using SqlConnection con = new(DataTestUtility.TCPConnectionString); @@ -323,11 +331,11 @@ public static void CheckLegacyNullRowVersionIsEmptyArray() } } - private static bool? SetLegacyRowVersionNullBehavior(bool? value) + private static Tristate SetLegacyRowVersionNullBehavior(Tristate value) { Type switchesType = typeof(SqlCommand).Assembly.GetType("Microsoft.Data.SqlClient.LocalAppContextSwitches"); FieldInfo switchField = switchesType.GetField("s_legacyRowVersionNullBehavior", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Static); - bool? originalValue = (bool?)switchField.GetValue(null); + Tristate originalValue = (Tristate)switchField.GetValue(null); switchField.SetValue(null, value); return originalValue; } From f1243c0aac36451d9bea53008accb4eaa66b38b9 Mon Sep 17 00:00:00 2001 From: Wraith2 Date: Sat, 18 Nov 2023 01:42:52 +0000 Subject: [PATCH 5/5] more typos. note, do not code at 2am. --- .../src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs index 792ed84517..ed2c4aedf7 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/LocalAppContextSwitches.cs @@ -20,7 +20,7 @@ private enum Tristate : byte internal const string SuppressInsecureTLSWarningString = @"Switch.Microsoft.Data.SqlClient.SuppressInsecureTLSWarning"; internal const string UseMinimumLoginTimeoutString = @"Switch.Microsoft.Data.SqlClient.UseOneSecFloorInTimeoutCalculationDuringLogin"; - // this field is accessed through reflection in tests and should be be renamed or have the type changed without refactoring NullRow related tests + // this field is accessed through reflection in tests and should not be renamed or have the type changed without refactoring NullRow related tests private static Tristate s_legacyRowVersionNullBehavior; private static Tristate s_suppressInsecureTLSWarning; private static Tristate s_makeReadAsyncBlocking;