From 5fcc6f14e1197c313edc3673ad67f6af26c2199e Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Thu, 28 Feb 2019 20:06:57 +0100 Subject: [PATCH] Fix resource stream for collectible assemblies The GetManifestResourceStream was returning a stream that didn't keep the underlying assembly alive. For collectible assemblies, that means that an assembly could be collected and the stream still kept, potentially reading garbage. The fix is to create a new stream type that stores a reference to the underlying assembly, thus ensuring the proper lifetime management. --- .../src/System/Reflection/RuntimeAssembly.cs | 12 ++- .../Regressions/coreclr/GitHub_22888/test22888.cs | 93 ++++++++++++++++ .../coreclr/GitHub_22888/test22888.csproj | 40 +++++++ .../coreclr/GitHub_22888/test22888.resx | 120 +++++++++++++++++++++ .../coreclr/GitHub_22888/test22888resources.cs | 11 ++ .../coreclr/GitHub_22888/test22888resources.csproj | 44 ++++++++ 6 files changed, 319 insertions(+), 1 deletion(-) create mode 100644 tests/src/Regressions/coreclr/GitHub_22888/test22888.cs create mode 100644 tests/src/Regressions/coreclr/GitHub_22888/test22888.csproj create mode 100644 tests/src/Regressions/coreclr/GitHub_22888/test22888.resx create mode 100644 tests/src/Regressions/coreclr/GitHub_22888/test22888resources.cs create mode 100644 tests/src/Regressions/coreclr/GitHub_22888/test22888resources.csproj diff --git a/src/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs b/src/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs index d670031..6905d7a 100644 --- a/src/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs +++ b/src/System.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs @@ -29,6 +29,16 @@ namespace System.Reflection #endregion + private class ManifestResourceStream : UnmanagedMemoryStream + { + private RuntimeAssembly _manifestAssembly; + + internal unsafe ManifestResourceStream(RuntimeAssembly manifestAssembly, byte* pointer, long length, long capacity, FileAccess access) : base(pointer, length, capacity, access) + { + _manifestAssembly = manifestAssembly; + } + } + internal object SyncRoot { get @@ -227,7 +237,7 @@ namespace System.Reflection if (pbInMemoryResource != null) { - return new UnmanagedMemoryStream(pbInMemoryResource, length, length, FileAccess.Read); + return new ManifestResourceStream(this, pbInMemoryResource, length, length, FileAccess.Read); } return null; diff --git a/tests/src/Regressions/coreclr/GitHub_22888/test22888.cs b/tests/src/Regressions/coreclr/GitHub_22888/test22888.cs new file mode 100644 index 0000000..c94f814 --- /dev/null +++ b/tests/src/Regressions/coreclr/GitHub_22888/test22888.cs @@ -0,0 +1,93 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. +using System; +using System.IO; +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Runtime.Loader; + +class TestAssemblyLoadContext : AssemblyLoadContext +{ + public TestAssemblyLoadContext() : base(isCollectible: true) + { + } + + protected override Assembly Load(AssemblyName name) + { + return null; + } +} +public class Test22888 +{ + [MethodImpl(MethodImplOptions.NoInlining)] + static Stream LoadGetResourceStreamAndUnload(string assemblyPath, out WeakReference alcWeakRef) + { + var alc = new TestAssemblyLoadContext(); + alcWeakRef = new WeakReference(alc); + + Assembly a = alc.LoadFromAssemblyPath(assemblyPath); + Stream resourceStream = a.GetManifestResourceStream("test22888.resources"); + alc.Unload(); + + return resourceStream; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static bool LoadAndUnload(string assemblyPath, out WeakReference alcWeakRef) + { + Stream s = LoadGetResourceStreamAndUnload(assemblyPath, out alcWeakRef); + + bool success = (s != null); + + if (success) + { + for (int i = 0; alcWeakRef.IsAlive && (i < 10); i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + + // Verify that the ALC is still alive - it should be kept alive by the Stream. + success = alcWeakRef.IsAlive; + if (!success) + { + Console.WriteLine("Failed to keep AssemblyLoadContext alive by the resource stream"); + } + GC.KeepAlive(s); + } + else + { + Console.WriteLine("Failed to get resource stream from the test assembly"); + } + + return success; + } + + public static int Main() + { + string currentAssemblyDirectory = Path.GetDirectoryName(new Uri(Assembly.GetExecutingAssembly().CodeBase).AbsolutePath); + string testAssemblyFullPath = Path.Combine(currentAssemblyDirectory, "test22888resources.exe"); + + WeakReference alcWeakRef; + bool success = LoadAndUnload(testAssemblyFullPath, out alcWeakRef); + + if (success) + { + for (int i = 0; alcWeakRef.IsAlive && (i < 10); i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + + // Now the ALC should not be alive anymore as the resource stream is gone + success = !alcWeakRef.IsAlive; + if (!success) + { + Console.WriteLine("Failed to unload the test assembly"); + } + } + + return success ? 100 : 101; + } +} diff --git a/tests/src/Regressions/coreclr/GitHub_22888/test22888.csproj b/tests/src/Regressions/coreclr/GitHub_22888/test22888.csproj new file mode 100644 index 0000000..3e5b3ed --- /dev/null +++ b/tests/src/Regressions/coreclr/GitHub_22888/test22888.csproj @@ -0,0 +1,40 @@ + + + + + Debug + AnyCPU + 2.0 + {9D87696F-8C3A-49B8-86AD-A26D850B5A14} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + true + BuildAndRun + 0 + + + + + + + + + False + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/src/Regressions/coreclr/GitHub_22888/test22888.resx b/tests/src/Regressions/coreclr/GitHub_22888/test22888.resx new file mode 100644 index 0000000..1af7de1 --- /dev/null +++ b/tests/src/Regressions/coreclr/GitHub_22888/test22888.resx @@ -0,0 +1,120 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + text/microsoft-resx + + + 2.0 + + + System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + + System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 + + \ No newline at end of file diff --git a/tests/src/Regressions/coreclr/GitHub_22888/test22888resources.cs b/tests/src/Regressions/coreclr/GitHub_22888/test22888resources.cs new file mode 100644 index 0000000..49a5b03 --- /dev/null +++ b/tests/src/Regressions/coreclr/GitHub_22888/test22888resources.cs @@ -0,0 +1,11 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. +using System; + +class Program +{ + static void Main(string[] args) + { + } +} diff --git a/tests/src/Regressions/coreclr/GitHub_22888/test22888resources.csproj b/tests/src/Regressions/coreclr/GitHub_22888/test22888resources.csproj new file mode 100644 index 0000000..72d4f78 --- /dev/null +++ b/tests/src/Regressions/coreclr/GitHub_22888/test22888resources.csproj @@ -0,0 +1,44 @@ + + + + + Debug + AnyCPU + 2.0 + {558D34B5-6EC1-4F30-8A15-B94F7EC74DA9} + Exe + {786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC} + ..\..\ + true + BuildOnly + 0 + + + + + + + + + False + + + + + + + + + ResXFileCodeGenerator + + + + + + + + + + + + \ No newline at end of file -- 2.7.4