1

I am working on an API and am having problems with making multiple calls to a service and it's different methods, I have each method creating and using new DBContext (or at least that's the intention), but after the first service call the others complain that the DBContext has been disposed, I was hoping you could point me in the right direction, because as far as I can see I am creating a new context for each of these calls - obviously I am doing something wrong here, any help would be much appreciated.

The actual error I am getting is "Cannot access a disposed object."

I know I can maybe pull the db interaction and context creation code out of the service and into the controller method here (it's a simplified example), but will need to use more services in other parts of the application and have encountered the problem there also, so would like to try and identify what is causing my problem in this example so that I can apply the fix elsewhere.

Here are the simplified classes involved.

public class UserController : Controller
    {
        private readonly IUserService userService;

        public UserController(IUserService userService)
        {
            this.userService = userService;
        }

        [HttpPost]
        [ActionName("PostUserDetails")]
        public async Task<IActionResult> PostUserDetails([FromBody]UserDetailsContract userDetailsContract)
        {
            // this call is fine
            var user = await userService.GetUserByCode(userDetailsContract.Code);

            if (user == null)
            {
                return BadRequest("User not found");
            }

            // this call fails with the object disposed error 
            var userDetails = await userService.GetUserDetailsByCode(userDetailsContract.Code);

            if (userDetails != null)
            {
                return BadRequest("UserDetails already exists");
            }

            // .. go on to save new entity

            return Ok();
        }
    }
public class UserService : IUserService
{
    private readonly IDatabaseFactory databaseFactory;

    public UserService(IDatabaseFactory databaseFactory)
    {
        this.databaseFactory = databaseFactory;
    }

    public async Task<User> GetUserByCode(string code)
    {
        using (var db = databaseFactory.Create())
        {
            return await db.Users.GetByCode(code);
        }
    }

    public async Task<IEnumerable<UserDetail>> GetUserDetailsByCode(string code)
    {
        using (var db = databaseFactory.Create())
        {
            return await db.UserDetails.GetByCode(code);
        }
    }   
}
public class ApiDbContext : DbContext, IApiDbContext
{
    public DbSet<User> Users { get; set; }

    public DbSet<UserDetail> UserDetails { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer(@"Server=192.168.1.1;Database=dbname;User Id=user; Password=pwd; MultipleActiveResultSets=True;");
    }
}
public class DatabaseFactory : IDatabaseFactory
{
    public IApiDatabase Create()
    {
        return new ApiDatabase(new ApiDbContext());
    }
}
public class ApiDatabase : RepositoriesBase, IApiDatabase
{
    private IUserRepository userRepository;
    private IUserDetailsRepository userDetailsRepository;

    public ApiDatabase(ApiDbContext context) : base(context)
    {
    }

    public IUserRepository Users => userRepository ?? (userRepository = new UserRepository(context));

    public IUserDetailsRepository UserExchange => userDetailsRepository ?? (userDetailsRepository = new UserDetailsRepository(context));
}
public abstract class RepositoriesBase : IRepositories
{
    internal readonly ApiDbContext context;

    private bool isDisposing;

    protected RepositoriesBase(ApiDbContext context)
    {
    }

    public void Dispose()
    {
        if (!isDisposing)
        {
            isDisposing = true;
            context?.Dispose();
        }
    }

    public Task SaveChanges() => context.SaveChangesAsync();
}

public class UserRepository : Repository<User>, IUserRepository
{
    public UserRepository(ApiDbContext context) : base(context)
    {
    }

    public async Task<User> GetByCode(string code)
    {
        return Filter(x => x.code == code).Result.FirstOrDefault();
    }
}
public class UserDetailsRepository : Repository<UserDetail>, IUserDetailRepository
{
    public UserExchangeRepository(ApiDbContext context) : base(context)
    {
    }

    public async Task<IEnumerable<UserDetail>> GetByUserId(int userId)
    {
        return await Filter(x => x.UserId == userId);
    }
}
public class Repository<T> : IRepository<T> where T : class, IEntity
{
    private readonly ApiDbContext context;

    public Repository(ApiDbContext context) => this.context = context;

    public async Task Add(T entity)
    {
        context.Set<T>().Add(entity);
    }

    public async Task Add(IEnumerable<T> entities)
    {
        foreach (var entity in entities)
        {
            context.Set<T>().Add(entity);
        }
    }

    public async Task Delete(T entity)
    {
        context.Set<T>().Remove(entity);
    }

    public async Task Delete(IEnumerable<T> entities)
    {
        foreach (var entity in entities)
        {
            context.Set<T>().Remove(entity);
        }
    }

    public async Task Delete(int id)
    {
        var entityToDelete = context.Set<T>().FirstOrDefault(e => e.Id == id);
        if (entityToDelete != null)
        {
           context.Set<T>().Remove(entityToDelete);
        }
    }

    public async Task Update(T entity)
    {
        context.Set<T>().Update(entity);
    }

    public async Task Edit(T entity)
    {
        var editedEntity = context.Set<T>().FirstOrDefault(e => e.Id == entity.Id);
        editedEntity = entity;
    }

    public async Task<IEnumerable<T>> GetAll(Expression<Func<T, bool>> predicate = null)
    {
        var query = context.Set<T>().Include(context.GetIncludePaths(typeof(T)));

        if (predicate != null)
        {
            query = query.Where(predicate);
        }

        return await query.ToListAsync();
    }

    public async Task<T> GetById(int id)
    {
        return context.Set<T>().FirstOrDefault(e => e.Id == id);
    }

    public async Task<IEnumerable<T>> Filter()
    {
        return context.Set<T>();
    }

    public virtual async Task<IEnumerable<T>> Filter(Func<T, bool> predicate)
    {
        return context.Set<T>().Where(predicate);
    }

    public async Task SaveChanges() => context.SaveChanges();
}

In my DI config I have DatabaseFactory and UserService defined as singletons.

Error: "Cannot access a disposed object."

More error details: " at Microsoft.EntityFrameworkCore.DbContext.CheckDisposed() at Microsoft.EntityFrameworkCore.DbContext.get_DbContextDependencies()
at Microsoft.EntityFrameworkCore.DbContext.get_Model() at Microsoft.EntityFrameworkCore.Internal.InternalDbSet1.get_EntityType() at Microsoft.EntityFrameworkCore.Internal.InternalDbSet1.get_EntityQueryable() at Microsoft.EntityFrameworkCore.Internal.InternalDbSet1.System.Collections.Generic.IEnumerable<TEntity>.GetEnumerator() at System.Linq.Enumerable.WhereEnumerableIterator1.MoveNext() at System.Linq.Enumerable.Any[TSource](IEnumerable1 source, Func2 predicate) at App.Api.Controllers.UserController.PostUserDetail(UserDetailContract userDetailContract) in D:\Repositories\application\src\App\Api\Controllers\UserController.cs:line 89"

Thank you

JuanR
  • 7,405
  • 1
  • 19
  • 30
  • `Task GetByCode(string code)` is mixing asynchronous calls with `.Result` which is bad practice. Go async all the way or don't do it at all. You could easily change that method's body to `return (await Filter(x => x.code == code)).FirstOrDefault();` or remove things like `Filter` in favor of using the DbSet types or IQueryable types directly which would be my preference. Don't abstract an abstraction. – Igor Jun 19 '19 at 20:12
  • Would this help? https://stackoverflow.com/questions/50788272/how-to-instantiate-a-dbcontext-in-ef-core/50788386#50788386 – Stefan Jun 19 '19 at 20:16
  • @Igor, DbContext is leaking abstraction (leaking in technical and domain related ways). Having "pure" abstraction will save some headaches. But you are right - OP's implementation is just an extra (redundant) wrapper around DbContext – Fabio Jun 19 '19 at 20:22
  • Where exception is thrown, which line of code? Can you show `Filter` method in `UserDetailsRepository`? Error message clearly stays, that you are using already disposing object, check where repository returns "not materialized" query. – Fabio Jun 19 '19 at 20:33
  • Can you include the code of `Repository` in your question? – pfx Jun 19 '19 at 20:50
  • Hi and welcome to SO. Can you please post the code for the `Filter` method? – JuanR Jun 19 '19 at 21:21
  • Hey thanks, I have now included my Repository and Filter method. Also know that the .Result thing isn't ideal, without it was giving me a syntax error that I need to address in the future. I did change it to put the result into a var and then returning the var also, but didn't solve this error unfortunately – user3378787 Jun 19 '19 at 21:24
  • 1
    The one line that catches my eye is: `return await Filter(x => x.UserId == userId);`. Consider using `ToArray()` at the end to materialize the objects. – JuanR Jun 19 '19 at 21:26
  • Also added extra details on the error as well, hopefully helps. thanks – user3378787 Jun 19 '19 at 21:26
  • @JuanR Thank you, the ToArray() solved it, such a little thing was the cause of the issues! Have also corrected the .Result stuff too, realised I needed to wrap the result in parenthesis thanks to Igor – user3378787 Jun 19 '19 at 21:34
  • @user3378787: Awesome. I posted a more detailed answer. – JuanR Jun 19 '19 at 21:36

2 Answers2

0

I think you may be a victim of delayed execution. The following piece of code creates an instance of of ApiDatabase which in turn creates a new ApiDbContext:

public IApiDatabase Create() //in DatabaseFactory
{
    return new ApiDatabase(new ApiDbContext());
}

I detect a code smell here, by the way, as ApiDbContext is disposable so you should be tracking this reference and disposing of it properly.

Anyways, ApiDatabase is disposable since it's wrapped in a using statement, so I think the the context is being disposed after the call to GetByUserId:

public async Task<IEnumerable<UserDetail>> GetByUserId(int userId)
{
    return await Filter(x => x.UserId == userId);
}

Notice you are returning an enumeration. I think it may not be materialized by the time you use it, hence the error. Add a cast to an array to force materialization:

return await Filter(x => x.UserId == userId).ToArray();
JuanR
  • 7,405
  • 1
  • 19
  • 30
0

Your problem is the signature of this method:

public async Task<IEnumerable<UserDetail>> GetUserDetailsByCode(string code)
{
  using (var db = databaseFactory.Create())
  {
    return await db.UserDetails.GetByCode(code);
  }
}   

IEnumerable<T> is an enumerable, which are generally lazy-evaluated. In the meantime, the Task<T> is considered complete once the enumerable is defined (not when it is completed). And the context is disposed once that enumerable is defined. You would have the same problem if the code was synchronous.

The fix is to "reify" (evaluate) the enumerable before the context is disposed:

public async Task<IReadOnlyCollection<UserDetail>> GetUserDetailsByCode(string code)
{
  using (var db = databaseFactory.Create())
  {
    return await db.UserDetails.GetByCode(code).ToList();
  }
}   
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810