DisableBitTests.EnabledThroughFeatureSwitch/DisabledAlwaysInBrowser failures

[tmds]

These tests are failing in our CI which is building with these flags:

/p:NoPgoOptimize=true --portablebuild false /p:DotNetBuildFromSource=true /p:DotNetBuildSourceOnly=true /p:DotNetBuildTests=true --runtimeconfiguration Release --librariesConfiguration Debug

The test failures are:

System.Runtime.Serialization.Formatters.Tests.DisableBitTests.EnabledThroughFeatureSwitch

Microsoft.DotNet.RemoteExecutor.RemoteExecutionException: Microsoft.DotNet.RemoteExecutor.RemoteExecutionException : Remote process failed with an unhandled exception.
        Child exception:
Xunit.Sdk.ThrowsAnyException: Assert.ThrowsAny() Failure: No exception was thrown
Expected: typeof(System.NotSupportedException)
at System.Runtime.Serialization.Formatters.Tests.DisableBitTests.<>c.<RunRemoteTest>b__9_1() in /home/tester/runtime/src/libraries/System.Runtime.Serialization.Formatters/tests/DisableBitTests.cs:line 93
at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args) in /home/tester/runtime/src/coreclr/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.CoreCLR.cs:line 36
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr) in /home/tester/runtime/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.cs:line 57
Child process:
System.Runtime.Serialization.Formatters.Tests, Version=9.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51 System.Runtime.Serialization.Formatters.Tests.DisableBitTests+<>c Void <RunRemoteTest>b__9_1()

System.Runtime.Serialization.Formatters.Tests.DisableBitTests.DisabledAlwaysInBrowser

Xunit.Sdk.ThrowsException: Assert.Throws() Failure: No exception was thrown\nExpected: typeof(System.PlatformNotSupportedException)
        at System.Runtime.Serialization.Formatters.Tests.DisableBitTests.DisabledAlwaysInBrowser() in /home/tester/runtime/src/libraries/System.Runtime.Serialization.Formatters/tests/DisableBitTests.cs:line 30
at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args) in /home/tester/runtime/src/coreclr/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.CoreCLR.cs:line 36
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr) in /home/tester/runtime/src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.cs:line 57

Probably this is triggered by the changes in: #103255.

@bartonjs do you have an idea what may be the issue?

cc @omajid

[adamsitnik]

@tmds BinaryFormatter was moved to an OOB package (not part of shared SDK) in #103255

The code that checks whether it's enabled can be found here:
https://github.com/dotnet/runtime/blob/main/src/libraries/System.Runtime.Serialization.Formatters/tests/TestConfiguration.cs
and here:

runtime/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs

Lines 728 to 756 in 5c53e8e

	private static bool DetermineBinaryFormatterSupport()
	{
	if (IsNetFramework)
	{
	return true;
	}
	else if (IsNativeAot)
	{
	return false;
	}
	Assembly assembly = typeof(System.Runtime.Serialization.Formatters.Binary.BinaryFormatter).Assembly;
	AssemblyName name = assembly.GetName();
	Version assemblyVersion = name.Version;
	bool isSupported = true;
	// Version 8.1 is the version in the shared runtime (.NET 9+) that has the type disabled with no config.
	// Assembly versions beyond 8.1 are the fully functional version from NuGet.
	// Assembly versions before 8.1 probably won't be encountered, since that's the past.
	if (assemblyVersion.Major == 8 && assemblyVersion.Minor == 1)
	{
	isSupported = false;
	}
	return isSupported;
	}
	}

I am afraid that it's going to require some debugging to see what does not work as expected

[tmds]

Thanks for the pointers @adamsitnik!

[tmds]

@adamsitnik and @ViktorHofer (because @adamsitnik's status indicates he is unavailable),

I dug into this a bit deeper.

I have a dll that is version 8.1. According to DetermineBinaryFormatterSupport it should not support serialization, but according to the way the test fails, it supports serialization.

The test results are different when I run dotnet test against the test project. Then there are no test failures. To reproduce the issue I need to run the tests from build.sh using --test (like our CI is doing).

In xunit.console.runtimeconfig.json I see "System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization": false. I think

runtime/src/libraries/System.Runtime.Serialization.Formatters/tests/System.Runtime.Serialization.Formatters.Tests.csproj

Lines 74 to 78 in 7946e6f

	<Target Name="RemoveSerializationRuntimeOption" BeforeTargets="GenerateBuildRuntimeConfigurationFiles">
	<ItemGroup>
	<RuntimeHostConfigurationOption Remove="System.Runtime.Serialization.EnableUnsafeBinaryFormatterSerialization" />
	</ItemGroup>
	</Target>

is probably meant to prevent that. It may not be working and therefore mess up the test conditions.

[ViktorHofer]

I have a dll that is version 8.1. According to DetermineBinaryFormatterSupport it should not support serialization, but according to the way the test fails, it supports serialization.

The System.Runtime.Serialization.Formatters assembly with an 8.1.0.0 version is the one that supports serialization. The 9.0.0.0 assembly is the PNSE throwing one.

[tmds]

The System.Runtime.Serialization.Formatters assembly with an 8.1.0.0 version is the one that supports serialization.

I assumed 8.1 should not support serialization based on:

runtime/src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs

Lines 745 to 752 in 2fe4afa

	// Version 8.1 is the version in the shared runtime (.NET 9+) that has the type disabled with no config.
	// Assembly versions beyond 8.1 are the fully functional version from NuGet.
	// Assembly versions before 8.1 probably won't be encountered, since that's the past.
	if (assemblyVersion.Major == 8 && assemblyVersion.Minor == 1)
	{
	isSupported = false;
	}

The test results are different when I run dotnet test against the test project. Then there are no test failures. To reproduce the issue I need to run the tests from build.sh using --test (like our CI is doing).

I looked further into this difference.

The assembly that is used by the System.Runtime.Serialization.Formatters test differs between executing the top-level ./build.sh with --test argument vs running dotnet test against the test project directly.

In both cases the assembly version is 8.1.0.0. For the first invocation, the serialization implementation is present in the assembly, while the second command uses an assembly that throws PNSE.
The second assembly matches the DetermineBinaryFormatterSupport expectation for assembly version, so the tests pass.

[ViktorHofer]

Oh sorry, I got this wrong. You are right, that the inbox implementation is pinned to 8.1.0.0. The OOB one has 9.0.0.0.

runtime/src/libraries/System.Runtime.Serialization.Formatters/Directory.Build.targets

Lines 5 to 12 in 2fe4afa

	<!--
	The real implementation of this library is built for NetCoreAppMinimum,
	the NetCoreAppCurrent build has a non-functional copy of BinaryFormatter.
	The NetCoreAppCurrent build is only included in the shared runtime,
	and should always lose to the package for unification; so it is pinned
	at an assembly version that will always lose.
	-->
	<AssemblyVersion Condition="'$(TargetFramework)' == '$(NetCoreAppCurrent)'">8.1.0.0</AssemblyVersion>