Add HttpApplication.RegisterModule

src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/HostingRuntimeExtensions.cs

@@ -22,7 +22,7 @@ public static void AddHostingRuntime(this IServiceCollection services)
         services.TryAddSingleton<HostingEnvironmentAccessor>();
         services.TryAddSingleton<VirtualPathUtilityImpl>();
         services.TryAddSingleton<IMapPathUtility, MapPathUtility>();
-        services.TryAddEnumerable(ServiceDescriptor.Singleton<IStartupFilter, HostingEnvironmentStartupFilter>());
+        services.TryAddEnumerable(ServiceDescriptor.Transient<IStartupFilter, HostingEnvironmentStartupFilter>());
 
         services.AddOptions<SystemWebAdaptersOptions>()
 
@@ -55,11 +55,14 @@ public static void AddHostingRuntime(this IServiceCollection services)
             });
     }
 
-    private sealed class HostingEnvironmentStartupFilter : IStartupFilter, IDisposable
+    private sealed class HostingEnvironmentStartupFilter : IStartupFilter
     {
         public HostingEnvironmentStartupFilter(HostingEnvironmentAccessor accessor)
         {
-            HostingEnvironmentAccessor.Current = accessor;
+            // We don't need to store this as it will remain in the DI container. However, we force it to be injected here
+            // so that it will be activated early on in the pipeline and set the current runtime. When the host is completed,
+            // it will be disposed and unset itself from the System.Web runtime.
+            _ = accessor;
         }
 
         public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next)
@@ -76,11 +79,6 @@ public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next)
 
                 next(builder);
             };
-
-        public void Dispose()
-        {
-            HostingEnvironmentAccessor.Current = null;
-        }
     }
 
     /// <summary>

src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/HttpApplication/HttpApplicationExtensions.cs

@@ -4,25 +4,29 @@
 using System;
 using System.Web;
 using Microsoft.AspNetCore.Builder;
-using Microsoft.AspNetCore.Hosting;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.SystemWebAdapters;
 using Microsoft.AspNetCore.SystemWebAdapters.Features;
 using Microsoft.Extensions.DependencyInjection.Extensions;
+using Microsoft.Extensions.Hosting;
 using Microsoft.Extensions.ObjectPool;
 using Microsoft.Extensions.Options;
 
 namespace Microsoft.Extensions.DependencyInjection;
 
 public static class HttpApplicationExtensions
 {
+    public static ISystemWebAdapterBuilder AddHttpApplication(this ISystemWebAdapterBuilder builder)
+        => builder.AddHttpApplication(_ => { });
+
     public static ISystemWebAdapterBuilder AddHttpApplication(this ISystemWebAdapterBuilder builder, Action<HttpApplicationOptions> configure)
     {
         ArgumentNullException.ThrowIfNull(builder);
 
+        builder.Services.TryAddSingleton<ModuleCollection>();
+        builder.Services.AddTransient<IModuleRegistrar>(sp => sp.GetRequiredService<ModuleCollection>());
         builder.Services.TryAddSingleton<HttpApplicationPooledObjectPolicy>();
-        builder.Services.AddTransient<IStartupFilter, HttpApplicationStartupFilter>();
-        builder.Services.TryAddSingleton<IPooledObjectPolicy<HttpApplication>>(ctx => ctx.GetRequiredService<HttpApplicationPooledObjectPolicy>());
+        builder.Services.TryAddSingleton<IPooledObjectPolicy<HttpApplication>>(sp => sp.GetRequiredService<HttpApplicationPooledObjectPolicy>());
         builder.Services.TryAddSingleton<ObjectPool<HttpApplication>>(sp =>
         {
             var options = sp.GetRequiredService<IOptions<HttpApplicationOptions>>();
@@ -37,8 +41,11 @@ public static ISystemWebAdapterBuilder AddHttpApplication(this ISystemWebAdapter
         });
 
         builder.Services.AddOptions<HttpApplicationOptions>()
-            .Configure(configure)
-            .PostConfigure(c => c.MakeReadOnly());
+            .Configure<ModuleCollection>((options, modules) =>
+            {
+                options.ModuleCollection = modules;
+            })
+            .Configure(configure);
 
         return builder;
     }
@@ -140,63 +147,5 @@ public static IApplicationBuilder UseAuthorizationEvents(this IApplicationBuilde
     }
 
     internal static bool AreHttpApplicationEventsRequired(this IApplicationBuilder builder)
-    {
-        const string AreHttpApplicationEventsRequired = nameof(AreHttpApplicationEventsRequired);
-
-        if (builder.Properties.TryGetValue(AreHttpApplicationEventsRequired, out var existing) && existing is bool b)
-        {
-            return b;
-        }
-
-        var options = builder.ApplicationServices.GetRequiredService<IOptions<HttpApplicationOptions>>().Value;
-
-        var hasModules = options.Modules.Count > 0;
-        var hasCustomApplication = options.ApplicationType != typeof(HttpApplication);
-
-        var areEventsRequired = hasModules || hasCustomApplication;
-
-        builder.Properties[AreHttpApplicationEventsRequired] = areEventsRequired;
-
-        return areEventsRequired;
-    }
-
-    private sealed class HttpApplicationStartupFilter : IStartupFilter
-    {
-        private readonly ObjectPool<HttpApplication> _pool;
-
-        public HttpApplicationStartupFilter(ObjectPool<HttpApplication> pool)
-        {
-            _pool = pool;
-        }
-
-        public Action<IApplicationBuilder> Configure(Action<IApplicationBuilder> next) => builder =>
-        {
-            CallStartup(builder.ApplicationServices);
-            next(builder);
-        };
-
-        private void CallStartup(IServiceProvider services)
-        {
-            using var scope = services.CreateScope();
-
-            var app = _pool.Get();
-
-            // ASP.NET Framework provided an HttpContext instance that was not tied to a request for Start
-            app.Context = new DefaultHttpContext
-            {
-                RequestServices = scope.ServiceProvider,
-            }.AsSystemWeb();
-
-            try
-            {
-                // This is only invoked at the beginning of the application
-                // See https://referencesource.microsoft.com/#System.Web/HttpApplication.cs,2417
-                app.InvokeEvent(ApplicationEvent.ApplicationStart);
-            }
-            finally
-            {
-                _pool.Return(app);
-            }
-        }
-    }
+        => builder.ApplicationServices.GetRequiredService<IOptions<HttpApplicationOptions>>().Value.IsAdded;
 }

src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/HttpApplication/HttpApplicationOptions.cs

@@ -13,7 +13,18 @@ namespace Microsoft.AspNetCore.SystemWebAdapters;
 
 public class HttpApplicationOptions
 {
-    private readonly ModuleCollection _modules = new();
+    private ModuleCollection? _modules;
+
+    internal ModuleCollection ModuleCollection
+    {
+        get => _modules ?? throw new InvalidOperationException("HttpApplicationOptions must be initialized with a module collection");
+        set => _modules = value;
+    }
+
+    /// <summary>
+    /// Used to track if the services were added or if the options was just automatically created.
+    /// </summary>
+    internal bool IsAdded => _modules is { };
 
     private Type _applicationType = typeof(HttpApplication);
 
@@ -24,7 +35,7 @@ public Type ApplicationType
         {
             ArgumentNullException.ThrowIfNull(value);
 
-            _modules.CheckIsReadOnly();
+            ModuleCollection.CheckIsReadOnly();
 
             if (!_applicationType.IsAssignableTo(typeof(HttpApplication)))
             {
@@ -35,9 +46,7 @@ public Type ApplicationType
         }
     }
 
-    public IDictionary<string, Type> Modules => _modules;
-
-    internal void MakeReadOnly() => _modules.MakeReadOnly();
+    public IDictionary<string, Type> Modules => ModuleCollection;
 
     /// <summary>
     /// Gets or sets the number of <see cref="HttpApplication"/> retained for reuse. In order to support modules and applications that may contain state,
@@ -53,99 +62,6 @@ public void RegisterModule(Type type, string? name = null)
     {
         ArgumentNullException.ThrowIfNull(type);
 
-        Modules.Add(name ?? MakeUniqueModuleName(type), type);
-
-        // Gets a dynamic name similar to how ASP.NET Framework did in the static HttpApplication.RegisterModule(Type moduleType) method
-        static string MakeUniqueModuleName(Type type)
-            => Invariant($"__DynamicModule_{type.AssemblyQualifiedName}_{Guid.NewGuid()}");
-    }
-
-    /// <summary>
-    /// A collection that validates that the types added are actual IHttpModule types
-    /// </summary>
-    private sealed class ModuleCollection : IDictionary<string, Type>
-    {
-        private readonly Dictionary<string, Type> _inner;
-
-        public ModuleCollection()
-        {
-            _inner = new(StringComparer.InvariantCultureIgnoreCase);
-        }
-
-        public Type this[string key]
-        {
-            get => _inner[key];
-            set => _inner[key] = value;
-        }
-
-        public ICollection<string> Keys => _inner.Keys;
-
-        public ICollection<Type> Values => _inner.Values;
-
-        public int Count => _inner.Count;
-
-        public bool IsReadOnly { get; private set; }
-
-        public void Add(string key, Type type)
-        {
-            CheckIsReadOnly();
-
-            if (Contains(new(key, type)))
-            {
-                throw new InvalidOperationException($"Module {type.FullName} is already registered with key '{key}'.");
-            }
-
-            if (!type.IsAssignableTo(typeof(IHttpModule)))
-            {
-                throw new InvalidOperationException($"Type {type.FullName} is not a valid IHttpModule.");
-            }
-
-            _inner.Add(key, type);
-        }
-
-        public void Add(KeyValuePair<string, Type> item) => Add(item.Key, item.Value);
-
-        public void Clear()
-        {
-            CheckIsReadOnly();
-            _inner.Clear();
-        }
-
-        public bool Contains(KeyValuePair<string, Type> item) => ((IDictionary<string, Type>)_inner).Contains(item);
-
-        public bool ContainsKey(string key) => _inner.ContainsKey(key);
-
-        public void CopyTo(KeyValuePair<string, Type>[] array, int arrayIndex) => ((ICollection<KeyValuePair<string, Type>>)_inner).CopyTo(array, arrayIndex);
-
-        public IEnumerator<KeyValuePair<string, Type>> GetEnumerator() => ((IEnumerable<KeyValuePair<string, Type>>)_inner).GetEnumerator();
-
-        public void MakeReadOnly()
-        {
-            IsReadOnly = true;
-        }
-
-        public bool Remove(string key)
-        {
-            CheckIsReadOnly();
-            return _inner.Remove(key);
-        }
-
-        public bool Remove(KeyValuePair<string, Type> item)
-        {
-            CheckIsReadOnly();
-            return ((ICollection<KeyValuePair<string, Type>>)_inner).Remove(item);
-        }
-
-        public bool TryGetValue(string key, [MaybeNullWhen(false)] out Type value) => _inner.TryGetValue(key, out value);
-
-        IEnumerator IEnumerable.GetEnumerator() => _inner.GetEnumerator();
-
-        public void CheckIsReadOnly()
-        {
-            if (IsReadOnly)
-            {
-                throw new InvalidOperationException("Module collection is readonly");
-            }
-        }
+        ModuleCollection.RegisterModule(type, name);
     }
 }

src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/HttpApplication/HttpApplicationPooledObjectPolicy.cs

@@ -6,6 +6,7 @@
 using System.Linq;
 using System.Reflection;
 using System.Web;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.SystemWebAdapters.Features;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Logging;
@@ -108,6 +109,8 @@ public HttpApplicationPooledObjectPolicy(IServiceProvider services, IOptions<Htt
 
     public override HttpApplication Create()
     {
+        EnsureStartHasBeenCalled();
+
         var app = _factory.Value(_services);
 
         // This is invoked each time an HttpApplication is constructed
@@ -123,13 +126,15 @@ public override bool Return(HttpApplication obj)
     }
 
     /// <summary>
-    /// Creates a callback that will regsiter implicit events on <see cref="HttpApplication"/>.
+    /// Creates a callback that will register implicit events on <see cref="HttpApplication"/>.
     /// </summary>
     /// <param name="options">Options for the <see cref="HttpApplication"/>.</param>
     /// <returns>A callback to create a new <see cref="HttpApplication"/> instance.</returns>
     /// <seealso cref="https://referencesource.microsoft.com/#System.Web/HttpApplication.cs,b24816e1097719dd"/>
     private Func<IServiceProvider, HttpApplication> CreateFactory(HttpApplicationOptions options)
     {
+        options.ModuleCollection.MakeReadOnly();
+
         var eventInitializer = GetEventInitializer(options);
         var factory = ActivatorUtilities.CreateFactory(options.ApplicationType, Array.Empty<Type>());
         var moduleFactories = options.Modules
@@ -242,4 +247,39 @@ private enum EventParseState
         NotSupported,
         InvalidSignature,
     }
+
+    private bool _started;
+
+    private void EnsureStartHasBeenCalled()
+    {
+        if (!_started)
+        {
+            CallApplicationStart();
+        }
+    }
+
+    private void CallApplicationStart()
+    {
+        lock (_factory)
+        {
+            if (!_started)
+            {
+                using var scope = _services.CreateScope();
+
+                var app = _factory.Value(_services);
+
+                // ASP.NET Framework provided an HttpContext instance that was not tied to a request for Start
+                app.Context = new DefaultHttpContext
+                {
+                    RequestServices = scope.ServiceProvider,
+                }.AsSystemWeb();
+
+                // This is only invoked at the beginning of the application
+                // See https://referencesource.microsoft.com/#System.Web/HttpApplication.cs,2417
+                app.InvokeEvent(ApplicationEvent.ApplicationStart);
+
+                _started = true;
+            }
+        }
+    }
 }