From 91e2ab8e17889906c284df100e6284d58a9518d0 Mon Sep 17 00:00:00 2001 From: MapleWheels Date: Sun, 22 Oct 2023 16:13:46 -0400 Subject: [PATCH] - Fixed NRE in TryBeginDispose - Made `OnException` event useful. - Added some null checks where expected. - Fixed overridden Unload not being called. - Removed partial from AssemblyManager.cs - Made ClearTypesList() actually work. - Made exception details show in console on release builds. - Made content package name show on plugin load. - Made execution standard instead of none for autogenerated and erroneous RunConfigs. --- .../LuaCs/Plugins/AssemblyManager.cs | 82 ++++++++++++------- .../LuaCs/Plugins/CsPackageManager.cs | 43 +++++----- .../MemoryFileAssemblyContextLoader.cs | 25 +++--- .../SharedSource/LuaCs/Plugins/RunConfig.cs | 16 ++-- 4 files changed, 91 insertions(+), 75 deletions(-) diff --git a/Barotrauma/BarotraumaShared/SharedSource/LuaCs/Plugins/AssemblyManager.cs b/Barotrauma/BarotraumaShared/SharedSource/LuaCs/Plugins/AssemblyManager.cs index 28821efdd7..09133aad60 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/LuaCs/Plugins/AssemblyManager.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/LuaCs/Plugins/AssemblyManager.cs @@ -25,7 +25,7 @@ namespace Barotrauma; /// Provides functionality for the loading, unloading and management of plugins implementing IAssemblyPlugin. /// All plugins are loaded into their own AssemblyLoadContext along with their dependencies. /// -public partial class AssemblyManager +public class AssemblyManager { #region ExternalAPI @@ -143,16 +143,23 @@ public IEnumerable GetSubTypesInLoadedAssemblies(bool rebuildList) { if (!_subTypesLookupCache.TryAdd(typeName, list1)) { - ModUtils.Logging.PrintError($"{nameof(AssemblyManager)}: Unable to add subtypes to cache of type {typeName}!"); + ModUtils.Logging.PrintError( + $"{nameof(AssemblyManager)}: Unable to add subtypes to cache of type {typeName}!"); } } else { - ModUtils.Logging.PrintMessage($"{nameof(AssemblyManager)}: Warning: No types found during search for subtypes of {typeName}"); + ModUtils.Logging.PrintMessage( + $"{nameof(AssemblyManager)}: Warning: No types found during search for subtypes of {typeName}"); } return list1; } + catch (Exception e) + { + this.OnException?.Invoke($"{nameof(AssemblyManager)}::{nameof(GetSubTypesInLoadedAssemblies)}() | Error: {e.Message}", e); + return ImmutableList.Empty; + } finally { OpsLockLoaded.ExitReadLock(); @@ -187,7 +194,6 @@ public bool TryGetSubTypesFromACL(Guid id, out IEnumerable types) /// /// /// - /// /// public bool TryGetSubTypesFromACL(Guid id, out IEnumerable types) { @@ -206,7 +212,7 @@ public bool TryGetSubTypesFromACL(Guid id, out IEnumerable types) /// Allows iteration over all types, including interfaces, in all loaded assemblies in the AsmMgr who's names match the string. /// Note: Will return the by-reference equivalent type if the type name is prefixed with "out " or "ref ". /// - /// The string name of the type to search for. + /// The string name of the type to search for. /// An Enumerator for matching types. List will be empty if bad params are supplied. public IEnumerable GetTypesByName(string typeName) { @@ -243,12 +249,19 @@ public IEnumerable GetTypesByName(string typeName) types.Add(byRef ? t.MakeByRefType() : t); return types; } - + foreach (var assembly in AppDomain.CurrentDomain.GetAssemblies()) { - t = assembly.GetType(typeName, false, false); - if (t is not null) - types.Add(byRef ? t.MakeByRefType() : t); + try + { + t = assembly.GetType(typeName, false, false); + if (t is not null) + types.Add(byRef ? t.MakeByRefType() : t); + } + catch (Exception e) + { + this.OnException?.Invoke($"{nameof(AssemblyManager)}::{nameof(GetTypesByName)}() | Error: {e.Message}", e); + } } } finally @@ -316,9 +329,9 @@ public IEnumerable GetAllTypesInLoadedAssemblies() /// public IEnumerable GetAllLoadedACLs() { + OpsLockLoaded.EnterReadLock(); try { - OpsLockLoaded.EnterReadLock(); return LoadedACLs.Select(kvp => kvp.Value).ToImmutableList(); } finally @@ -360,6 +373,9 @@ public AssemblyLoadingSuccessState LoadAssemblyFromMemory([NotNull] string compi // validation if (compiledAssemblyName.IsNullOrWhiteSpace()) return AssemblyLoadingSuccessState.BadName; + + if (syntaxTree is null) + return AssemblyLoadingSuccessState.InvalidAssembly; if (!GetOrCreateACL(id, friendlyName, out var acl)) return AssemblyLoadingSuccessState.ACLLoadFailure; @@ -419,8 +435,10 @@ public AssemblyLoadingSuccessState LoadAssembliesFromLocations([NotNull] IEnumer if (filePaths is null) { - throw new ArgumentNullException( + var exception = new ArgumentNullException( $"{nameof(AssemblyManager)}::{nameof(LoadAssembliesFromLocations)}() | file paths supplied is null!"); + this.OnException?.Invoke($"Error: {exception.Message}", exception); + throw exception; } ImmutableList assemblyFilePaths = filePaths.ToImmutableList(); // copy the list before loading @@ -468,12 +486,15 @@ public bool TryBeginDispose() { if (loadedAcl.Value.Acl is not null) { - foreach (Delegate del in IsReadyToUnloadACL.GetInvocationList()) + if (IsReadyToUnloadACL is not null) { - if (del is System.Func { } func) + foreach (Delegate del in IsReadyToUnloadACL.GetInvocationList()) { - if (!func.Invoke(loadedAcl.Value)) - return false; // Not ready, exit + if (del is System.Func { } func) + { + if (!func.Invoke(loadedAcl.Value)) + return false; // Not ready, exit + } } } @@ -492,9 +513,10 @@ public bool TryBeginDispose() LoadedACLs.Clear(); return true; } - catch + catch(Exception e) { // should never happen + this.OnException?.Invoke($"{nameof(TryBeginDispose)}() | Error: {e.Message}", e); return false; } finally @@ -609,9 +631,9 @@ private bool GetOrCreateACL(Guid id, string friendlyName, out LoadedACL acl) } } - catch + catch(Exception e) { - // should never happen but in-case + this.OnException?.Invoke($"{nameof(GetOrCreateACL)}Error: {e.Message}", e); acl = null; return false; } @@ -648,9 +670,9 @@ private bool DisposeACL(Guid id) return true; } - catch + catch (Exception e) { - // should never happen + this.OnException?.Invoke($"{nameof(DisposeACL)}() | Error: {e.Message}", e); return false; } finally @@ -677,8 +699,9 @@ private void RebuildTypesList() .ToImmutableDictionary(t => t.FullName ?? t.Name, t => t); _subTypesLookupCache.Clear(); } - catch(ArgumentException _) + catch(ArgumentException ae) { + this.OnException?.Invoke($"{nameof(RebuildTypesList)}() | Error: {ae.Message}", ae); try { // some types must've had duplicate type names, build the list while filtering @@ -699,6 +722,7 @@ private void RebuildTypesList() } catch (Exception e) { + this.OnException?.Invoke($"{nameof(RebuildTypesList)}() | Error: {e.Message}", e); ModUtils.Logging.PrintError($"{nameof(AssemblyManager)}: Unable to create list of default assembly types! Default AssemblyLoadContext types searching not available."); #if DEBUG ModUtils.Logging.PrintError($"{nameof(AssemblyManager)}: Exception Details :{e.Message} | {e.InnerException}"); @@ -729,14 +753,14 @@ public sealed class LoadedACL public readonly Guid Id; private ImmutableDictionary _assembliesTypes = ImmutableDictionary.Empty; public readonly MemoryFileAssemblyContextLoader Acl; - private readonly AssemblyManager _manager; internal LoadedACL(Guid id, AssemblyManager manager, string friendlyName) { this.Id = id; - this.Acl = new(manager); - this._manager = manager; - this.Acl.FriendlyName = friendlyName; + this.Acl = new(manager) + { + FriendlyName = friendlyName + }; } public ImmutableDictionary AssembliesTypes => _assembliesTypes; @@ -752,7 +776,7 @@ internal void RebuildTypesList() .SelectMany(a => a.GetSafeTypes()) .ToImmutableDictionary(t => t.FullName ?? t.Name, t => t); } - catch(ArgumentException _) + catch(ArgumentException) { // some types must've had duplicate type names, build the list while filtering Dictionary types = new(); @@ -774,7 +798,7 @@ internal void RebuildTypesList() internal void ClearTypesList() { - _assembliesTypes.Clear(); + _assembliesTypes = ImmutableDictionary.Empty; } } @@ -802,12 +826,12 @@ public static IEnumerable GetSafeTypes(this Assembly assembly) { return re.Types.Where(x => x != null)!; } - catch (InvalidOperationException ioe) + catch (InvalidOperationException) { return new List(); } } - catch (Exception e) + catch (Exception) { return new List(); } diff --git a/Barotrauma/BarotraumaShared/SharedSource/LuaCs/Plugins/CsPackageManager.cs b/Barotrauma/BarotraumaShared/SharedSource/LuaCs/Plugins/CsPackageManager.cs index fb87c17699..f041279e48 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/LuaCs/Plugins/CsPackageManager.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/LuaCs/Plugins/CsPackageManager.cs @@ -11,6 +11,7 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using MonoMod.Utils; +// ReSharper disable InconsistentNaming namespace Barotrauma; @@ -147,12 +148,12 @@ public bool LuaTryRegisterPackageTypes(string name, bool caseSensitive = false) /// /// Whether or not plugins' types have been instantiated. /// - public bool PluginsInitialized { get; private set; } = false; + public bool PluginsInitialized { get; private set; } /// /// Whether or not plugins are fully loaded. /// - public bool PluginsLoaded { get; private set; } = false; + public bool PluginsLoaded { get; private set; } public IEnumerable GetCurrentPackagesByLoadOrder() => _currentPackagesByLoadOrder; @@ -333,7 +334,7 @@ public AssemblyLoadingSuccessState LoadAssemblyPackages() throw new DirectoryNotFoundException("No publicized assemblies found."); } // no directory found, use the other one - catch (DirectoryNotFoundException dne) + catch (DirectoryNotFoundException) { if (_luaCsSetup.Config.PreferToUseWorkshopLuaSetup) { @@ -454,8 +455,7 @@ public AssemblyLoadingSuccessState LoadAssemblyPackages() if (reliableMap && OrderAndFilterPackagesByDependencies( _packagesDependencies, out var readyToLoad, - out var cannotLoadPackages, - null)) + out var cannotLoadPackages)) { packagesToLoadInOrder.AddRange(readyToLoad); if (cannotLoadPackages is not null) @@ -611,21 +611,19 @@ public AssemblyLoadingSuccessState LoadAssemblyPackages() bool ShouldRunPackage(ContentPackage package, RunConfig config) { - if (config.AutoGenerated) - return false; return (!_luaCsSetup.Config.TreatForcedModsAsNormal && config.IsForced()) || (ContentPackageManager.EnabledPackages.All.Contains(package) && config.IsForcedOrStandard()); } - void UpdatePackagesToDisable(ref HashSet list, + void UpdatePackagesToDisable(ref HashSet set, ContentPackage newDisabledPackage, IEnumerable>> dependenciesMap) { - list.Add(newDisabledPackage); + set.Add(newDisabledPackage); foreach (var package in dependenciesMap) { if (package.Value.Contains(newDisabledPackage)) - list.Add(newDisabledPackage); + set.Add(newDisabledPackage); } } } @@ -655,7 +653,7 @@ public void RunPluginsInit() // init foreach (var plugin in contentPlugins.Value) { - TryRun(() => plugin.Initialize(), $"{nameof(IAssemblyPlugin.Initialize)}", plugin.GetType().Name); + TryRun(() => plugin.Initialize(), $"{nameof(IAssemblyPlugin.Initialize)}", $"CP: {_reverseLookupGuidList[contentPlugins.Key].Name} Plugin: {plugin.GetType().Name}"); } } @@ -664,7 +662,7 @@ public void RunPluginsInit() // load complete foreach (var plugin in contentPlugins.Value) { - TryRun(() => plugin.OnLoadCompleted(), $"{nameof(IAssemblyPlugin.OnLoadCompleted)}", plugin.GetType().Name); + TryRun(() => plugin.OnLoadCompleted(), $"{nameof(IAssemblyPlugin.OnLoadCompleted)}", $"CP: {_reverseLookupGuidList[contentPlugins.Key].Name} Plugin: {plugin.GetType().Name}"); } } @@ -698,7 +696,7 @@ public void RunPluginsPreInit() // init foreach (var plugin in contentPlugins.Value) { - TryRun(() => plugin.PreInitPatching(), $"{nameof(IAssemblyPlugin.PreInitPatching)}", plugin.GetType().Name); + TryRun(() => plugin.PreInitPatching(), $"{nameof(IAssemblyPlugin.PreInitPatching)}", $"CP: {_reverseLookupGuidList[contentPlugins.Key].Name} Plugin: {plugin.GetType().Name}"); } } @@ -741,21 +739,20 @@ public void InstantiatePlugins(bool force = false) try { plugin = (IAssemblyPlugin)Activator.CreateInstance(type); + _loadedPlugins[pair.Key].Add(plugin); } catch (Exception e) { ModUtils.Logging.PrintError($"{nameof(CsPackageManager)}: Error while instantiating plugin of type {type}. Now disposing..."); -#if DEBUG ModUtils.Logging.PrintError($"{nameof(CsPackageManager)}: Details: {e.Message} | {e.InnerException}"); -#endif - TryRun(() => plugin?.Dispose(), "Dispose", type.FullName ?? type.Name); - plugin = null; + if (plugin is not null) + { + // ReSharper disable once AccessToModifiedClosure + TryRun(() => plugin?.Dispose(), nameof(IAssemblyPlugin.Dispose), type.FullName ?? type.Name); + plugin = null; + } } - if (plugin is not null) - _loadedPlugins[pair.Key].Add(plugin); - else - ModUtils.Logging.PrintError($"{nameof(CsPackageManager)}: Error while instantiating plugin of type {type}"); } } @@ -772,7 +769,7 @@ public void UnloadPlugins() { foreach (var plugin in contentPlugins.Value) { - TryRun(() => plugin.Dispose(), $"{nameof(IAssemblyPlugin.Dispose)}", plugin.GetType().Name); + TryRun(() => plugin.Dispose(), $"{nameof(IAssemblyPlugin.Dispose)}", $"CP: {_reverseLookupGuidList[contentPlugins.Key].Name} Plugin: {plugin.GetType().Name}"); } contentPlugins.Value.Clear(); } @@ -816,9 +813,7 @@ private void TryRun(Action action, string messageMethodName, string messageTypeN catch (Exception e) { ModUtils.Logging.PrintError($"{nameof(CsPackageManager)}: Error while running {messageMethodName}() on plugin of type {messageTypeName}"); -#if DEBUG ModUtils.Logging.PrintError($"{nameof(CsPackageManager)}: Details: {e.Message} | {e.InnerException}"); -#endif } } diff --git a/Barotrauma/BarotraumaShared/SharedSource/LuaCs/Plugins/MemoryFileAssemblyContextLoader.cs b/Barotrauma/BarotraumaShared/SharedSource/LuaCs/Plugins/MemoryFileAssemblyContextLoader.cs index b4bbd5a6a4..7dde35577c 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/LuaCs/Plugins/MemoryFileAssemblyContextLoader.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/LuaCs/Plugins/MemoryFileAssemblyContextLoader.cs @@ -5,13 +5,10 @@ using System.IO; using System.Linq; using System.Reflection; -using System.Runtime.CompilerServices; using System.Runtime.Loader; -using System.Threading; -using Barotrauma; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.Emit; +// ReSharper disable ConditionIsAlwaysTrueOrFalse namespace Barotrauma; @@ -23,16 +20,16 @@ namespace Barotrauma; public class MemoryFileAssemblyContextLoader : AssemblyLoadContext { // public - public string FriendlyName { get; set; } = null; + public string FriendlyName { get; set; } // ReSharper disable MemberCanBePrivate.Global - public Assembly CompiledAssembly { get; private set; } = null; - public byte[] CompiledAssemblyImage { get; private set; } = null; + public Assembly CompiledAssembly { get; private set; } + public byte[] CompiledAssemblyImage { get; private set; } // ReSharper restore MemberCanBePrivate.Global // internal private readonly Dictionary _dependencyResolvers = new(); // path-folder, resolver protected bool IsResolving; //this is to avoid circular dependency lookup. private AssemblyManager _assemblyManager; - public bool IsTemplateMode { get; set; } = false; + public bool IsTemplateMode { get; set; } public MemoryFileAssemblyContextLoader(AssemblyManager assemblyManager) : base(isCollectible: true) { @@ -73,23 +70,23 @@ public AssemblyLoadingSuccessState LoadFromFiles([NotNull] IEnumerable a LoadFromAssemblyPath(sanitizedFilePath); } // on fail of any we're done because we assume that loaded files are related. This ACL needs to be unloaded and collected. - catch (ArgumentNullException ane) + catch (ArgumentNullException) { return AssemblyLoadingSuccessState.BadFilePath; } - catch (ArgumentException ae) + catch (ArgumentException) { return AssemblyLoadingSuccessState.BadFilePath; } - catch (FileLoadException fle) + catch (FileLoadException) { return AssemblyLoadingSuccessState.CannotLoadFile; } - catch (FileNotFoundException fne) + catch (FileNotFoundException) { return AssemblyLoadingSuccessState.NoAssemblyFound; } - catch (BadImageFormatException bfe) + catch (BadImageFormatException) { return AssemblyLoadingSuccessState.InvalidAssembly; } @@ -281,7 +278,7 @@ protected override Assembly Load(AssemblyName assemblyName) } - private new void Unload() + public new void Unload() { CompiledAssembly = null; CompiledAssemblyImage = null; diff --git a/Barotrauma/BarotraumaShared/SharedSource/LuaCs/Plugins/RunConfig.cs b/Barotrauma/BarotraumaShared/SharedSource/LuaCs/Plugins/RunConfig.cs index d954dca524..7ca448edd8 100644 --- a/Barotrauma/BarotraumaShared/SharedSource/LuaCs/Plugins/RunConfig.cs +++ b/Barotrauma/BarotraumaShared/SharedSource/LuaCs/Plugins/RunConfig.cs @@ -32,7 +32,7 @@ public RunConfig(bool autoGenerated) this.AutoGenerated = autoGenerated; if (autoGenerated) { - (Client, Server) = ("None", "None"); + (Client, Server) = ("Standard", "Standard"); } } @@ -60,18 +60,18 @@ public RunConfig Sanitize() { Client = SanitizeRunSetting(Client); } - catch (Exception e) + catch (Exception) { - Client = "None"; + Client = "Standard"; } try { Server = SanitizeRunSetting(Server); } - catch (Exception e) + catch (Exception) { - Server = "None"; + Server = "Standard"; } Dependencies ??= new RunConfig.Dependency[] { }; @@ -79,9 +79,9 @@ public RunConfig Sanitize() static string SanitizeRunSetting(string str) => str switch { - null => "None", - "" => "None", - " " => "None", + null => "Standard", + "" => "Standard", + " " => "Standard", _ => str[0].ToString().ToUpper() + str.Substring(1).ToLower() };