ASP.Net Core MVC Repository Pattern Unexpectedly disposing

Question

When I try to add a comment, I get the following error:

ObjectDisposedException: Cannot access a disposed object.

When the code runs the second line:

m_context.Comments.Add(comment);
m_context.SaveChanges();

Why is the context being disposed? If move the the TryAddComment method into the controller, it doesn't call Dispose early.

Here is what my Controller and Repository class look like (simplified).

CommentsController.cs:

public class CommentsController : Controller
{

    private ICommentRepository m_commentRepository;

    public CommentsController(ICommentRepository commentRepository)
    {
        m_commentRepository = commentRepository;
    }

    // POST: api/Comments
    [HttpPost]
    public async Task<IActionResult> PostComment([FromBody] CommentAddViewModel commentVM)
    {
        Comment comment = new Comment
        {
            ApplicationUserId = User.GetUserId(),
            PostId = commentVM.PostId,
            Text = commentVM.Text
        };

        bool didAdd = m_commentRepository.TryAddComment(comment);

        if (!didAdd)
        {
            return new HttpStatusCodeResult(StatusCodes.Status409Conflict);
        }

        return CreatedAtRoute("GetComment", new { id = comment.CommentId }, comment);
    }

}

CommentRepository.cs:

public class CommentRepository : ICommentRepository, IDisposable
{

    public ApplicationDbContext m_context;

    public CommentRepository(ApplicationDbContext context)
    {
        m_context = context;
    }
    public bool TryAddComment(Comment comment)
    {
        m_context.Comments.Add(comment);
        m_context.SaveChanges();

        return true;
    }
    private bool disposed = false;

    protected virtual void Dispose(bool disposing)
    {
        if (!this.disposed)
        {
            if (disposing)
            {
                m_context.Dispose();
            }
        }
        this.disposed = true;
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
}

Edit:

If I use a local CommentRepository, it works as expected. For example:

    CommentRepository localCommentRepo = new CommentRepository(m_context);
    bool didAdd = localCommentRepo.TryAddComment(comment);

Edit2:

In Startup.cs, I registered the IcommentRepository as Scoped and works as expected. Originally it was Singleton. Why would a singleton cause this issue?

services.AddSingleton<ICommentRepository, CommentRepository>(); //breaks
services.AddScoped<ICommentRepository, CommentRepository>(); //works

Edit3:

ApplicationDbContext.cs:

public class ApplicationDbContext : IdentityDbContext<ApplicationUser>
{
    protected override void OnModelCreating(ModelBuilder builder)
    {
        base.OnModelCreating(builder);
        // Customize the ASP.NET Identity model and override the defaults if needed.
        // For example, you can rename the ASP.NET Identity table names and more.
        // Add your customizations after calling base.OnModelCreating(builder);

    }
    public DbSet<Post> Posts { get; set; }
    public DbSet<Comment> Comments { get; set; }
}

Answer 1

Neither your repository nor your DbContext should be singletons. The correct way to register them is services.AddScoped or services.AddTransient, as a DbContext shouldn't live longer than a request and the AddScoped is exactly for this.

AddScoped will return the same instance of a DbContext (and repository if you register it as such) for the lifetime of the scope (which in ASP.NET Core equals the lifetime of a request).

When you use AddScope you shouldn't dispose the context yourself, because the next object that resolves your repository will have an disposed context.

Entity Framework by default registers the context as scoped, so your repository should be either scoped (same lifetime as the context and request) or transient (each service instance gets it's own instance of the repository, but all repositories within a request still share the same context).

Making the context singleton causes serious issues, especially with the memory (the more you work on it, the more memory the context consumes, as it has to track more records). So a DbContext should be as short-lived as possible.

Duration of the context has the advantage that you can still roll back all operations during the request if something goes wrong and handle it as one single transaction.