diff --git a/src/Microsoft.VisualStudio.Composition/Configuration/MetadataViewGenerator.cs b/src/Microsoft.VisualStudio.Composition/Configuration/MetadataViewGenerator.cs index 54e90b778..4c4ee1366 100644 --- a/src/Microsoft.VisualStudio.Composition/Configuration/MetadataViewGenerator.cs +++ b/src/Microsoft.VisualStudio.Composition/Configuration/MetadataViewGenerator.cs @@ -10,6 +10,7 @@ namespace Microsoft.VisualStudio.Composition { using System; using System.Collections.Generic; + using System.Collections.Immutable; using System.Globalization; using System.Linq; using System.Reflection; @@ -79,7 +80,6 @@ internal static class MetadataViewGenerator private const string MetadataViewFactoryName = "Create"; private static readonly Dictionary MetadataViewFactories = new Dictionary(); - private static readonly AssemblyName ProxyAssemblyName = new AssemblyName(string.Format(CultureInfo.InvariantCulture, "MetadataViewProxies_{0}", Guid.NewGuid())); private static readonly Type[] CtorArgumentTypes = new Type[] { typeof(IReadOnlyDictionary), typeof(IReadOnlyDictionary) }; private static readonly MethodInfo MdvDictionaryTryGet = CtorArgumentTypes[0].GetTypeInfo().GetMethod("TryGetValue"); @@ -87,28 +87,41 @@ internal static class MetadataViewGenerator private static readonly MethodInfo ObjectGetType = typeof(object).GetTypeInfo().GetMethod("GetType", Type.EmptyTypes); private static readonly ConstructorInfo ObjectCtor = typeof(object).GetTypeInfo().GetConstructor(Type.EmptyTypes); - private static ModuleBuilder transparentProxyModuleBuilder; - private static SkipClrVisibilityChecks skipClrVisibilityChecks; + private static readonly Dictionary, ModuleBuilder> TransparentProxyModuleBuilderByVisibilityCheck = new Dictionary, ModuleBuilder>(new ByContentEqualityComparer()); public delegate object MetadataViewFactory(IReadOnlyDictionary metadata, IReadOnlyDictionary defaultMetadata); private static AssemblyBuilder CreateProxyAssemblyBuilder(ConstructorInfo constructorInfo) { - return AssemblyBuilder.DefineDynamicAssembly(ProxyAssemblyName, AssemblyBuilderAccess.Run); + var proxyAssemblyName = new AssemblyName(string.Format(CultureInfo.InvariantCulture, "MetadataViewProxies_{0}", Guid.NewGuid())); + return AssemblyBuilder.DefineDynamicAssembly(proxyAssemblyName, AssemblyBuilderAccess.Run); } - private static ModuleBuilder GetProxyModuleBuilder() + /// + /// Gets the to use for generating a proxy for the given type. + /// + /// The type of the interface to generate a proxy for. + /// The to use. + private static ModuleBuilder GetProxyModuleBuilder(TypeInfo viewType) { + Requires.NotNull(viewType, nameof(viewType)); Assumes.True(Monitor.IsEntered(MetadataViewFactories)); - if (transparentProxyModuleBuilder == null) + + // Dynamic assemblies are relatively expensive. We want to create as few as possible. + // For each unique set of skip visibility check assemblies, we need a new dynamic assembly + // because the CLR will not honor any additions to that set once the first generated type is closed. + // So maintain a dictionary to point at dynamic modules based on the set of skip visiblity check assemblies they were generated with. + var skipVisibilityCheckAssemblies = SkipClrVisibilityChecks.GetSkipVisibilityChecksRequirements(viewType); + if (!TransparentProxyModuleBuilderByVisibilityCheck.TryGetValue(skipVisibilityCheckAssemblies, out ModuleBuilder moduleBuilder)) { - // make a new assemblybuilder and modulebuilder var assemblyBuilder = CreateProxyAssemblyBuilder(typeof(SecurityTransparentAttribute).GetTypeInfo().GetConstructor(Type.EmptyTypes)); - transparentProxyModuleBuilder = assemblyBuilder.DefineDynamicModule("MetadataViewProxiesModule"); - skipClrVisibilityChecks = new SkipClrVisibilityChecks(assemblyBuilder, transparentProxyModuleBuilder); + moduleBuilder = assemblyBuilder.DefineDynamicModule("MetadataViewProxiesModule"); + var skipClrVisibilityChecks = new SkipClrVisibilityChecks(assemblyBuilder, moduleBuilder); + skipClrVisibilityChecks.SkipVisibilityChecksFor(skipVisibilityCheckAssemblies); + TransparentProxyModuleBuilderByVisibilityCheck.Add(skipVisibilityCheckAssemblies, moduleBuilder); } - return transparentProxyModuleBuilder; + return moduleBuilder; } public static MetadataViewFactory GetMetadataViewFactory(Type viewType) @@ -141,9 +154,7 @@ private static TypeInfo GenerateInterfaceViewProxyType(Type viewType) TypeBuilder proxyTypeBuilder; Type[] interfaces = { viewType }; - var proxyModuleBuilder = GetProxyModuleBuilder(); - skipClrVisibilityChecks.SkipVisibilityChecksFor(viewType.GetTypeInfo()); - + var proxyModuleBuilder = GetProxyModuleBuilder(viewType.GetTypeInfo()); proxyTypeBuilder = proxyModuleBuilder.DefineType( string.Format(CultureInfo.InvariantCulture, "_proxy_{0}_{1}", viewType.FullName, Guid.NewGuid()), TypeAttributes.Public, @@ -269,5 +280,29 @@ private static IEnumerable GetAllProperties(this Type type) { return type.GetTypeInfo().GetInterfaces().Concat(new Type[] { type }).SelectMany(itf => itf.GetTypeInfo().GetProperties()); } + + private class ByContentEqualityComparer : IEqualityComparer> + { + public bool Equals(ImmutableHashSet x, ImmutableHashSet y) + { + if (x.Count != y.Count) + { + return false; + } + + return !x.Except(y).Any(); + } + + public int GetHashCode(ImmutableHashSet obj) + { + int hashCode = 0; + foreach (var item in obj) + { + hashCode += item.GetHashCode(); + } + + return hashCode; + } + } } } diff --git a/src/Microsoft.VisualStudio.Composition/Reflection/SkipClrVisibilityChecks.cs b/src/Microsoft.VisualStudio.Composition/Reflection/SkipClrVisibilityChecks.cs index 10ef3646c..ee1c62d41 100644 --- a/src/Microsoft.VisualStudio.Composition/Reflection/SkipClrVisibilityChecks.cs +++ b/src/Microsoft.VisualStudio.Composition/Reflection/SkipClrVisibilityChecks.cs @@ -4,6 +4,7 @@ namespace Microsoft.VisualStudio.Composition.Reflection { using System; using System.Collections.Generic; + using System.Collections.Immutable; using System.Linq; using System.Reflection; using System.Reflection.Emit; @@ -64,33 +65,64 @@ internal SkipClrVisibilityChecks(AssemblyBuilder assemblyBuilder, ModuleBuilder } /// - /// Ensures the CLR will skip visibility checks when accessing - /// the assembly that contains the specified member. + /// Gets the set of assemblies that a generated assembly must be granted the ability to skip visiblity checks for + /// in order to access the specified type. /// - /// The member that may not be publicly accessible. - internal void SkipVisibilityChecksFor(MemberInfo memberInfo) + /// The type which may be internal. + /// The set of names of assemblies to skip visibility checks for. + internal static ImmutableHashSet GetSkipVisibilityChecksRequirements(TypeInfo typeInfo) { - this.SkipVisibilityChecksFor(memberInfo.Module.Assembly); + Requires.NotNull(typeInfo, nameof(typeInfo)); + + if (ReflectionHelpers.IsPublic(typeInfo.AsType())) + { + return ImmutableHashSet.Empty; + } + else + { + var result = ImmutableHashSet.Empty + .Add(typeInfo.Assembly.GetName()); + + // If this type has a base type defined in another assembly that is also internal + // (with InternalsVisibleTo to the assembly hosting the original type) + // then we'll need to add that. + // TODO: Learn we somehow we don't need to do this, even given our test defines an internal interface + // that derives from another internal interface defined in another assembly. + ////if (typeInfo.BaseType != null) + ////{ + //// result = result.Union(GetSkipVisibilityChecksRequirements(typeInfo.BaseType.GetTypeInfo())); + ////} + + ////foreach (var iface in typeInfo.GetInterfaces()) + ////{ + //// result = result.Union(GetSkipVisibilityChecksRequirements(iface.GetTypeInfo())); + ////} + + return result; + } } /// - /// Add an attribute to the dynamic assembly so that the CLR will skip visibility checks - /// for the specified assembly. + /// Add attributes to a dynamic assembly so that the CLR will skip visibility checks + /// for the assemblies with the specified names. /// - /// The assembly to skip visibility checks for. - private void SkipVisibilityChecksFor(Assembly assembly) + /// The names of the assemblies to skip visibility checks for. + internal void SkipVisibilityChecksFor(IEnumerable assemblyNames) { - Requires.NotNull(assembly, nameof(assembly)); - var assemblyName = assembly.GetName(); - this.SkipVisibilityChecksFor(assemblyName); + Requires.NotNull(assemblyNames, nameof(assemblyNames)); + + foreach (var assemblyName in assemblyNames) + { + this.SkipVisibilityChecksFor(assemblyName); + } } /// - /// Add an attribute to the dynamic assembly so that the CLR will skip visibility checks + /// Add an attribute to a dynamic assembly so that the CLR will skip visibility checks /// for the assembly with the specified name. /// /// The name of the assembly to skip visibility checks for. - private void SkipVisibilityChecksFor(AssemblyName assemblyName) + internal void SkipVisibilityChecksFor(AssemblyName assemblyName) { Requires.NotNull(assemblyName, nameof(assemblyName)); diff --git a/src/tests/Microsoft.VisualStudio.Composition.Tests/ExportMetadataTests.cs b/src/tests/Microsoft.VisualStudio.Composition.Tests/ExportMetadataTests.cs index 0a102eda3..eaa8d2c10 100644 --- a/src/tests/Microsoft.VisualStudio.Composition.Tests/ExportMetadataTests.cs +++ b/src/tests/Microsoft.VisualStudio.Composition.Tests/ExportMetadataTests.cs @@ -130,6 +130,17 @@ public void ImportManyWithInternalMetadataInterface(IContainer container) Assert.IsType(importingPart.ImportingProperty.Single().Value); } + /// + /// Verifies that we can skip visibility checks for more than one assembly. + /// + [Trait("Access", "NonPublic")] + [MefFact(CompositionEngines.V3EmulatingV1, typeof(ImportManyPartWithInternalMetadataInterface), typeof(ImportManyPartWithInternalMetadataInterface2), typeof(PartWithExportMetadata))] + public void InternalMetadataViewInterfacesAcrossMultipleAssemblies(IContainer container) + { + container.GetExportedValue(); + container.GetExportedValue(); + } + [MefFact(CompositionEngines.Unspecified, typeof(ImportingPartWithMetadataInterface), typeof(PartWithExportMetadata))] [Trait("Efficiency", "InstanceReuse")] public void MetadataViewInterfaceInstanceSharedAcrossImports(IContainer container) @@ -645,6 +656,14 @@ public class ImportManyPartWithInternalMetadataInterface internal IEnumerable> ImportingProperty { get; set; } } + [MefV1.Export, MefV1.PartCreationPolicy(MefV1.CreationPolicy.NonShared)] + [Export] + public class ImportManyPartWithInternalMetadataInterface2 + { + [ImportMany, MefV1.ImportMany] + internal IEnumerable> ImportingProperty { get; set; } + } + [MefV1.Export, MefV1.PartCreationPolicy(MefV1.CreationPolicy.NonShared)] [Export] public class ImportManyPartWithMetadataClass