Minor adjustments based on PR feedback

natemcmaster · Dec 28, 2020 · 9fd191c · 9fd191c
1 parent ae7699d
commit 9fd191c
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 30 deletions.
diff --git a/src/Plugins/Loader/AssemblyLoadContextBuilder.cs b/src/Plugins/Loader/AssemblyLoadContextBuilder.cs
@@ -218,6 +218,11 @@ public AssemblyLoadContextBuilder PreferDefaultLoadContext(bool preferDefaultLoa
         /// Instructs the load context to lazy load dependencies of all shared assemblies.
         /// Reduces plugin load time at the expense of non-determinism in how transitive dependencies are loaded
         /// between the plugin and the host.
+        ///
+        /// Please be aware of the danger of using this option:
+        /// <seealso href="https://github.com/natemcmaster/DotNetCorePlugins/pull/164#issuecomment-751557873">
+        /// https://github.com/natemcmaster/DotNetCorePlugins/pull/164#issuecomment-751557873
+        /// </seealso>
         /// </summary>
         /// <param name="isLazyLoaded">True to lazy load, else false.</param>
         /// <returns>The builder.</returns>

diff --git a/src/Plugins/Loader/ManagedLoadContext.cs b/src/Plugins/Loader/ManagedLoadContext.cs
@@ -41,7 +41,7 @@ internal class ManagedLoadContext : AssemblyLoadContext
             IReadOnlyDictionary<string, ManagedLibrary> managedAssemblies,
             IReadOnlyDictionary<string, NativeLibrary> nativeLibraries,
             IReadOnlyCollection<string> privateAssemblies,
-            ICollection<string> defaultAssemblies,
+            IReadOnlyCollection<string> defaultAssemblies,
             IReadOnlyCollection<string> additionalProbingPaths,
             IReadOnlyCollection<string> resourceProbingPaths,
             AssemblyLoadContext defaultLoadContext,
@@ -66,7 +66,7 @@ internal class ManagedLoadContext : AssemblyLoadContext
             _basePath = Path.GetDirectoryName(mainAssemblyPath) ?? throw new ArgumentException(nameof(mainAssemblyPath));
             _managedAssemblies = managedAssemblies ?? throw new ArgumentNullException(nameof(managedAssemblies));
             _privateAssemblies = privateAssemblies ?? throw new ArgumentNullException(nameof(privateAssemblies));
-            _defaultAssemblies = defaultAssemblies ?? throw new ArgumentNullException(nameof(defaultAssemblies));
+            _defaultAssemblies = defaultAssemblies != null ? defaultAssemblies.ToList() : throw new ArgumentNullException(nameof(defaultAssemblies));
             _nativeLibraries = nativeLibraries ?? throw new ArgumentNullException(nameof(nativeLibraries));
             _additionalProbingPaths = additionalProbingPaths ?? throw new ArgumentNullException(nameof(additionalProbingPaths));
             _defaultLoadContext = defaultLoadContext;

diff --git a/src/Plugins/PluginConfig.cs b/src/Plugins/PluginConfig.cs
@@ -66,6 +66,11 @@ public PluginConfig(string mainAssemblyPath)
         /// If enabled, will lazy load dependencies of all shared assemblies.
         /// Reduces plugin load time at the expense of non-determinism in how transitive dependencies are loaded
         /// between the plugin and the host.
+        ///
+        /// Please be aware of the danger of using this option:
+        /// <seealso href="https://github.com/natemcmaster/DotNetCorePlugins/pull/164#issuecomment-751557873">
+        /// https://github.com/natemcmaster/DotNetCorePlugins/pull/164#issuecomment-751557873
+        /// </seealso>
         /// </summary>
         public bool IsLazyLoaded { get; set; } = false;
 

diff --git a/test/Plugins.Tests/SharedTypesTests.cs b/test/Plugins.Tests/SharedTypesTests.cs
@@ -40,25 +40,17 @@ public void PluginsCanForceSharedTypes()
         /// accounted for. Without this, the order in which code loads
         /// could cause different assembly versions to be loaded.
         /// </summary>
-        [Fact]
-        public void TransitiveAssembliesOfSharedTypesAreResolved()
-        {
-            using var loader = PluginLoader.CreateFromAssemblyFile(
-                    TestResources.GetTestProjectAssembly("TransitivePlugin"),
-                    sharedTypes: new[] { typeof(SharedType) });
-
-            TransitiveAssembliesOfSharedTypesAreResolvedBase(loader);
-        }
-
-        /// <summary>
-        /// Variant of <see cref="TransitiveAssembliesOfSharedTypesAreResolved"/> which uses lazy loading for assemblies.
-        /// </summary>
-        [Fact]
-        public void LazyLoadedTransitiveAssembliesOfSharedTypesAreResolved()
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void TransitiveAssembliesOfSharedTypesAreResolved(bool isLazyLoaded)
         {
-            using var loader = PluginLoader.CreateFromAssemblyFile(TestResources.GetTestProjectAssembly("TransitivePlugin"), sharedTypes: new[] { typeof(SharedType) }, config => config.IsLazyLoaded = true);
-
-            TransitiveAssembliesOfSharedTypesAreResolvedBase(loader);
+            using var loader = PluginLoader.CreateFromAssemblyFile(TestResources.GetTestProjectAssembly("TransitivePlugin"), sharedTypes: new[] { typeof(SharedType) }, config => config.IsLazyLoaded = isLazyLoaded);
+            var assembly = loader.LoadDefaultAssembly();
+            var configType = assembly.GetType("TransitivePlugin.PluginConfig", throwOnError: true)!;
+            var config = Activator.CreateInstance(configType);
+            var transitiveInstance = configType.GetMethod("GetTransitiveType")?.Invoke(config, null);
+            Assert.IsType<Test.Transitive.TransitiveSharedType>(transitiveInstance);
         }
 
         /// <summary>
@@ -72,7 +64,7 @@ public void LazyLoadedTransitiveAssembliesOfSharedTypesAreResolved()
         /// </summary>
         [Fact]
         public void NonDefaultLoadContextsAreSupported()
-        {
+        {
             /* The loaded plugin here will be in its own ALC.
              * It will load its own plugins, which are not known to this ALC.
              * Then this ALC will ask that ALC if it managed to successfully its own plugins.
@@ -109,14 +101,5 @@ public void NonDefaultLoadContextsAreSupported()
             var callingContext = AssemblyLoadContext.GetLoadContext(Assembly.GetExecutingAssembly());
             Assert.True(config?.TryLoadPluginsInCustomContext(callingContext));
         }
-
-        private void TransitiveAssembliesOfSharedTypesAreResolvedBase(PluginLoader loader)
-        {
-            var assembly = loader.LoadDefaultAssembly();
-            var configType = assembly.GetType("TransitivePlugin.PluginConfig", throwOnError: true)!;
-            var config = Activator.CreateInstance(configType);
-            var transitiveInstance = configType.GetMethod("GetTransitiveType")?.Invoke(config, null);
-            Assert.IsType<Test.Transitive.TransitiveSharedType>(transitiveInstance);
-        }
     }
 }