From 722429c37d866bba552b8a973f81f6aec86718fc Mon Sep 17 00:00:00 2001 From: Anairkoen Schno Date: Thu, 16 Jan 2020 23:52:28 -0600 Subject: [PATCH] Added guarantee that enable/disable will always be from main thread --- IPA.Loader/IPA.Loader.csproj | 1 + IPA.Loader/Loader/PluginManager.cs | 12 ++++++++- .../Loader/StateTransitionTransaction.cs | 5 ++-- .../Attributes/LifecycleAttributes.cs | 12 +++++++++ IPA.Loader/Utilities/Async/Coroutines.cs | 26 +++++++++++++++++++ .../Async/UnityMainThreadTaskScheduler.cs | 23 ++++++++++++++++ 6 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 IPA.Loader/Utilities/Async/Coroutines.cs diff --git a/IPA.Loader/IPA.Loader.csproj b/IPA.Loader/IPA.Loader.csproj index d67fc8e4..3f1dbfe8 100644 --- a/IPA.Loader/IPA.Loader.csproj +++ b/IPA.Loader/IPA.Loader.csproj @@ -138,6 +138,7 @@ + diff --git a/IPA.Loader/Loader/PluginManager.cs b/IPA.Loader/Loader/PluginManager.cs index f13105b3..2867087a 100644 --- a/IPA.Loader/Loader/PluginManager.cs +++ b/IPA.Loader/Loader/PluginManager.cs @@ -13,6 +13,7 @@ using Mono.Cecil; using UnityEngine; using Logger = IPA.Logging.Logger; using System.Threading.Tasks; +using IPA.Utilities.Async; #if NET4 using TaskEx = System.Threading.Tasks.Task; using TaskEx6 = System.Threading.Tasks.Task; @@ -77,7 +78,15 @@ namespace IPA.Loader => new StateTransitionTransaction(AllPlugins, DisabledPlugins); private static readonly object commitTransactionLockObject = new object(); + internal static Task CommitTransaction(StateTransitionTransaction transaction) + { + if (UnityGame.OnMainThread) + return CommitTransactionInternal(transaction); + else + return UnityMainThreadTaskScheduler.Factory.StartNew(() => CommitTransactionInternal(transaction)).Unwrap(); + } + private static Task CommitTransactionInternal(StateTransitionTransaction transaction) { lock (commitTransactionLockObject) { @@ -181,8 +190,9 @@ namespace IPA.Loader return TaskEx6.FromException(new CannotRuntimeDisableException(exec.Executor.Metadata)); var res = TaskEx.WhenAll(exec.Dependents.Select(d => Disable(d, alreadyDisabled))) - .ContinueWith(t => TaskEx.WhenAll(t, exec.Executor.Disable())).Unwrap(); + .ContinueWith(t => TaskEx.WhenAll(t, exec.Executor.Disable()), UnityMainThreadTaskScheduler.Default).Unwrap(); // The WhenAll above allows us to wait for the executor to disable, but still propagate errors + // By scheduling on a UnityMainThreadScheduler, we ensure that Disable() is always called on the Unity main thread alreadyDisabled.Add(exec.Executor, res); return res; } diff --git a/IPA.Loader/Loader/StateTransitionTransaction.cs b/IPA.Loader/Loader/StateTransitionTransaction.cs index 948bcd4e..16263863 100644 --- a/IPA.Loader/Loader/StateTransitionTransaction.cs +++ b/IPA.Loader/Loader/StateTransitionTransaction.cs @@ -209,11 +209,12 @@ namespace IPA.Loader /// /// // get your transaction... /// var complete = transaction.Commit(); - /// complete.ContinueWith(t => { + /// await complete.ContinueWith(t => { /// if (t.IsFaulted) /// Logger.log.Error($"Error disabling plugins: {t.Exception}"); - /// }).Wait(); // if not Wait(), then something else to wait for completion + /// }); /// + /// If you are running in a coroutine, you can use instead of . /// /// /// a which completes whenever all disables complete diff --git a/IPA.Loader/PluginInterfaces/Attributes/LifecycleAttributes.cs b/IPA.Loader/PluginInterfaces/Attributes/LifecycleAttributes.cs index 59ec24b5..3cd4ca6f 100644 --- a/IPA.Loader/PluginInterfaces/Attributes/LifecycleAttributes.cs +++ b/IPA.Loader/PluginInterfaces/Attributes/LifecycleAttributes.cs @@ -26,6 +26,9 @@ namespace IPA /// Typically, this will be used when the parameter of the plugins's /// is . /// + /// + /// The method marked by this attribute will always be called from the Unity main thread. + /// /// /// /// @@ -47,6 +50,9 @@ namespace IPA /// Typically, this will be used when the parameter of the plugins's /// is . /// + /// + /// The method marked by this attribute will always be called from the Unity main thread. + /// /// /// /// @@ -68,6 +74,9 @@ namespace IPA /// Typically, this will be used when the parameter of the plugins's /// is . /// + /// + /// The method marked by this attribute will always be called from the Unity main thread. + /// /// /// /// @@ -89,6 +98,9 @@ namespace IPA /// Typically, this will be used when the parameter of the plugins's /// is . /// + /// + /// The method marked by this attribute will always be called from the Unity main thread. + /// /// /// /// diff --git a/IPA.Loader/Utilities/Async/Coroutines.cs b/IPA.Loader/Utilities/Async/Coroutines.cs new file mode 100644 index 00000000..e94e3550 --- /dev/null +++ b/IPA.Loader/Utilities/Async/Coroutines.cs @@ -0,0 +1,26 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace IPA.Utilities.Async +{ + /// + /// A class providing coroutine helpers. + /// + public static class Coroutines + { + /// + /// Stalls the coroutine until completes, faults, or is canceled. + /// + /// the to wait for + /// a coroutine waiting for the given task + public static IEnumerator WaitForTask(Task task) + { + while (!task.IsCompleted && !task.IsCanceled && !task.IsFaulted) + yield return null; + } + } +} diff --git a/IPA.Loader/Utilities/Async/UnityMainThreadTaskScheduler.cs b/IPA.Loader/Utilities/Async/UnityMainThreadTaskScheduler.cs index 15ec96ea..f3ae0b52 100644 --- a/IPA.Loader/Utilities/Async/UnityMainThreadTaskScheduler.cs +++ b/IPA.Loader/Utilities/Async/UnityMainThreadTaskScheduler.cs @@ -20,6 +20,11 @@ namespace IPA.Utilities.Async /// /// a scheduler that is managed by BSIPA public static new TaskScheduler Default { get; } = new UnityMainThreadTaskScheduler(); + /// + /// Gets a factory for creating tasks on . + /// + /// a factory for creating tasks on the default scheduler + public static TaskFactory Factory { get; } = new TaskFactory(Default); private readonly ConcurrentDictionary tasks = new ConcurrentDictionary(); private int queueEndPosition = 0; @@ -90,6 +95,17 @@ namespace IPA.Utilities.Async /// /// When used as a Unity coroutine, runs the scheduler. Otherwise, this is an invalid call. /// + /// + /// + /// Do not ever call on this + /// coroutine, nor on the behaviour hosting + /// this coroutine. This has no way to detect this, and this object will become invalid. + /// + /// + /// If you need to stop this coroutine, first call , then wait for it to + /// exit on its own. + /// + /// /// a Unity coroutine /// if this scheduler is disposed /// if the scheduler is already running @@ -205,6 +221,10 @@ namespace IPA.Utilities.Async #region IDisposable Support private bool disposedValue = false; // To detect redundant calls + /// + /// Disposes this object. + /// + /// whether or not to dispose managed objects protected virtual void Dispose(bool disposing) { if (!disposedValue) @@ -218,6 +238,9 @@ namespace IPA.Utilities.Async } } + /// + /// Disposes this object. This puts the object into an unusable state. + /// // This code added to correctly implement the disposable pattern. public void Dispose() {