Why DI container disposes the scoped service twice?

The service:

    public interface IMessageWriter
    {
        void Write(string message);
    }

    public class ConsoleMessageWriter:IMessageWriter, IDisposable
    {
        public void Dispose()
        {
            Console.WriteLine($"The {GetType().Name} instanse is disposed.");
        }

        public void Write(string message)
        {
            Console.WriteLine(message);
        }
    }

Now I use it in a scope:

using DI_Sandbox.Models;
using Microsoft.Extensions.DependencyInjection;

var services = new ServiceCollection();
services
    .AddScoped<ConsoleMessageWriter>()
    .AddScoped<IMessageWriter>(provider => provider.GetRequiredService<ConsoleMessageWriter>());

var container = services.BuildServiceProvider(validateScopes:true);
using (var serviceScope = container.CreateScope())
{
    var messageWriter1 = serviceScope.ServiceProvider.GetRequiredService<IMessageWriter>();
    var messageWriter2 = serviceScope.ServiceProvider.GetRequiredService<IMessageWriter>();

    Console.WriteLine($"messageWriter1 == messageWriter2: {messageWriter1 == messageWriter2}");

    messageWriter1.Write("Hello DI!");
}
Console.Write("THE END");

The output:

messageWriter1 == messageWriter2: True
Hello DI!
The ConsoleMessageWriter instanse is disposed.
The ConsoleMessageWriter instanse is disposed.
THE END

Why the scoped service was disposed twice? How to fix it? I need dispose the service once.

Solution

You made two registrations. One for ConsoleMessageWriter and one for IMessageWriter:

services.AddScoped<ConsoleMessageWriter>()
services.AddScoped<IMessageWriter>(p =>
    p.GetRequiredService<ConsoleMessageWriter>());

While the registration for IMessageWriter calls back into the ConsoleMessageWriter registration, MS.DI has no knowledge of this happening. It does know, however, that both registrations result in an IDisposable object. This happens even though IMessageWriter does not implement IDisposable. The check is done at time of resolve. Each registration tracks its returned object, not knowing that this is the same object.

You can consider this to be a design flaw, because at the very least, MS.DI could have made sure that the list of disposable instance was deduplicated before calling Dispose on them. But that's probably a performance trade off, because:

Deduplicating the list could slow down the container
It's not often that this situation occurs
Dispose methods need to be idempotent anyway (as @spender mentioned in the comments), so it shouldn't be a problem when a dispose method is called twice

Would you have done the registration as follows, it would have lead to the Torn Lifestyle problem resulting in two ConsoleMessageWriter instances within a single scope:

// Incorrect registration. Leads to a Torn Lifestyle
services.AddScoped<ConsoleMessageWriter>()
services.AddScoped<IMessageWriter, ConsoleMessageWriter>();

MS.DI's design is simple (some would say naive) and the only way to prevent components from getting a Torn Lifestyle is making the registration you did in your question:

services.AddScoped<ConsoleMessageWriter>()
services.AddScoped<IMessageWriter>(p =>
    p.GetRequiredService<ConsoleMessageWriter>());

In other words: you have a registration that calls back into the container to resolve the actual component you are looking for.

More-mature DI Containers have more sophisticated ways of preventing Torn Lifestyles. Two that come to mind are Unity and Simple Injector. Simple Injector, for instance, would allow you to make the registration as follows while still ensuring one ConsoleMessageWriter instance per scope:

// Registrations using Simple Injector. No Torn Lifestyle here.
container.Register<ConsoleMessageWriter>(Lifestyle.Scoped);
container.Register<IMessageWriter, ConsoleMessageWriter>(Lifestyle.Scoped);

Unity IoC has similar behavior, while other DI Containers might require a similar construct to what you need with MS.DI.

If the duplicate dispose causes an issue, a way to circumvent this is by creating a non-disposable wrapper class for the IMessageWriter interface, e.g.:

public sealed class NonDisposableMessageWriterWrapper : IMessageWriter
{
    private readonly IMessageWriter decoratee;

    public NonDisposableMessageWriterWrapper(IMessageWriter decoratee) =>
       this.decoratee = decoratee;

    public void Write(string message) => this.decoratee.Write(message);
}

And change the registrations to the following:

services.AddScoped<ConsoleMessageWriter>()
services.AddScoped<IMessageWriter>(p =>
    new NonDisposableMessageWriterWrapper(
        p.GetRequiredService<ConsoleMessageWriter>()));

This way ConsoleMessageWriter is no longer disposed of twice.

But is this extra complexity worth it? Probably not.