Merge pull request #9342 from rokonec/rokonec/BinFmt-disabled-by-default

Experimental insertion for #9318
dotnet · Oct 18, 2023 · 361df61 · 361df61
2 parents 867e260 + 7679120
commit 361df61
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 25 deletions.
diff --git a/src/Build.UnitTests/BackEnd/BuildManager_Logging_Tests.cs b/src/Build.UnitTests/BackEnd/BuildManager_Logging_Tests.cs
@@ -82,17 +82,17 @@ public BuildManager_Logging_Tests(ITestOutputHelper output)
         [InlineData("1", true)]
         // [InlineData("0", true)] <-- explicitly opting out on core will lead to node crash (as documented)
         [InlineData(null, true)]
-        public void Build_WithCustomBuildArgs_NetCore(string envVariableValue, bool isWarningExpected)
-            => TestCustomEventWarning<BuildErrorEventArgs>(envVariableValue, isWarningExpected);
+        public void Build_WithCustomBuildArgs_NetCore(string envVariableValue, bool isErrorExpected)
+            => TestCustomEvent<BuildErrorEventArgs>(envVariableValue, isErrorExpected);
 
         [WindowsFullFrameworkOnlyTheory]
         [InlineData("1", true)]
         [InlineData("0", false)]
-        [InlineData(null, false)]
-        public void Build_WithCustomBuildArgs_Framework(string envVariableValue, bool isWarningExpected) =>
-            TestCustomEventWarning<BuildWarningEventArgs>(envVariableValue, isWarningExpected);
+        [InlineData(null, true)]
+        public void Build_WithCustomBuildArgs_Framework(string envVariableValue, bool isErrorExpected) =>
+            TestCustomEvent<BuildErrorEventArgs>(envVariableValue, isErrorExpected);
 
-        private void TestCustomEventWarning<T>(string envVariableValue, bool isWarningExpected) where T : LazyFormattedBuildEventArgs
+        private void TestCustomEvent<T>(string envVariableValue, bool isWarningExpected) where T : LazyFormattedBuildEventArgs
         {
             var testFiles = _env.CreateTestProjectWithFiles(string.Empty, new[] { "main", "child1" }, string.Empty);
 

diff --git a/src/Build/BackEnd/Node/OutOfProcNode.cs b/src/Build/BackEnd/Node/OutOfProcNode.cs
@@ -584,18 +584,16 @@ private void SendPacket(INodePacket packet)
         {
             if (_nodeEndpoint.LinkStatus == LinkStatus.Active)
             {
-#if RUNTIME_TYPE_NETCORE
                 if (packet is LogMessagePacketBase logMessage
                     && logMessage.EventType == LoggingEventType.CustomEvent 
-                    &&
-                    (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_8) || !Traits.Instance.EscapeHatches.IsBinaryFormatterSerializationAllowed)
+                    && (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10) || !Traits.Instance.EscapeHatches.IsBinaryFormatterSerializationAllowed)
                     && Traits.Instance.EscapeHatches.EnableWarningOnCustomBuildEvent)
                 {
                     BuildEventArgs buildEvent = logMessage.NodeBuildEvent.Value.Value;
 
                     // Serializing unknown CustomEvent which has to use unsecure BinaryFormatter by TranslateDotNet<T>
-                    // Since BinaryFormatter is deprecated in dotnet 8+, log error so users discover root cause easier
-                    // then by reading CommTrace where it would be otherwise logged as critical infra error.
+                    // Since BinaryFormatter is deprecated, log error so users discover root cause easier than
+                    // by reading CommTrace where it would be otherwise logged as critical infra error.
                     _loggingService.LogError(_loggingContext?.BuildEventContext ?? BuildEventContext.Invalid, null, BuildEventFileInfo.Empty,
                             "DeprecatedEventSerialization",
                             buildEvent?.GetType().Name ?? string.Empty);
@@ -604,9 +602,6 @@ private void SendPacket(INodePacket packet)
                 {
                     _nodeEndpoint.SendData(packet);
                 }
-#else
-                _nodeEndpoint.SendData(packet);
-#endif
             }
         }
 

diff --git a/src/Framework/Traits.cs b/src/Framework/Traits.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Diagnostics;
 using System.Globalization;
 
 #nullable disable
@@ -388,12 +389,8 @@ public bool EnableWarningOnCustomBuildEvent
 
                 if (value == null)
                 {
-                    // If variable is not set explicitly, for .NETCORE warning appears.
-#if RUNTIME_TYPE_NETCORE
+                    // If variable is not set explicitly, warning appears.
                     return true;
-#else
-                    return false;
-#endif
                 }
 
                 return value == "1";
@@ -407,19 +404,30 @@ public bool IsBinaryFormatterSerializationAllowed
             {
                 if (!_isBinaryFormatterSerializationAllowed.HasValue)
                 {
+#if !NET35
+
+                    if (AppContext.TryGetSwitch("System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization",
+                            out bool enabled))
+#else
+                    bool enabled;
+#endif
+                    {
 #if RUNTIME_TYPE_NETCORE
-                    AppContext.TryGetSwitch("System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization",
-                        out bool enabled);
-                    _isBinaryFormatterSerializationAllowed = enabled;
+                        // Unexpected, but not worth to throw, but since maybe in future it will be removed from .NET Core, let's assert here.
+                        Debug.Assert(!enabled, "Switch System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization is expected to be defined for current runtime.");
+                        // At this point it means it is actually possible to use BinFmt serialization, but we shan't used it anyway.
+                        enabled = false;
 #else
-                    _isBinaryFormatterSerializationAllowed = true;
+                        // We expect, if the switch is not configured, that it use default/old behavior of .NET Framework = enabled.
+                        enabled = true;
 #endif
+                    }
+                    _isBinaryFormatterSerializationAllowed = enabled;
                 }
 
                 return _isBinaryFormatterSerializationAllowed.Value;
+                }
             }
-        }
-
 
         private static bool? ParseNullableBoolFromEnvironmentVariable(string environmentVariable)
         {