Keep Plugins on Stack, Fix Potential Crash, GC on App Page Exit

Reloaded-Project · Dec 10, 2019 · 6b07cc9 · 6b07cc9
1 parent 07dde1d
commit 6b07cc9
Show file tree

Hide file tree

Showing 4 changed files with 68 additions and 37 deletions.
diff --git a/Source/Reloaded.Mod.Launcher/Commands/ApplicationConfigurationPage/ConfigureModCommand.cs b/Source/Reloaded.Mod.Launcher/Commands/ApplicationConfigurationPage/ConfigureModCommand.cs
@@ -4,26 +4,26 @@
 using System.ComponentModel;
 using System.IO;
 using System.Linq;
+using System.Runtime.CompilerServices;
 using System.Text;
 using System.Windows.Input;
 using McMaster.NETCore.Plugins;
 using Reloaded.Mod.Interfaces;
 using Reloaded.Mod.Interfaces.Internal;
 using Reloaded.Mod.Launcher.Commands.Templates;
+using Reloaded.Mod.Launcher.Models.Model;
 using Reloaded.Mod.Launcher.Models.ViewModel.ApplicationSubPages;
 using Reloaded.Mod.Launcher.Pages.Dialogs;
 using Reloaded.Mod.Loader.IO.Config;
+using Reloaded.Mod.Loader.IO.Structs;
 using MessageBox = Xceed.Wpf.Toolkit.MessageBox;
 
 namespace Reloaded.Mod.Launcher.Commands.ApplicationConfigurationPage
 {
     public class ConfigureModCommand : WithCanExecuteChanged, ICommand, IDisposable
     {
         private static Type[] _sharedTypes = { typeof(IConfigurator) };
-
         private readonly ApplicationSummaryViewModel _summaryViewModel;
-        private PluginLoader _loader;
-        private IConfigurator _configurator;
 
         public ConfigureModCommand(ApplicationSummaryViewModel summaryViewModel)
         {
@@ -39,10 +39,9 @@ public ConfigureModCommand(ApplicationSummaryViewModel summaryViewModel)
 
         public void Dispose()
         {
-            _configurator = null;
-            _loader?.Dispose();
-            _summaryViewModel.PropertyChanged -= SummaryViewModelOnPropertyChanged;
             GC.Collect();
+            GC.WaitForPendingFinalizers();
+            _summaryViewModel.PropertyChanged -= SummaryViewModelOnPropertyChanged;
         }
 
         /* Implementation */
@@ -54,51 +53,59 @@ private void SummaryViewModelOnPropertyChanged(object sender, PropertyChangedEve
 
         /* ICommand */
 
+        // Disallowed inlining to ensure nothing from library can be kept alive by stack references etc.
+        [MethodImpl(MethodImplOptions.NoInlining)]
         public void Execute(object parameter)
         {
-            if (!_configurator.TryRunCustomConfiguration())
+            // Important Note: We are keeping everything to the stack.
+            // Want our best to ensure that no types leak out anywhere making unloadability hard.
+            // Also, we must also keep loader used to load the configurator in stack, for obvious reasons.
+            if (TryGetConfigurator(_summaryViewModel.SelectedMod, out var configurator, out var loader))
             {
-                var window = new ConfigureModDialog(_configurator.GetConfigurations());
-                window.ShowDialog();
+                if (!configurator.TryRunCustomConfiguration())
+                {
+                    var window = new ConfigureModDialog(configurator.GetConfigurations());
+                    window.ShowDialog();
+                }
             }
         }
 
+        // Disallowed inlining to ensure nothing from library can be kept alive by stack references etc.
+        [MethodImpl(MethodImplOptions.NoInlining)]
         public bool CanExecute(object parameter)
         {
-            DisposeCurrent();
             var selectedMod = _summaryViewModel.SelectedMod;
             if (selectedMod != null)
-            {
-                var config = selectedMod.Generic.ModConfig;
-
-                string dllPath = ModConfig.GetDllPath(selectedMod.Generic.ModConfigPath, (ModConfig) config);
-                if (! File.Exists(dllPath))
-                    return false;
+                return TryGetConfigurator(selectedMod, out _, out _);
 
-                _loader = PluginLoader.CreateFromAssemblyFile(dllPath, true, _sharedTypes);
-                var assembly    = _loader.LoadDefaultAssembly();
-                var types       = assembly.GetTypes();
-                var entryPoint  = types.FirstOrDefault(t => typeof(IConfigurator).IsAssignableFrom(t) && !t.IsAbstract);
+            return false;
+        }
 
-                if (entryPoint != null)
-                {
-                    _configurator = (IConfigurator) Activator.CreateInstance(entryPoint);
-                    _configurator.SetModDirectory(Path.GetFullPath(Path.GetDirectoryName(selectedMod.Generic.ModConfigPath)));
-                    return true;
-                }
+        // Disallowed inlining to ensure nothing from library can be kept alive by stack references etc.
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private bool TryGetConfigurator(BooleanGenericTuple<ImageModPathTuple> selectedMod, out IConfigurator configurator, out PluginLoader loader)
+        {
+            var config = selectedMod.Generic.ModConfig;
+            string dllPath = ModConfig.GetDllPath(selectedMod.Generic.ModConfigPath, (ModConfig)config);
+            configurator = null;
+            loader = null;
 
-                _loader?.Dispose();
+            if (!File.Exists(dllPath))
                 return false;
+
+            loader = PluginLoader.CreateFromAssemblyFile(dllPath, true, _sharedTypes);
+            var assembly = loader.LoadDefaultAssembly();
+            var types = assembly.GetTypes();
+            var entryPoint = types.FirstOrDefault(t => typeof(IConfigurator).IsAssignableFrom(t) && !t.IsAbstract);
+
+            if (entryPoint != null)
+            {
+                configurator = (IConfigurator)Activator.CreateInstance(entryPoint);
+                configurator.SetModDirectory(Path.GetFullPath(Path.GetDirectoryName(selectedMod.Generic.ModConfigPath)));
+                return true;
             }
 
             return false;
         }
-
-        private void DisposeCurrent()
-        {
-            _configurator = null;
-            _loader?.Dispose();
-            GC.Collect();
-        }
     }
 }
diff --git a/...Reloaded.Mod.Launcher/Models/ViewModel/ApplicationSubPages/ApplicationSummaryViewModel.cs b/...Reloaded.Mod.Launcher/Models/ViewModel/ApplicationSubPages/ApplicationSummaryViewModel.cs
@@ -14,7 +14,7 @@
 
 namespace Reloaded.Mod.Launcher.Models.ViewModel.ApplicationSubPages
 {
-    public class ApplicationSummaryViewModel : ObservableObject
+    public class ApplicationSummaryViewModel : ObservableObject, IDisposable
     {
         public ObservableCollection<BooleanGenericTuple<ImageModPathTuple>> AllMods { get; set; }
         public BooleanGenericTuple<ImageModPathTuple> SelectedMod { get; set; }
@@ -41,6 +41,18 @@ public ApplicationSummaryViewModel(ApplicationViewModel model)
             AllMods.CollectionChanged += (sender, args) => SaveApplication(); // Save on reorder.
         }
 
+        ~ApplicationSummaryViewModel()
+        {
+            Dispose();
+        }
+
+        public void Dispose()
+        {
+            OpenModFolderCommand?.Dispose();
+            ConfigureModCommand?.Dispose();
+            GC.SuppressFinalize(this);
+        }
+
         private void OnSelectedModChanged(object sender, PropertyChangedEventArgs e)
         {
             if (e.PropertyName == nameof(SelectedMod))

diff --git a/...loaded.Mod.Launcher/Pages/BaseSubpages/ApplicationSubPages/ApplicationSummaryPage.xaml.cs b/...loaded.Mod.Launcher/Pages/BaseSubpages/ApplicationSubPages/ApplicationSummaryPage.xaml.cs
@@ -10,7 +10,7 @@ namespace Reloaded.Mod.Launcher.Pages.BaseSubpages.ApplicationSubPages
     /// <summary>
     /// Interaction logic for ApplicationSummaryPage.xaml
     /// </summary>
-    public partial class ApplicationSummaryPage : ApplicationSubPage
+    public partial class ApplicationSummaryPage : ApplicationSubPage, IDisposable
     {
         public ApplicationSummaryViewModel ViewModel { get; set; }
         private readonly ResourceManipulator _manipulator;
@@ -24,6 +24,18 @@ public ApplicationSummaryPage()
             _manipulator = new ResourceManipulator(this.Contents);
             _modsViewSource = _manipulator.Get<CollectionViewSource>("FilteredMods");
             _modsViewSource.Filter += ModsViewSourceOnFilter;
+            this.AnimateOutFinished += Dispose;
+        }
+
+        ~ApplicationSummaryPage()
+        {
+            Dispose();
+        }
+
+        public void Dispose()
+        {
+            ViewModel?.Dispose();
+            GC.SuppressFinalize(this);
         }
 
         private void ModsViewSourceOnFilter(object sender, FilterEventArgs e)

diff --git a/Source/Reloaded.Mod.Launcher/Reloaded.Mod.Launcher.csproj b/Source/Reloaded.Mod.Launcher/Reloaded.Mod.Launcher.csproj
@@ -6,7 +6,7 @@
     <UseWPF>true</UseWPF>
     <AssemblyName>Reloaded-II</AssemblyName>
     <RootNamespace>Reloaded.Mod.Launcher</RootNamespace>
-    <Version>1.3.1</Version>
+    <Version>1.3.2</Version>
     <Copyright>Sewer56 ~ $([System.DateTime]::UtcNow.Year) | Build: $([System.DateTime]::UtcNow.ToString("s"))</Copyright>
     <AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
     <ApplicationIcon>appicon.ico</ApplicationIcon>