I have a WPF application with the viewmodel calling a data access class to do database work. The DataAccess class has all async functions and the VM uses await _dataAccess.DoWork(withItem); Occsionally I'm getting an error that two operations are being done on the same context. This shouldn't be possible unless the service provider is using the same context in different calls to GetService.

In App.xaml.cs

services.AddDbContext<MyItemDbContext>(options =>
{
    options.UseLoggerFactory(_loggerFactory);
    options.UseSqlServer(config.GetConnectionString("My_Items_ConnectionString"));
});
// register concrete class for IDataAccess
services.AddScoped<IDataAccess, ItemDataAccess>();

In View code

private async void BtnReady_Click(object sender, RoutedEventArgs e)
{
    await SetStatusForSelectedItem(StatusIds.Ready);
}
private async Task SetStatusForSelectedItems(StatusIds statusId)
{
    // get selected itemDetail item from grid
    await _mainWindowVM.UpdateItemDetailState(itemDetail, stateId);
}

In ViewModel code

private readonly IDataAccess _dataAccess;
public MainWindowVM(IDataAccess dataAccess)
{
    _dataAccess = dataAccess;
}
public async Task UpdateItemDetailState(Item item, StatusIds stateId)
{
    // other necessary code
    item.StateId = stateId;
    await _dataAccess.UpdateItem(item);
}

In DataAccess (DalBase) code

private readonly IServiceProvider _serviceProvider;
public DalBase(IServiceProvider serviceProvider)
{
    _serviceProvider = serviceProvider;
}

protected MyItemDbContext Get_Item_DbContext()
{
    return _serviceProvider.GetService<MyItemDbContext>();
}
// other contexts for different purposes

In DataAccess (derived)

public ItemDataAccess(IServiceProvider serviceProvider) : base(serviceProvider) { }

public async Task UpdateItem(Item item)
{
    MyItemDbContext db = Get_Item_DbContext(); // <<< === should be new instance each time, right?
    db.Items.Update(item);
    await db.SaveChangesAsync();
}

I'm following this pattern throughout except that I took out a lot of try...catch to cut the size. Each of the functions in the DataAccess starts out with a call to Get_Item_DbContext() I used to have have using (var db = Get_Item_DbContext()) but an article I read said to let dependency injection decide on the lifespan. I can't find any place where I'm not using await on an async function but I get an error like this...

A second operation was started on this context before a previous operation completed...

There's one operation that's working through the list and it calls this update status function, and there's a button that the user can use to call the same function. That's when I get the error.

EDIT I used to have using (var db = Get_Item_DbContext) but then I got a different error... Cannot access a disposed context instance.

UPDATE I tried having the context injected into DalBase. I still get the error when I click run the second operation. In this case it's probably because both operations run through the same VM which now has one _dataAccess and a single context that was injected. I was trying to avoid that by getting a new instance from each function.

So my questions are these How can I find out which threads (and their call stacks) are getting the same context? Why do two separate threads (from async calls) getting the same context from GetService? Is there an option that I have to add to the AddDbContext to make it scope correctly?

Thanks Mike

CodePudding user response：

I would recommend stop injecting ServiceProvider in classes and getting your service like that. I would call that an anti pattern (Service Locator). Instead inject your dependency directly in the classes.

So instead:

In DataAccess (DalBase) code

private readonly MyItemDbContext _myItemDbContext;
public DalBase(MyItemDbContext myItemDbContext)
{
    _myItemDbContext = myItemDbContext;
}

// Don't expose the myItemDbContext from this class. Just use it to what you need

// other contexts for different purposes

In DataAccess (derived)

private readonly MyItemDbContext _myItemDbContext;
public ItemDataAccess(MyItemDbContext myItemDbContext)
{
    _myItemDbContext = myItemDbContext;
}

public async Task UpdateItem(Item item)
{
    _myItemDbContext.Items.Update(item);
    await _myItemDbContext.SaveChangesAsync();
}

I would think your problem will go away if you dont expose the usage of your MyItemDbContext to other classes.

CodePudding user response：

Being a WPF application requires a bit of a different perpective on managing EF DbContexts. Many examples out there that talk about Dependency Injection and lifetime scopes are referring to web applications which generally have a clear-cut lifetime scope defined, being the web request. In those situations you can register your DbContext directly with the container with a "Per Request" lifetime scope and everything simply works. With WPF there is no clear-cut scope as views are rendered and responding to events. So you have to define the lifetime scope for a DbContext explicitly. Usually this is done through a Unit of Work pattern where your dependency injector can provide a UoW scope factory.

There is nothing inherently wrong with using a using(var context = new AppDbContext()) approach, but there is a very important detail that you must adhere to: Entity instances read within the scope of the DbContext must remain within the scope of that DbContext or else be detached and treated in an as-is state from that point forward. This means that functionality like Lazy Loading which can fetch related data on-demain only works so long as the DbContext is in scope.

When you have code like:

using(var context = new AppDbContext())
{
    var order = context.Orders.Single(x => x.OrderId == orderId);
    orderForm.Model = order;
}

or something similar where we define a context scope, fetch an entity, then share that reference to anything, we are allowing that entity reference to leave the scope of the DbContext that spawned it. That entity is now a detached entity in a orphaned state. We can access it's properties, but the minute we try to access any navigation property that wasn't eager-loaded or filled in by the DbContext prior to being loaded, we will get an error. The entity still thinks it is attached to a DbContext but that DbContext is disposed. We can explicitly detach it:

using(var context = new AppDbContext())
{
    var order = context.Orders.Single(x => x.OrderId == orderId);
    context.Entity(order).State = EntityState.Detached;
    orderForm.Model = order;
}

or load it without tracking:

using(var context = new AppDbContext())
{
    var order = context.Orders.AsNoTracking().Single(x => x.OrderId == orderId);
    orderForm.Model = order;
}

Which will effectively do the same thing, returning a detached entity. However now, if you attempt to access an unloaded navigation property you will encounter #nulls. This can be problematic because does a #null mean that order doesn't actually have a reference, or just that it was not pre-fetched?

Your change to have a service locator provide a DbContext on-demand avoids the "out of scope" problem, but the problem now becomes that every call to Get_Item_DbContext() is going to return that single DbContext instance and you are bound to encounter Cross-threading exceptions if you are attempting to run operations asynchronously or via worker threads. To solve this problem using the service locator would require you to explicitly manage the scope of the DbContext and know when a shared DbContext instance should be provided vs. a new DbContext instance. This still can lead to problems where entities are passed between these scopes. (I.e. loaded on a worker thread and returned to another thread that had a different DbContext instance.)

Working with detached entities should be avoided wherever possible. Not only do they lead to errors and potentially invalid data state, but they also represent a risk of overwriting data with stale values. Lazy loading is a convenient crutch for a system to fall back on to load data on-demand if and when it is needed, but designing around it means your application will be hobbling around with a lot of SELECT n 1 performance issues. My recommendation when it comes to designing systems, whether web or WPF Windows applications is to leverage POCO view models for all data that leaves a DbContext scope, and keep those DbContext instances open only as long as absolutely necessary.

using(var context = new AppDbContext())
{
    var orderViewModel = context.Orders
        .Where(x => x.OrderId == orderId)
        .Select(x => new OrderViewModel 
        {
            // Populate view model with just the fields about the order and related data that the view needs.
        }).Single();
    orderForm.Model = orderViewModel;
}

Or with Automapper:

//Note: Configuration can be centralized elsewhere, and can include any/all mappings for the entire root aggregate (Order and it's associated entities)
var config = new MapperConfiguration(cfg => cfg.CreateMap<Order, OrderViewModel>());

using(var context = new AppDbContext())
{
    var orderViewModel = context.Orders
        .Where(x => x.OrderId == orderId)
        .ProjectTo<OrderViewModel>(config)
        .Single();
    orderForm.Model = orderViewModel;
}

While a disconnected entity can have required navigation properties eager loaded and passed to a view, projecting down to a view model can produce much more efficient queries, future-proofs the system from unintentional errors or performance issues etc. when new relationships are appended, and helps avoid confusion when passing entity references around where methods may get attached or detached entities in various levels of completion. (You can easily differentiate between the view model and the associated entity)