From 4b9de312403990b5cdcfc4a9b5f32d4f3d355647 Mon Sep 17 00:00:00 2001 From: goat Date: Fri, 29 Sep 2023 20:47:54 +0200 Subject: [PATCH] fix: scoped services must register their dependencies with PluginManager to ensure the backing services are kept alive long enough --- Dalamud/IoC/Internal/ServiceContainer.cs | 3 +- Dalamud/ServiceManager.cs | 35 ++++--------- Dalamud/Service{T}.cs | 67 +++++++++++++++++++++--- 3 files changed, 72 insertions(+), 33 deletions(-) diff --git a/Dalamud/IoC/Internal/ServiceContainer.cs b/Dalamud/IoC/Internal/ServiceContainer.cs index a82440029..3dd76473f 100644 --- a/Dalamud/IoC/Internal/ServiceContainer.cs +++ b/Dalamud/IoC/Internal/ServiceContainer.cs @@ -53,7 +53,6 @@ internal class ServiceContainer : IServiceProvider, IServiceType } this.instances[typeof(T)] = new(instance.ContinueWith(x => new WeakReference(x.Result)), typeof(T)); - this.RegisterInterfaces(typeof(T)); } /// @@ -69,7 +68,7 @@ internal class ServiceContainer : IServiceProvider, IServiceType foreach (var resolvableType in resolveViaTypes) { Log.Verbose("=> {InterfaceName} provides for {TName}", resolvableType.FullName ?? "???", type.FullName ?? "???"); - + Debug.Assert(!this.interfaceToTypeMap.ContainsKey(resolvableType), "A service already implements this interface, this is not allowed"); Debug.Assert(type.IsAssignableTo(resolvableType), "Service does not inherit from indicated ResolveVia type"); diff --git a/Dalamud/ServiceManager.cs b/Dalamud/ServiceManager.cs index f2ff864c3..57e4ace10 100644 --- a/Dalamud/ServiceManager.cs +++ b/Dalamud/ServiceManager.cs @@ -145,12 +145,12 @@ internal static class ServiceManager if (serviceKind is ServiceKind.None) continue; - // Scoped service do not go through Service, so we must let ServiceContainer know what their interfaces map to - if (serviceKind is ServiceKind.ScopedService) - { - serviceContainer.RegisterInterfaces(serviceType); + // Let IoC know about the interfaces this service implements + serviceContainer.RegisterInterfaces(serviceType); + + // Scoped service do not go through Service and are never early loaded + if (serviceKind.HasFlag(ServiceKind.ScopedService)) continue; - } Debug.Assert( !serviceKind.HasFlag(ServiceKind.ManualService) && !serviceKind.HasFlag(ServiceKind.ScopedService), @@ -176,15 +176,10 @@ internal static class ServiceManager earlyLoadingServices.Add(serviceType); } - dependencyServicesMap[serviceType] = - (List)typeof(Service<>) - .MakeGenericType(serviceType) - .InvokeMember( - "GetDependencyServices", - BindingFlags.InvokeMethod | BindingFlags.Static | BindingFlags.Public, - null, - null, - null); + var typeAsServiceT = ServiceHelpers.GetAsService(serviceType); + dependencyServicesMap[serviceType] = ServiceHelpers.GetDependencies(typeAsServiceT) + .Select(x => typeof(Service<>).MakeGenericType(x)) + .ToList(); } _ = Task.Run(async () => @@ -327,16 +322,8 @@ internal static class ServiceManager Log.Verbose("Calling GetDependencyServices for '{ServiceName}'", serviceType.FullName!); - dependencyServicesMap[serviceType] = - ((List)typeof(Service<>) - .MakeGenericType(serviceType) - .InvokeMember( - "GetDependencyServices", - BindingFlags.InvokeMethod | BindingFlags.Static | BindingFlags.Public, - null, - null, - null))! - .Select(x => x.GetGenericArguments()[0]).ToList(); + var typeAsServiceT = ServiceHelpers.GetAsService(serviceType); + dependencyServicesMap[serviceType] = ServiceHelpers.GetDependencies(typeAsServiceT); allToUnload.Add(serviceType); } diff --git a/Dalamud/Service{T}.cs b/Dalamud/Service{T}.cs index aa10ead6e..539941c27 100644 --- a/Dalamud/Service{T}.cs +++ b/Dalamud/Service{T}.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Reflection; using System.Threading.Tasks; @@ -133,8 +134,8 @@ internal static class Service where T : IServiceType res.AddRange(typeof(T) .GetFields(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic) - .Select(x => x.FieldType) - .Where(x => x.GetCustomAttribute(true) != null)); + .Where(x => x.GetCustomAttribute(true) != null) + .Select(x => x.FieldType)); res.AddRange(typeof(T) .GetCustomAttributes() @@ -148,13 +149,32 @@ internal static class Service where T : IServiceType { if (!serviceType.IsAssignableTo(typeof(IServiceType))) continue; - - var attr = serviceType.GetCustomAttribute(true); - if (attr == null) + + if (serviceType == typeof(PluginManager)) continue; - + // Scoped plugin services lifetime is tied to their scopes. They go away when LocalPlugin goes away. + // Nonetheless, their direct dependencies must be considered. if (serviceType.GetServiceKind() == ServiceManager.ServiceKind.ScopedService) + { + var typeAsServiceT = ServiceHelpers.GetAsService(serviceType); + var dependencies = ServiceHelpers.GetDependencies(typeAsServiceT); + ServiceManager.Log.Verbose("Found dependencies of scoped plugin service {Type} ({Cnt})", serviceType.FullName!, dependencies!.Count); + + foreach (var scopedDep in dependencies) + { + if (scopedDep == typeof(PluginManager)) + throw new Exception("Scoped plugin services cannot depend on PluginManager."); + + ServiceManager.Log.Verbose("PluginManager MUST depend on {Type} via {BaseType}", scopedDep.FullName!, serviceType.FullName!); + res.Add(scopedDep); + } + + continue; + } + + var pluginInterfaceAttribute = serviceType.GetCustomAttribute(true); + if (pluginInterfaceAttribute == null) continue; ServiceManager.Log.Verbose("PluginManager MUST depend on {Type}", serviceType.FullName!); @@ -164,7 +184,6 @@ internal static class Service where T : IServiceType return res .Distinct() - .Select(x => typeof(Service<>).MakeGenericType(x)) .ToList(); } @@ -295,3 +314,37 @@ internal static class Service where T : IServiceType } } } + +/// +/// Helper functions for services. +/// +internal static class ServiceHelpers +{ + /// + /// Get a list of dependencies for a service. Only accepts Service<T> types. + /// These are returned as Service<T> types. + /// + /// The dependencies for this service. + /// A list of dependencies. + public static List GetDependencies(Type serviceType) + { + return (List)serviceType.InvokeMember( + "GetDependencyServices", + BindingFlags.InvokeMethod | BindingFlags.Static | BindingFlags.Public, + null, + null, + null) ?? new List(); + } + + /// + /// Get the Service<T> type for a given service type. + /// This will throw if the service type is not a valid service. + /// + /// The type to obtain a Service<T> for. + /// The Service<T>. + public static Type GetAsService(Type type) + { + return typeof(Service<>) + .MakeGenericType(type); + } +}