Runtime: System.Configuration.ConfigurationManager cannot read AppSettings when published with publishSingleFile=true

[mshobohm]

Description

System.Configuration.ConfigurationManager cannot read AppSettings when published with publishSingleFile=yes

Configuration

.net 5 preview 8
tried out with windows-x64 and linux-64

Regression?

I couldnt get this bug working in 3.1

Other information

C:ProjekteAppSetting>dotnet --version
5.0.100-preview.8.20417.9

C:ProjekteAppSetting>dotnet run
OK

C:ProjekteAppSetting>dotnet publish -c Release
Microsoft (R)-Build-Engine, Version 16.8.0-preview-20414-02+a55ce4fbb für .NET
Copyright (C) Microsoft Corporation. Alle Rechte vorbehalten.

Wiederherzustellende Projekte werden ermittelt...
Alle Projekte sind für die Wiederherstellung auf dem neuesten Stand.
Sie verwenden eine Vorschauversion von .NET Core. Weitere Informationen: https://aka.ms/dotnet-core-preview
AppSetting -> C:ProjekteAppSettingbinReleasenet50win-x64AppSetting.dll
AppSetting -> C:ProjekteAppSettingbinReleasenet50win-x64publish

C:ProjekteAppSetting>C:ProjekteAppSettingbinReleasenet50win-x64publishAppSetting.exe
Not Found
(Expected behavior is the output of "OK" as with dotnet run)

AppSetting.zip

[msftbot[bot]]

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

[am11]

@agocke, @safern, this is clearly a 3.1 to 5.0.0 regression. Should it be considered for backporting?

[msftbot[bot]]

Tagging subscribers to this area: @safern
See info in area-owners.md if you want to be subscribed.

[safern]

@agocke, @safern, this is clearly a 3.1 to 5.0.0 regression. Should it be considered for backporting?

cc: @ericstj @danmosemsft

[danmosemsft]

Can someone investigate and understand the issue?

[am11]

@danmosemsft, done and fixed by https://github.com/dotnet/runtime/pull/42176.

[ericstj]

So the breaking change was that GetEntryAssembly().ManifestModule.Name was returning the app name for single file in 3.x and starting in 5.0 it returns null. System.Configuration.ConfigurationManager was just one consumer of this. Why not undo the change to runtime that caused the regression?

[jkotas]

This regression is side-effect of the true single file in .NET 5. The assemblies do not physically exist on disk in the true single file model in .NET 5 . Many libraries (including System.Configuration.ConfigurationManager in this case) assume that assemblies physically exist on disk and break when used in true single file context. https://github.com/dotnet/designs/blob/main/accepted/2020/form-factors.md#single-file discusses this in more detail.

We do provide build option for the old unzip-to-disk single file, so this is not strictly a blocker. If an app used the unzip-to-disk single file in .NET 3.1 and the true single file in .NET 5 breaks it, it can continue using unzip-to-disk single file as a workaround.

[ericstj]

I believe the entry assembly physically exists on disk, right?

[am11]

starting in 5.0 it returns null

The breaking change was that it returns <Unknown> string. We have detected isSingleFile and special cased it.

[jkotas]

No, the assembly does not physically exist on disk as .dll. The only file that physically exist on disk in true single-file mode is the unmanaged host. All managed assemblies are embedded as resources to it and loaded as in-memory loads.

[jkotas]

The breaking change was that it returns <Unknown> string.

<Unknown> is the expected return value of this API for assemblies loaded from memory.

[ericstj]

I see, understood. I can see why you want to do this, because most code that would follow after consuming a faked GetEntryAssembly will do the wrong thing.

I wonder if all these places need similar fixes: https://source.dot.net/#System.Private.CoreLib/Assembly.cs,0b58cbf47b813b13,references

[am11]

<Unknown> is the expected return value of this API for assemblies loaded from memory.

True. I meant from the prespective of its usage in ConfigurationManager, it is a breaking change. Even with 3.1, the assembly name <appname>.exe exists on the disk, but not <appname>.dll in case of single file on Windows, but the underlying API returns <appname>.dll which is how it was working correctly before.

[jkotas]

Even with 3.1, the assembly name .exe exists on the disk, but not .dll in case of single file on Windows

<appname>.dll does exist on disk in the temp directory in 3.1.

[ericstj]

I tested a few of the cases I found in the link shared above and this is broken as well:
C# try { var store = IsolatedStorageFile.GetUserStoreForApplication(); var file = store.CreateFile("test.txt"); Console.WriteLine(file.Name); } catch (Exception e) { Console.WriteLine(e); }

Works in 3.1 single file, fails in 5.0 single file. Haven't gone through all.

[am11]

@ericstj, here is a quick search result of ManifestModule usage in the repo:

# git pathspec: include all C# code and exclude *Test* like paths
~/projects/runtime $ git grep \\.ManifestModule :/*.cs ':(exclude)*Test*'

src/coreclr/src/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs:            return GetCustomAttributes((RuntimeModule)target.ManifestModule, RuntimeAssembly.GetToken(target.GetNativeHandle()));
src/coreclr/src/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs:            return IsCustomAttributeDefined((assembly.ManifestModule as RuntimeModule)!, RuntimeAssembly.GetToken(assembly.GetNativeHandle()), caType);
src/coreclr/src/System.Private.CoreLib/src/System/Reflection/CustomAttribute.cs:            return GetCustomAttributes((assembly.ManifestModule as RuntimeModule)!, assemblyToken, 0, caType);
src/coreclr/src/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.cs:                s_anonymouslyHostedDynamicMethodsModule = (InternalModuleBuilder)assembly.ManifestModule!;
src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs:                    string configBasePath = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, exeAssembly.ManifestModule.Name);
src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/ClientConfigPaths.cs:                Uri codeBase = new Uri(Path.Combine(AppDomain.CurrentDomain.BaseDirectory, assembly.ManifestModule.Name));
src/libraries/System.Reflection.Context/src/System/Reflection/Context/Delegation/DelegatingAssembly.cs:            get { return UnderlyingAssembly.ManifestModule; }
src/libraries/System.Reflection.Context/src/System/Reflection/Context/Projection/ProjectingAssembly.cs:            get { return Projector.ProjectModule(base.ManifestModule); }
src/libraries/System.Reflection.MetadataLoadContext/src/System/Reflection/MetadataLoadContext.Loading.cs:                    if (candidate.ManifestModule.ModuleVersionId != winner.ManifestModule.ModuleVersionId)
src/mono/netcore/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.cs:            public static readonly Module anon_host_module = AssemblyBuilder.DefineDynamicAssembly(new AssemblyName() { Name = "Anonymously Hosted DynamicMethods Assembly" }, AssemblyBuilderAccess.Run).ManifestModule;

Name property is not used in many places.

[am11]

Works in 3.1 single file, fails in 5.0 single file. Haven't gone through all.

This is different as it is about another API CodeBase deprecated in 5.0. The exception is:

System.NotSupportedException: CodeBase is not supported on assemblies loaded from a single-file bundle.
   at System.Reflection.RuntimeAssembly.get_CodeBase()

Should we have a separate issue to track it?

[ericstj]

about another API CodeBase deprecated in 5.0.

This isn't a deprecation. Deprecation is design-time warning with no runtime behavior change. This is a breaking change due to single-file changes. Sure, it can be a new issue. I'm more worried that this is a new issue. That means there are multiple breaking changes to single-file that don't have good test coverage in libraries. cc @jeffschwMSFT

[am11]

This isn't a deprecation. Deprecation is design-time warning with no runtime behavior change.

Agreed. We suppress the deprecation warning here: https://github.com/dotnet/runtime/blob/b19d7980c6a0e1f03312c8c2033fcbd13822378f/src/libraries/System.IO.IsolatedStorage/src/System/IO/IsolatedStorage/Helper.Win32Unix.cs#L57-L59

Maybe we can try to use the recommended Assembly.Location API there and handle isSingleFile = Assembly.Location.Length ==0; if(isSingleFile) ..use currentProcess.MainModule?.FileName instead...

Single file integration tests (maybe with remote execution; outerloop) support would be great. I tried to figure out how to effectively test the PR changes in the CI, but couldn't find much.

[agocke]

That means there are multiple breaking changes to single-file that don't have good test coverage in libraries.

I would agree with this. Our current inability to compile the unit tests and run them through single-file publish means our test coverage is very limited.

[agocke]

@am11 We have some limited API tests for running in single-file here: https://github.com/dotnet/runtime/blob/master/src/installer/tests/Assets/TestProjects/SingleFileApiTests/Program.cs

Unit test setup here: https://github.com/dotnet/runtime/blob/master/src/installer/tests/Microsoft.NET.HostModel.Tests/AppHost.Bundle.Tests/SingleFileApiTests.cs

[ericstj]

Seems like this is a similar test problem that we've solved in the past for blazor / UWP / etc. If single file presents itself in a way that runtime and libraries have significantly different behaviors we should test it like a new platform. cc @ViktorHofer

[am11]

@agocke, thanks. Would it be out of scope of this test project, if we add the ConfigurationManager and IsolatedStorage APIs tests in SingleFileApiTests (for now™️, until there is proper infra support for singlefile in src/libraries tests)?

[agocke]

@ericstj I suspect many of these APIs are similarly broken for blazor/UWP/.NET Native. I know CodeBase throws on .NET Native, so anything that uses it is certainly broken.

[ericstj]

I was saying that folks solved the engineering problem to get testing running for those platforms. I think we could solve the same here if we think it’s important enough to not regress.

We actually did a lot of work to minimize the number of libraries that were broken on these platforms in the past. Fixes similar to what @am11 did here.

Right now “certainly broken” is “certainly regressed from 3.1” which is why we are talking about this now.

[agocke]

Agreed, I think finding a way to run at least some of the unit tests in a single-file publish is possible and valuable.

[safern]

Seems like this is a similar test problem that we've solved in the past for blazor / UWP / etc. If single file presents itself in a way that runtime and libraries have significantly different behaviors we should test it like a new platform. cc @ViktorHofer

With the infrastructure that we did to build against ref pack and publish mobile tests with live runtime pack, this should be very doable and we could just add new build legs.