From 219696275df054d25cd385f950eb01ee33312e76 Mon Sep 17 00:00:00 2001 From: Jesse Plamondon-Willard Date: Sat, 11 Jan 2020 13:20:37 -0500 Subject: fix errors due to async threads creating content managers --- src/SMAPI/Framework/ContentCoordinator.cs | 113 ++++++++++++++++++------------ src/SMAPI/Framework/InternalExtensions.cs | 70 ++++++++++++++++++ 2 files changed, 139 insertions(+), 44 deletions(-) (limited to 'src') diff --git a/src/SMAPI/Framework/ContentCoordinator.cs b/src/SMAPI/Framework/ContentCoordinator.cs index 82d3805b..0e62837c 100644 --- a/src/SMAPI/Framework/ContentCoordinator.cs +++ b/src/SMAPI/Framework/ContentCoordinator.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Globalization; using System.IO; using System.Linq; +using System.Threading; using Microsoft.Xna.Framework.Content; using StardewModdingAPI.Framework.Content; using StardewModdingAPI.Framework.ContentManagers; @@ -48,6 +49,10 @@ namespace StardewModdingAPI.Framework /// Whether the content coordinator has been disposed. private bool IsDisposed; + /// A lock used to prevent asynchronous changes to the content manager list. + /// The game may adds content managers in asynchronous threads (e.g. when populating the load screen). + private readonly ReaderWriterLockSlim ContentManagerLock = new ReaderWriterLockSlim(); + /********* ** Accessors @@ -96,9 +101,12 @@ namespace StardewModdingAPI.Framework /// A name for the mod manager. Not guaranteed to be unique. public GameContentManager CreateGameContentManager(string name) { - GameContentManager manager = new GameContentManager(name, this.MainContentManager.ServiceProvider, this.MainContentManager.RootDirectory, this.MainContentManager.CurrentCulture, this, this.Monitor, this.Reflection, this.OnDisposing, this.OnLoadingFirstAsset); - this.ContentManagers.Add(manager); - return manager; + return this.ContentManagerLock.InWriteLock(() => + { + GameContentManager manager = new GameContentManager(name, this.MainContentManager.ServiceProvider, this.MainContentManager.RootDirectory, this.MainContentManager.CurrentCulture, this, this.Monitor, this.Reflection, this.OnDisposing, this.OnLoadingFirstAsset); + this.ContentManagers.Add(manager); + return manager; + }); } /// Get a new content manager which handles reading files from a SMAPI mod folder with support for unpacked files. @@ -107,20 +115,23 @@ namespace StardewModdingAPI.Framework /// The game content manager used for map tilesheets not provided by the mod. public ModContentManager CreateModContentManager(string name, string rootDirectory, IContentManager gameContentManager) { - ModContentManager manager = new ModContentManager( - name: name, - gameContentManager: gameContentManager, - serviceProvider: this.MainContentManager.ServiceProvider, - rootDirectory: rootDirectory, - currentCulture: this.MainContentManager.CurrentCulture, - coordinator: this, - monitor: this.Monitor, - reflection: this.Reflection, - jsonHelper: this.JsonHelper, - onDisposing: this.OnDisposing - ); - this.ContentManagers.Add(manager); - return manager; + return this.ContentManagerLock.InWriteLock(() => + { + ModContentManager manager = new ModContentManager( + name: name, + gameContentManager: gameContentManager, + serviceProvider: this.MainContentManager.ServiceProvider, + rootDirectory: rootDirectory, + currentCulture: this.MainContentManager.CurrentCulture, + coordinator: this, + monitor: this.Monitor, + reflection: this.Reflection, + jsonHelper: this.JsonHelper, + onDisposing: this.OnDisposing + ); + this.ContentManagers.Add(manager); + return manager; + }); } /// Get the current content locale. @@ -132,8 +143,11 @@ namespace StardewModdingAPI.Framework /// Perform any cleanup needed when the locale changes. public void OnLocaleChanged() { - foreach (IContentManager contentManager in this.ContentManagers) - contentManager.OnLocaleChanged(); + this.ContentManagerLock.InReadLock(() => + { + foreach (IContentManager contentManager in this.ContentManagers) + contentManager.OnLocaleChanged(); + }); } /// Get whether this asset is mapped to a mod folder. @@ -179,13 +193,16 @@ namespace StardewModdingAPI.Framework /// The internal SMAPI asset key. public T LoadManagedAsset(string contentManagerID, string relativePath) { - // get content manager - IContentManager contentManager = this.ContentManagers.FirstOrDefault(p => p.IsNamespaced && p.Name == contentManagerID); - if (contentManager == null) - throw new InvalidOperationException($"The '{contentManagerID}' prefix isn't handled by any mod."); - - // get fresh asset - return contentManager.Load(relativePath, this.DefaultLanguage, useCache: false); + return this.ContentManagerLock.InReadLock(() => + { + // get content manager + IContentManager contentManager = this.ContentManagers.FirstOrDefault(p => p.IsNamespaced && p.Name == contentManagerID); + if (contentManager == null) + throw new InvalidOperationException($"The '{contentManagerID}' prefix isn't handled by any mod."); + + // get fresh asset + return contentManager.Load(relativePath, this.DefaultLanguage, useCache: false); + }); } /// Purge matched assets from the cache. @@ -208,28 +225,31 @@ namespace StardewModdingAPI.Framework /// Returns the invalidated asset names. public IEnumerable InvalidateCache(Func predicate, bool dispose = false) { - // invalidate cache & track removed assets - IDictionary> removedAssets = new Dictionary>(StringComparer.InvariantCultureIgnoreCase); - foreach (IContentManager contentManager in this.ContentManagers) + return this.ContentManagerLock.InReadLock(() => { - foreach (var entry in contentManager.InvalidateCache(predicate, dispose)) + // invalidate cache & track removed assets + IDictionary> removedAssets = new Dictionary>(StringComparer.InvariantCultureIgnoreCase); + foreach (IContentManager contentManager in this.ContentManagers) { - if (!removedAssets.TryGetValue(entry.Key, out ISet assets)) - removedAssets[entry.Key] = assets = new HashSet(new ObjectReferenceComparer()); - assets.Add(entry.Value); + foreach (var entry in contentManager.InvalidateCache(predicate, dispose)) + { + if (!removedAssets.TryGetValue(entry.Key, out ISet assets)) + removedAssets[entry.Key] = assets = new HashSet(new ObjectReferenceComparer()); + assets.Add(entry.Value); + } } - } - // reload core game assets - if (removedAssets.Any()) - { - IDictionary propagated = this.CoreAssets.Propagate(this.MainContentManager, removedAssets.ToDictionary(p => p.Key, p => p.Value.First().GetType())); // use an intercepted content manager - this.Monitor.Log($"Invalidated {removedAssets.Count} asset names ({string.Join(", ", removedAssets.Keys.OrderBy(p => p, StringComparer.InvariantCultureIgnoreCase))}); propagated {propagated.Count(p => p.Value)} core assets.", LogLevel.Trace); - } - else - this.Monitor.Log("Invalidated 0 cache entries.", LogLevel.Trace); + // reload core game assets + if (removedAssets.Any()) + { + IDictionary propagated = this.CoreAssets.Propagate(this.MainContentManager, removedAssets.ToDictionary(p => p.Key, p => p.Value.First().GetType())); // use an intercepted content manager + this.Monitor.Log($"Invalidated {removedAssets.Count} asset names ({string.Join(", ", removedAssets.Keys.OrderBy(p => p, StringComparer.InvariantCultureIgnoreCase))}); propagated {propagated.Count(p => p.Value)} core assets.", LogLevel.Trace); + } + else + this.Monitor.Log("Invalidated 0 cache entries.", LogLevel.Trace); - return removedAssets.Keys; + return removedAssets.Keys; + }); } /// Dispose held resources. @@ -244,6 +264,8 @@ namespace StardewModdingAPI.Framework contentManager.Dispose(); this.ContentManagers.Clear(); this.MainContentManager = null; + + this.ContentManagerLock.Dispose(); } @@ -257,7 +279,10 @@ namespace StardewModdingAPI.Framework if (this.IsDisposed) return; - this.ContentManagers.Remove(contentManager); + this.ContentManagerLock.InWriteLock(() => + { + this.ContentManagers.Remove(contentManager); + }); } } } diff --git a/src/SMAPI/Framework/InternalExtensions.cs b/src/SMAPI/Framework/InternalExtensions.cs index c3155b1c..8b45e196 100644 --- a/src/SMAPI/Framework/InternalExtensions.cs +++ b/src/SMAPI/Framework/InternalExtensions.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Reflection; +using System.Threading; using Microsoft.Xna.Framework.Graphics; using StardewModdingAPI.Framework.Events; using StardewModdingAPI.Framework.Reflection; @@ -83,6 +84,75 @@ namespace StardewModdingAPI.Framework return exception; } + /**** + ** ReaderWriterLockSlim + ****/ + /// Run code within a read lock. + /// The lock to set. + /// The action to perform. + public static void InReadLock(this ReaderWriterLockSlim @lock, Action action) + { + @lock.EnterReadLock(); + try + { + action(); + } + finally + { + @lock.ExitReadLock(); + } + } + + /// Run code within a read lock. + /// The action's return value. + /// The lock to set. + /// The action to perform. + public static TReturn InReadLock(this ReaderWriterLockSlim @lock, Func action) + { + @lock.EnterReadLock(); + try + { + return action(); + } + finally + { + @lock.ExitReadLock(); + } + } + + /// Run code within a write lock. + /// The lock to set. + /// The action to perform. + public static void InWriteLock(this ReaderWriterLockSlim @lock, Action action) + { + @lock.EnterWriteLock(); + try + { + action(); + } + finally + { + @lock.ExitWriteLock(); + } + } + + /// Run code within a write lock. + /// The action's return value. + /// The lock to set. + /// The action to perform. + public static TReturn InWriteLock(this ReaderWriterLockSlim @lock, Func action) + { + @lock.EnterWriteLock(); + try + { + return action(); + } + finally + { + @lock.ExitWriteLock(); + } + } + /**** ** Sprite batch ****/ -- cgit