Merge pull request #1151 from goaaats/unload_order

Walk dependencies to unload services

Dalamud/Game/Framework.cs

@@ -492,7 +492,7 @@ public sealed class Framework : IDisposable, IServiceType
         Log.Information("Framework::Destroy!");
         Service<Dalamud>.Get().Unload();
         this.RunPendingTickTasks();
-        ServiceManager.UnloadAllServices();
+        ServiceManager.WaitForServiceUnload();
         Log.Information("Framework::Destroy OK!");
 
         return this.destroyHook.OriginalDisposeSafe(framework);

Dalamud/Plugin/Internal/PluginManager.cs

@@ -32,14 +32,14 @@ namespace Dalamud.Plugin.Internal;
 
 /// <summary>
 /// Class responsible for loading and unloading plugins.
+/// NOTE: ALL plugin exposed services are marked as dependencies for PluginManager in Service{T}.
 /// </summary>
 [ServiceManager.EarlyLoadedService]
 #pragma warning disable SA1015
-// DalamudTextureWrap registers textures to dispose with IM
-[InherentDependency<InterfaceManager.InterfaceManagerWithScene>]
 
-// DalamudPluginInterface asks to remove chat link handlers
+// DalamudTextureWrap registers textures to dispose with IM
-[InherentDependency<ChatGui>]
+[InherentDependency<InterfaceManager>]
+
 #pragma warning restore SA1015
 internal partial class PluginManager : IDisposable, IServiceType
 {

Dalamud/ServiceManager.cs

@@ -1,8 +1,10 @@
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using System.Reflection;
+using System.Threading;
 using System.Threading.Tasks;
 
 using Dalamud.Configuration.Internal;
@@ -19,6 +21,13 @@ namespace Dalamud;
 /// </summary>
 internal static class ServiceManager
 {
+    /**
+     * TODO:
+     * - Unify dependency walking code(load/unload
+     * - Visualize/output .dot or imgui thing
+     */
+
+
     /// <summary>
     /// Static log facility for Service{T}, to avoid duplicate instances for different types.
     /// </summary>
@@ -28,6 +37,40 @@ internal static class ServiceManager
 
     private static readonly List<Type> LoadedServices = new();
 
+    private static ManualResetEvent unloadResetEvent = new(false);
+    
+    /// <summary>
+    /// Kinds of services.
+    /// </summary>
+    [Flags]
+    public enum ServiceKind
+    {
+        /// <summary>
+        /// Not a service.
+        /// </summary>
+        None = 0,
+        
+        /// <summary>
+        /// Regular service.
+        /// </summary>
+        ManualService = 1 << 0,
+        
+        /// <summary>
+        /// Service that is loaded asynchronously while the game starts.
+        /// </summary>
+        EarlyLoadedService = 1 << 1,
+        
+        /// <summary>
+        /// Service that is loaded before the game starts.
+        /// </summary>
+        BlockingEarlyLoadedService = 1 << 2,
+        
+        /// <summary>
+        /// Service that is loaded automatically when the game starts, synchronously or asynchronously.
+        /// </summary>
+        AutoLoadService = EarlyLoadedService | BlockingEarlyLoadedService,
+    }
+
     /// <summary>
     /// Gets task that gets completed when all blocking early loading services are done loading.
     /// </summary>
@@ -89,9 +132,11 @@ internal static class ServiceManager
 
         foreach (var serviceType in Assembly.GetExecutingAssembly().GetTypes())
         {
-            var attr = serviceType.GetCustomAttribute<Service>(true)?.GetType();
+            var serviceKind = serviceType.GetServiceKind();
-            if (attr?.IsAssignableTo(typeof(EarlyLoadedService)) != true)
+            if (serviceKind == ServiceKind.None)
                 continue;
+            
+            Debug.Assert(!serviceKind.HasFlag(ServiceKind.ManualService), "Regular services should never end up here");
 
             var getTask = (Task)typeof(Service<>)
                                 .MakeGenericType(serviceType)
@@ -102,9 +147,9 @@ internal static class ServiceManager
                                     null,
                                     null);
 
-            if (attr.IsAssignableTo(typeof(BlockingEarlyLoadedService)))
+            if (serviceKind.HasFlag(ServiceKind.BlockingEarlyLoadedService))
             {
-                getAsyncTaskMap[serviceType] = getTask;
+                getAsyncTaskMap[typeof(Service<>).MakeGenericType(serviceType)] = getTask;
                 blockingEarlyLoadingServices.Add(serviceType);
             }
             else
@@ -148,8 +193,24 @@ internal static class ServiceManager
             {
                 foreach (var serviceType in servicesToLoad)
                 {
-                    if (dependencyServicesMap[serviceType].Any(
+                    var hasDeps = true;
-                            x => getAsyncTaskMap.GetValueOrDefault(x)?.IsCompleted == false))
+                    foreach (var dependency in dependencyServicesMap[serviceType])
+                    {
+                        var depServiceKind = dependency.GetServiceKind();
+                        var depResolveTask = getAsyncTaskMap.GetValueOrDefault(dependency);
+
+                        if (depResolveTask == null && (depServiceKind.HasFlag(ServiceKind.EarlyLoadedService) || depServiceKind.HasFlag(ServiceKind.BlockingEarlyLoadedService)))
+                        {
+                            Log.Error("{Type}: {Dependency} has no resolver task, is it early loaded or blocking early loaded?", serviceType.FullName!, dependency.FullName!);
+                            Debug.Assert(false, $"No resolver for dependent service {dependency.FullName}");
+                        }
+                        else if (depResolveTask is { IsCompleted: false })
+                        {
+                            hasDeps = false;
+                        }
+                    }
+                    
+                    if (!hasDeps)
                         continue;
 
                     tasks.Add((Task)typeof(Service<>)
@@ -227,23 +288,108 @@ internal static class ServiceManager
             return;
         }
 
-        lock (LoadedServices)
+        unloadResetEvent.Reset();
-        {
-            while (LoadedServices.Any())
-            {
-                var serviceType = LoadedServices.Last();
-                LoadedServices.RemoveAt(LoadedServices.Count - 1);
 
-                typeof(Service<>)
+        var dependencyServicesMap = new Dictionary<Type, List<Type>>();
-                    .MakeGenericType(serviceType)
+        var allToUnload = new HashSet<Type>();
+        var unloadOrder = new List<Type>();
+        
+        Log.Information("==== COLLECTING SERVICES TO UNLOAD ====");
+        
+        foreach (var serviceType in Assembly.GetExecutingAssembly().GetTypes())
+        {
+            if (!serviceType.IsAssignableTo(typeof(IServiceType)))
+                continue;
+            
+            dependencyServicesMap[serviceType] =
+                ((List<Type>)typeof(Service<>)
+                            .MakeGenericType(serviceType)
+                            .InvokeMember(
+                                "GetDependencyServices",
+                                BindingFlags.InvokeMethod | BindingFlags.Static | BindingFlags.Public,
+                                null,
+                                null,
+                                null))!
+                .Select(x => x.GetGenericArguments()[0]).ToList();
+
+            allToUnload.Add(serviceType);
+        }
+
+        void UnloadService(Type serviceType)
+        {
+            if (unloadOrder.Contains(serviceType))
+                return;
+
+            var deps = dependencyServicesMap[serviceType];
+            foreach (var dep in deps)
+            {
+                UnloadService(dep);
+            }
+
+            unloadOrder.Add(serviceType);
+            Log.Information("Queue for unload {Type}", serviceType.FullName!);
+        }
+        
+        foreach (var serviceType in allToUnload)
+        {
+            UnloadService(serviceType);
+        }
+        
+        Log.Information("==== UNLOADING ALL SERVICES ====");
+
+        unloadOrder.Reverse();
+        foreach (var type in unloadOrder)
+        {
+            Log.Verbose("Unload {Type}", type.FullName!);
+
+            typeof(Service<>)
+                    .MakeGenericType(type)
                     .InvokeMember(
                         "Unset",
                         BindingFlags.InvokeMethod | BindingFlags.Static | BindingFlags.NonPublic,
                         null,
                         null,
                         null);
-            }
         }
+        
+        lock (LoadedServices)
+        {
+            LoadedServices.Clear();
+        }
+
+        unloadResetEvent.Set();
+    }
+
+    /// <summary>
+    /// Wait until all services have been unloaded.
+    /// </summary>
+    public static void WaitForServiceUnload()
+    {
+        unloadResetEvent.WaitOne();
+    }
+
+    /// <summary>
+    /// Get the service type of this type.
+    /// </summary>
+    /// <param name="type">The type to check.</param>
+    /// <returns>The type of service this type is.</returns>
+    public static ServiceKind GetServiceKind(this Type type)
+    {
+        var attr = type.GetCustomAttribute<Service>(true)?.GetType();
+        if (attr == null)
+            return ServiceKind.None;
+        
+        Debug.Assert(
+            type.IsAssignableTo(typeof(IServiceType)),
+            "Service did not inherit from IServiceType");
+
+        if (attr.IsAssignableTo(typeof(BlockingEarlyLoadedService)))
+            return ServiceKind.BlockingEarlyLoadedService;
+        
+        if (attr.IsAssignableTo(typeof(EarlyLoadedService)))
+            return ServiceKind.EarlyLoadedService;
+
+        return ServiceKind.ManualService;
     }
 
     /// <summary>

Dalamud/Service{T}.cs

@@ -6,6 +6,7 @@ using System.Threading.Tasks;
 
 using Dalamud.IoC;
 using Dalamud.IoC.Internal;
+using Dalamud.Plugin.Internal;
 using Dalamud.Utility.Timing;
 using JetBrains.Annotations;
 
@@ -121,18 +122,43 @@ internal static class Service<T> where T : IServiceType
     public static List<Type> GetDependencyServices()
     {
         var res = new List<Type>();
-        res.AddRange(GetServiceConstructor()
+
-                     .GetParameters()
+        var ctor = GetServiceConstructor();
-                     .Select(x => x.ParameterType));
+        if (ctor != null)
+        {
+            res.AddRange(ctor
+               .GetParameters()
+               .Select(x => x.ParameterType));
+        }
+        
         res.AddRange(typeof(T)
-                     .GetFields(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)
+                         .GetFields(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)
-                     .Select(x => x.FieldType)
+                         .Select(x => x.FieldType)
-                     .Where(x => x.GetCustomAttribute<ServiceManager.ServiceDependency>(true) != null));
+                         .Where(x => x.GetCustomAttribute<ServiceManager.ServiceDependency>(true) != null));
+        
         res.AddRange(typeof(T)
                      .GetCustomAttributes()
                      .OfType<InherentDependencyAttribute>()
                      .Select(x => x.GetType().GetGenericArguments().First()));
 
+
+        // HACK: PluginManager needs to depend on ALL plugin exposed services
+        if (typeof(T) == typeof(PluginManager))
+        {
+            foreach (var serviceType in Assembly.GetExecutingAssembly().GetTypes())
+            {
+                if (!serviceType.IsAssignableTo(typeof(IServiceType)))
+                    continue;
+
+                var attr = serviceType.GetCustomAttribute<PluginInterfaceAttribute>(true);
+                if (attr == null)
+                    continue;
+
+                ServiceManager.Log.Verbose("PluginManager MUST depend on {Type}", serviceType.FullName!);
+                res.Add(serviceType);
+            }
+        }
+
         return res
                .Distinct()
                .Select(x => typeof(Service<>).MakeGenericType(x))
@@ -228,19 +254,22 @@ internal static class Service<T> where T : IServiceType
                              .GetValue(task);
     }
 
-    private static ConstructorInfo GetServiceConstructor()
+    private static ConstructorInfo? GetServiceConstructor()
     {
         const BindingFlags ctorBindingFlags =
             BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic |
             BindingFlags.CreateInstance | BindingFlags.OptionalParamBinding;
         return typeof(T)
                .GetConstructors(ctorBindingFlags)
-               .Single(x => x.GetCustomAttributes(typeof(ServiceManager.ServiceConstructor), true).Any());
+               .SingleOrDefault(x => x.GetCustomAttributes(typeof(ServiceManager.ServiceConstructor), true).Any());
     }
 
     private static async Task<T> ConstructObject()
     {
         var ctor = GetServiceConstructor();
+        if (ctor == null)
+            throw new Exception($"Service \"{typeof(T).FullName}\" had no applicable constructor");
+        
         var args = await Task.WhenAll(
                        ctor.GetParameters().Select(x => ResolveServiceFromTypeAsync(x.ParameterType)));
         using (Timings.Start($"{typeof(T).Name} Construct"))