From 476cfea78571fe99356d429c2661c0481808db5f Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Tue, 13 Jun 2023 15:41:26 -0500 Subject: [PATCH 1/3] Fix DependencyContext splitting on semi-colon https://github.com/dotnet/runtime/commit/5e67657e20665c32c2bd5c4ac1c8b1af78c9677e introduced a bug in DependencyContextPaths where the static array is not initialized before it is being used in the Create static method. This fix reorders the fields so that s_semicolon is initialized before it is first used. --- .../src/DependencyContextPaths.cs | 4 ++-- .../tests/DependencyContextTests.cs | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyModel/src/DependencyContextPaths.cs b/src/libraries/Microsoft.Extensions.DependencyModel/src/DependencyContextPaths.cs index 4d056a25bd76c..6807aba0a819f 100644 --- a/src/libraries/Microsoft.Extensions.DependencyModel/src/DependencyContextPaths.cs +++ b/src/libraries/Microsoft.Extensions.DependencyModel/src/DependencyContextPaths.cs @@ -12,6 +12,8 @@ internal sealed class DependencyContextPaths private const string DepsFilesProperty = "APP_CONTEXT_DEPS_FILES"; private const string FxDepsFileProperty = "FX_DEPS_FILE"; + private static readonly char[] s_semicolon = new[] { ';' }; + public static DependencyContextPaths Current { get; } = GetCurrent(); public string? Application { get; } @@ -20,8 +22,6 @@ internal sealed class DependencyContextPaths public IEnumerable NonApplicationPaths { get; } - private static readonly char[] s_semicolon = new[] { ';' }; - public DependencyContextPaths( string? application, string? sharedRuntime, diff --git a/src/libraries/Microsoft.Extensions.DependencyModel/tests/DependencyContextTests.cs b/src/libraries/Microsoft.Extensions.DependencyModel/tests/DependencyContextTests.cs index 0c570d6a441f2..6427c1ec01cce 100644 --- a/src/libraries/Microsoft.Extensions.DependencyModel/tests/DependencyContextTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyModel/tests/DependencyContextTests.cs @@ -276,6 +276,18 @@ public void MergeMergesRuntimeGraph() Subject.Fallbacks.Should().BeEquivalentTo("win7-x64", "win7-x86"); } + [Fact] + public void DefaultWorksCorrectly() + { + // only need to assert the context contains non-null properties. + + var context = DependencyContext.Default; + Assert.NotNull(context); + Assert.NotNull(context.RuntimeGraph); + Assert.NotNull(context.RuntimeLibraries); + Assert.NotNull(context.Target); + } + private TargetInfo CreateTargetInfo() { return new TargetInfo( From c6026e3eee432c18fac5d0dbd75686cd7d2a0633 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Tue, 13 Jun 2023 16:11:57 -0500 Subject: [PATCH 2/3] - Don't cache the semicolon array since it is only used once at startup - Skip test on netfx since it doesn't work. --- .../src/DependencyContextPaths.cs | 10 +++++++--- .../tests/DependencyContextTests.cs | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyModel/src/DependencyContextPaths.cs b/src/libraries/Microsoft.Extensions.DependencyModel/src/DependencyContextPaths.cs index 6807aba0a819f..ffad428492aa0 100644 --- a/src/libraries/Microsoft.Extensions.DependencyModel/src/DependencyContextPaths.cs +++ b/src/libraries/Microsoft.Extensions.DependencyModel/src/DependencyContextPaths.cs @@ -12,8 +12,6 @@ internal sealed class DependencyContextPaths private const string DepsFilesProperty = "APP_CONTEXT_DEPS_FILES"; private const string FxDepsFileProperty = "FX_DEPS_FILE"; - private static readonly char[] s_semicolon = new[] { ';' }; - public static DependencyContextPaths Current { get; } = GetCurrent(); public string? Application { get; } @@ -42,7 +40,13 @@ private static DependencyContextPaths GetCurrent() internal static DependencyContextPaths Create(string? depsFiles, string? sharedRuntime) { - string[]? files = depsFiles?.Split(s_semicolon, StringSplitOptions.RemoveEmptyEntries); +#if NETCOREAPP + const char separator = ';'; +#else + // This method is only executed at startup. No need to cache the char[]. + char[] separator = { ';' }; +#endif + string[]? files = depsFiles?.Split(separator, StringSplitOptions.RemoveEmptyEntries); string? application = files != null && files.Length > 0 ? files[0] : null; string[]? nonApplicationPaths = files? diff --git a/src/libraries/Microsoft.Extensions.DependencyModel/tests/DependencyContextTests.cs b/src/libraries/Microsoft.Extensions.DependencyModel/tests/DependencyContextTests.cs index 6427c1ec01cce..f657aead2a578 100644 --- a/src/libraries/Microsoft.Extensions.DependencyModel/tests/DependencyContextTests.cs +++ b/src/libraries/Microsoft.Extensions.DependencyModel/tests/DependencyContextTests.cs @@ -277,6 +277,7 @@ public void MergeMergesRuntimeGraph() } [Fact] + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "GetEntryAssembly() returns null")] public void DefaultWorksCorrectly() { // only need to assert the context contains non-null properties. From e67bb2447b48ff3a1145582e00594bf4bd978266 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Tue, 13 Jun 2023 16:46:48 -0500 Subject: [PATCH 3/3] Address PR feedback Co-authored-by: Stephen Toub --- .../src/DependencyContextPaths.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.DependencyModel/src/DependencyContextPaths.cs b/src/libraries/Microsoft.Extensions.DependencyModel/src/DependencyContextPaths.cs index ffad428492aa0..b5c6730eef3ee 100644 --- a/src/libraries/Microsoft.Extensions.DependencyModel/src/DependencyContextPaths.cs +++ b/src/libraries/Microsoft.Extensions.DependencyModel/src/DependencyContextPaths.cs @@ -43,7 +43,7 @@ internal static DependencyContextPaths Create(string? depsFiles, string? sharedR #if NETCOREAPP const char separator = ';'; #else - // This method is only executed at startup. No need to cache the char[]. + // This method is only executed once at startup. No need to cache the char[]. char[] separator = { ';' }; #endif string[]? files = depsFiles?.Split(separator, StringSplitOptions.RemoveEmptyEntries);