Skip to content

ServiceProviderTransformerFactory(Async)/ServiceProviderMapperFactory(Async) never dispose their IServiceScope, causing unbounded memory growth for IDisposable transients #4252

Description

@cmyhill-justeat

Describe the bug

ServiceProviderTransformerFactory, ServiceProviderTransformerFactoryAsync, ServiceProviderMapperFactory, and ServiceProviderMapperFactoryAsync (in Paramore.Brighter.Extensions.DependencyInjection) each construct a single ServiceProviderLifetimeScope instance for the lifetime of the factory, and that factory is itself typically a process-lifetime singleton (owned by Dispatcher).

When the configured lifetime is ServiceLifetime.Transient (the default for TransformerLifetime/MapperLifetime), every Create(...) call goes through:

// ServiceProviderLifetimeScope.cs
private T? GetTransient<T>(Type objectType) where T : class
{
    _scope ??= _serviceProvider.CreateScope();   // created ONCE, reused forever
    return (T?)_scope.ServiceProvider.GetService(objectType);
}

_scope is created lazily on the first call and then reused for every subsequent resolution for the life of the process. The built-in .NET DI container tracks every IDisposable/IAsyncDisposable instance resolved from a scope in an internal list, so it can dispose them all when the scope itself is disposed. Because _scope here is never disposed (only the owning factory's Dispose() would do that, which in practice only happens at process shutdown), that internal tracking list grows by one entry for every message processed, for as long as the process runs -- a genuine unbounded memory leak for any IAmAMessageTransform/IAmAMessageTransformAsync implementation that implements IDisposable (both interfaces extend it).

Calling factory.Release(instance) after each message does call .Dispose() on the individual resolved instance:

// ServiceProviderLifetimeScope.cs
public void Release(object? instance)
{
    if (_lifetime == ServiceLifetime.Singleton) return;
    if (instance is IDisposable disposal)
        disposal.Dispose();
}

...but this has no effect on the leak: disposing an instance directly does not remove the .NET DI container's own internal reference to it from the scope's tracked-disposables list. Only disposing the scope itself would do that.

For comparison, ServiceProviderHandlerFactory gets this right: it keys a ConcurrentDictionary<IAmALifetime, ServiceProviderLifetimeScope> by the per-message IAmALifetime, and ReleaseLifetimeScope(lifetime) calls _lifetimeScopes.TryRemove(lifetime, out var scope) followed by scope.Dispose() -- a fresh scope per unit of work, correctly disposed afterwards. The transformer and mapper factories don''t do this; they use one shared _lifetimeScope field for the whole factory''s lifetime with no per-message scope creation/disposal at all.

(IAmAMessageMapper/IAmAMessageMapperAsync don''t currently implement IDisposable, so the mapper factories aren''t exploitable today -- but the same design flaw is present there and would leak identically the moment a disposable mapper implementation is introduced.)

Steps to reproduce

  1. Register a custom IAmAMessageTransformAsync implementation that also implements IDisposable, e.g.:

    public class MyTransform : IAmAMessageTransformAsync
    {
        public static int LiveInstances;
        public MyTransform() => Interlocked.Increment(ref LiveInstances);
        public void Dispose() { /* no-op is enough to reproduce */ }
        public void InitializeUnwrapFromAttributeParams(params object[] initializerList) { }
        public void InitializeWrapFromAttributeParams(params object[] initializerList) { }
        public Task<Message> WrapAsync(Message message, Publication publication, RequestContext? context, CancellationToken cancellationToken = default) => Task.FromResult(message);
        public Task<Message> UnwrapAsync(Message message, CancellationToken cancellationToken = default) => Task.FromResult(message);
    }
  2. Register it with the default (Transient) TransformerLifetime:

    services.AddTransient<MyTransform>();
    services.AddBrighter() // TransformerLifetime defaults to ServiceLifetime.Transient
        // ... wire MyTransform onto a message pipeline via [MyTransformAttribute] or similar
  3. Run a consumer loop that processes N messages through a pipeline that uses MyTransform.

  4. Observe MyTransform.LiveInstances after each message: it keeps counting up and never comes back down, even though Dispose() is invoked on each instance via Release(). A heap snapshot (e.g. via dotnet-gcdump) taken after processing confirms N live MyTransform instances are all still reachable, retained via:

    MyTransform
      <- (whatever the transform''s Context/fields reference)
      <- System.Object[] / List<Object>   } the DI container''s internal
                                            } disposables-tracking list for one scope
      <- Microsoft.Extensions.DependencyInjection.ServiceLookup.ServiceProviderEngineScope
      <- Paramore.Brighter.Extensions.DependencyInjection.ServiceProviderLifetimeScope
      <- Paramore.Brighter.Extensions.DependencyInjection.ServiceProviderTransformerFactoryAsync
      <- Paramore.Brighter.ServiceActivator.Dispatcher (singleton)
    

Expected behavior

Each Transient-lifetime, IDisposable transform (or mapper, in future) instance should be eligible for garbage collection shortly after Release()/Dispose() is called on it -- memory usage for a steady message-processing workload should plateau, not grow without bound.

Actual behavior

Every message resolved through a Transient-lifetime transform/mapper factory permanently retains that instance (and anything it references, e.g. a Context/RequestContext assigned to it per-message) for the life of the process, because the single, shared IServiceScope used to resolve it is never disposed.

Suggested fix

Create and dispose a short-lived IServiceScope per resolution (mirroring what ServiceProviderHandlerFactory already does correctly via its per-IAmALifetime-keyed ConcurrentDictionary<IAmALifetime, ServiceProviderLifetimeScope> + ReleaseLifetimeScope), e.g.:

private T? GetTransient<T>(Type objectType) where T : class
{
    using var scope = _serviceProvider.CreateScope();
    return (T?)scope.ServiceProvider.GetService(objectType);
}

or, if per-call scope creation is a performance concern for high-throughput consumers, key transient scopes by the per-message IAmALifetime the same way ServiceProviderHandlerFactory does, so each is disposed once its message finishes processing instead of being permanently cached on the factory.

Environment

  • Paramore.Brighter / Paramore.Brighter.Extensions.DependencyInjection: 10.6.0
  • Affected files: Paramore.Brighter.Extensions.DependencyInjection/ServiceProviderLifetimeScope.cs, ServiceProviderTransformerFactory.cs, ServiceProviderTransformerFactoryAsync.cs, ServiceProviderMapperFactory.cs, ServiceProviderMapperFactoryAsync.cs
  • .NET 8, Microsoft.Extensions.DependencyInjection

Workaround

Setting TransformerLifetime/MapperLifetime to ServiceLifetime.Singleton avoids the unbounded growth (one instance is cached forever instead of one new instance being permanently tracked per message), at the cost of sharing one instance across all concurrent calls -- safe only if the transform/mapper implementation has no per-call mutable state.

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions