Fix resource stream for collectible assemblies
authorJan Vorlicek <janvorli@microsoft.com>
Thu, 28 Feb 2019 19:06:57 +0000 (20:06 +0100)
committerJan Vorlicek <janvorli@microsoft.com>
Thu, 28 Feb 2019 19:06:57 +0000 (20:06 +0100)
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.Private.CoreLib/src/System/Reflection/RuntimeAssembly.cs
tests/src/Regressions/coreclr/GitHub_22888/test22888.cs [new file with mode: 0644]
tests/src/Regressions/coreclr/GitHub_22888/test22888.csproj [new file with mode: 0644]
tests/src/Regressions/coreclr/GitHub_22888/test22888.resx [new file with mode: 0644]
tests/src/Regressions/coreclr/GitHub_22888/test22888resources.cs [new file with mode: 0644]
tests/src/Regressions/coreclr/GitHub_22888/test22888resources.csproj [new file with mode: 0644]

index d670031..6905d7a 100644 (file)
@@ -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 (file)
index 0000000..c94f814
--- /dev/null
@@ -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 (file)
index 0000000..3e5b3ed
--- /dev/null
@@ -0,0 +1,40 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{9D87696F-8C3A-49B8-86AD-A26D850B5A14}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <CLRTestKind>BuildAndRun</CLRTestKind>
+    <CLRTestPriority>0</CLRTestPriority>
+  </PropertyGroup>
+  <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
+  </PropertyGroup>
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+  <ItemGroup>
+    <!-- Add Compile Object Here -->
+    <Compile Include="test22888.cs" />
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <ItemGroup>
+    <ProjectReference Include="../../../Common/CoreCLRTestLibrary/CoreCLRTestLibrary.csproj" />
+    <ProjectReference Include="test22888resources.csproj" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
+  </PropertyGroup>
+</Project>
\ 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 (file)
index 0000000..1af7de1
--- /dev/null
@@ -0,0 +1,120 @@
+<?xml version="1.0" encoding="utf-8"?>
+<root>
+  <!-- 
+    Microsoft ResX Schema 
+    
+    Version 2.0
+    
+    The primary goals of this format is to allow a simple XML format 
+    that is mostly human readable. The generation and parsing of the 
+    various data types are done through the TypeConverter classes 
+    associated with the data types.
+    
+    Example:
+    
+    ... ado.net/XML headers & schema ...
+    <resheader name="resmimetype">text/microsoft-resx</resheader>
+    <resheader name="version">2.0</resheader>
+    <resheader name="reader">System.Resources.ResXResourceReader, System.Windows.Forms, ...</resheader>
+    <resheader name="writer">System.Resources.ResXResourceWriter, System.Windows.Forms, ...</resheader>
+    <data name="Name1"><value>this is my long string</value><comment>this is a comment</comment></data>
+    <data name="Color1" type="System.Drawing.Color, System.Drawing">Blue</data>
+    <data name="Bitmap1" mimetype="application/x-microsoft.net.object.binary.base64">
+        <value>[base64 mime encoded serialized .NET Framework object]</value>
+    </data>
+    <data name="Icon1" type="System.Drawing.Icon, System.Drawing" mimetype="application/x-microsoft.net.object.bytearray.base64">
+        <value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
+        <comment>This is a comment</comment>
+    </data>
+                
+    There are any number of "resheader" rows that contain simple 
+    name/value pairs.
+    
+    Each data row contains a name, and value. The row also contains a 
+    type or mimetype. Type corresponds to a .NET class that support 
+    text/value conversion through the TypeConverter architecture. 
+    Classes that don't support this are serialized and stored with the 
+    mimetype set.
+    
+    The mimetype is used for serialized objects, and tells the 
+    ResXResourceReader how to depersist the object. This is currently not 
+    extensible. For a given mimetype the value must be set accordingly:
+    
+    Note - application/x-microsoft.net.object.binary.base64 is the format 
+    that the ResXResourceWriter will generate, however the reader can 
+    read any of the formats listed below.
+    
+    mimetype: application/x-microsoft.net.object.binary.base64
+    value   : The object must be serialized with 
+            : System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
+            : and then encoded with base64 encoding.
+    
+    mimetype: application/x-microsoft.net.object.soap.base64
+    value   : The object must be serialized with 
+            : System.Runtime.Serialization.Formatters.Soap.SoapFormatter
+            : and then encoded with base64 encoding.
+
+    mimetype: application/x-microsoft.net.object.bytearray.base64
+    value   : The object must be serialized into a byte array 
+            : using a System.ComponentModel.TypeConverter
+            : and then encoded with base64 encoding.
+    -->
+  <xsd:schema id="root" xmlns="" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:msdata="urn:schemas-microsoft-com:xml-msdata">
+    <xsd:import namespace="http://www.w3.org/XML/1998/namespace" />
+    <xsd:element name="root" msdata:IsDataSet="true">
+      <xsd:complexType>
+        <xsd:choice maxOccurs="unbounded">
+          <xsd:element name="metadata">
+            <xsd:complexType>
+              <xsd:sequence>
+                <xsd:element name="value" type="xsd:string" minOccurs="0" />
+              </xsd:sequence>
+              <xsd:attribute name="name" use="required" type="xsd:string" />
+              <xsd:attribute name="type" type="xsd:string" />
+              <xsd:attribute name="mimetype" type="xsd:string" />
+              <xsd:attribute ref="xml:space" />
+            </xsd:complexType>
+          </xsd:element>
+          <xsd:element name="assembly">
+            <xsd:complexType>
+              <xsd:attribute name="alias" type="xsd:string" />
+              <xsd:attribute name="name" type="xsd:string" />
+            </xsd:complexType>
+          </xsd:element>
+          <xsd:element name="data">
+            <xsd:complexType>
+              <xsd:sequence>
+                <xsd:element name="value" type="xsd:string" minOccurs="0" msdata:Ordinal="1" />
+                <xsd:element name="comment" type="xsd:string" minOccurs="0" msdata:Ordinal="2" />
+              </xsd:sequence>
+              <xsd:attribute name="name" type="xsd:string" use="required" msdata:Ordinal="1" />
+              <xsd:attribute name="type" type="xsd:string" msdata:Ordinal="3" />
+              <xsd:attribute name="mimetype" type="xsd:string" msdata:Ordinal="4" />
+              <xsd:attribute ref="xml:space" />
+            </xsd:complexType>
+          </xsd:element>
+          <xsd:element name="resheader">
+            <xsd:complexType>
+              <xsd:sequence>
+                <xsd:element name="value" type="xsd:string" minOccurs="0" msdata:Ordinal="1" />
+              </xsd:sequence>
+              <xsd:attribute name="name" type="xsd:string" use="required" />
+            </xsd:complexType>
+          </xsd:element>
+        </xsd:choice>
+      </xsd:complexType>
+    </xsd:element>
+  </xsd:schema>
+  <resheader name="resmimetype">
+    <value>text/microsoft-resx</value>
+  </resheader>
+  <resheader name="version">
+    <value>2.0</value>
+  </resheader>
+  <resheader name="reader">
+    <value>System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
+  </resheader>
+  <resheader name="writer">
+    <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
+  </resheader>
+</root>
\ 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 (file)
index 0000000..49a5b03
--- /dev/null
@@ -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 (file)
index 0000000..72d4f78
--- /dev/null
@@ -0,0 +1,44 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
+  <PropertyGroup>
+    <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
+    <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
+    <SchemaVersion>2.0</SchemaVersion>
+    <ProjectGuid>{558D34B5-6EC1-4F30-8A15-B94F7EC74DA9}</ProjectGuid>
+    <OutputType>Exe</OutputType>
+    <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
+    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <CLRTestKind>BuildOnly</CLRTestKind>
+    <CLRTestPriority>0</CLRTestPriority>
+  </PropertyGroup>
+  <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
+  </PropertyGroup>
+  <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' ">
+  </PropertyGroup>
+  <ItemGroup>
+    <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
+      <Visible>False</Visible>
+    </CodeAnalysisDependentAssemblyPaths>
+  </ItemGroup>
+  <ItemGroup>
+    <!-- Add Compile Object Here -->
+    <Compile Include="test22888resources.cs" />
+  </ItemGroup>
+  <ItemGroup>
+    <EmbeddedResource Include="test22888.resx">
+      <Generator>ResXFileCodeGenerator</Generator>
+    </EmbeddedResource>
+  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
+  </ItemGroup>
+  <ItemGroup>
+    <ProjectReference Include="../../../Common/CoreCLRTestLibrary/CoreCLRTestLibrary.csproj" />
+  </ItemGroup>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
+  <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' ">
+  </PropertyGroup>
+</Project>
\ No newline at end of file