I have a Pay endpoint in an ASP.NET Web API project. It calls several services, including DiscountService in the application layer). This service has 5 methods.

I've added PayLoggerService class in DiscountService and I'm calling it's LogMessage() in 4 different places in the DiscountService.

This is the sequence of methods executed in one request:

Pay() 
    -> DiscountService -> DiscountService.A() 
       -> PayLoggerService.LogMessage() 
       -> DiscountService.B() -> PayLoggerService.LogMessage().

Problem :

DbContext is getting disposed if I call SaveChangesAsync() after every LogMessage().

Error :

Cannot access a disposed context instance. A common cause of this error is disposing a context instance that was resolved from dependency injection and then later trying to use the same context instance elsewhere in your application. This may occur if you are calling 'Dispose' on the context instance, or wrapping it in a using statement. If you are using dependency injection, you should let the dependency injection container take care of disposing context instances

This happens after DiscountService.A() calls LogMessage(), since the LogMessage() calls SaveChangesAsync() at the end:

public class DiscountService
{
    private readonly PayLoggerService _payLoggerService;

    public DiscountService(PayLoggerService payLoggerService)
    {
        _payLoggerService = payLoggerService;
    }

    // NOT ASYNC. Has too many references to make it async now.
    public string A()
    {
        // Do some work
        _payLoggerService.LogMessage(message).ConfigureAwait(false);
    }

    // This will not be entered if LogMessage(executed from A(), will call SaveChangesAsync())
    public string B()
    {
        // Do some work
        _payLoggerService.LogMessage(message).ConfigureAwait(false);
    }
}

public class PayLoggerService : GeneralService
{
    public PayLoggerService(PayingContext dbContext) : base(dbContext)
    {
    }

    public async Task LogMessage(string message)
    {
        // Irrelevant code

        var bettingHistory = new BettingHistory
                                 {
                                     // Irrelevant code
                                 };

        // DbScope is of type PayingContext, created in GeneralService.cs
        await DbScope.PayLogs.AddAsync(bettingHistory);
        await DbScope.SaveChangesAsync(); // This line causes an error. 
    }
}

Possible solutions I have thought of :

• Implement unit of work and call SaveChanges from there in Pay() endpoint
• Implement DbContextFactory and make a new instance of DbContext every time I need it

Is there anything better I could do?

I have a Pay endpoint in an ASP.NET Web API project. It calls several services, including DiscountService in the application layer). This service has 5 methods.

I've added PayLoggerService class in DiscountService and I'm calling it's LogMessage() in 4 different places in the DiscountService.

This is the sequence of methods executed in one request:

Pay() 
    -> DiscountService -> DiscountService.A() 
       -> PayLoggerService.LogMessage() 
       -> DiscountService.B() -> PayLoggerService.LogMessage().

Problem :

DbContext is getting disposed if I call SaveChangesAsync() after every LogMessage().

Error :

Cannot access a disposed context instance. A common cause of this error is disposing a context instance that was resolved from dependency injection and then later trying to use the same context instance elsewhere in your application. This may occur if you are calling 'Dispose' on the context instance, or wrapping it in a using statement. If you are using dependency injection, you should let the dependency injection container take care of disposing context instances

This happens after DiscountService.A() calls LogMessage(), since the LogMessage() calls SaveChangesAsync() at the end:

public class DiscountService
{
    private readonly PayLoggerService _payLoggerService;

    public DiscountService(PayLoggerService payLoggerService)
    {
        _payLoggerService = payLoggerService;
    }

    // NOT ASYNC. Has too many references to make it async now.
    public string A()
    {
        // Do some work
        _payLoggerService.LogMessage(message).ConfigureAwait(false);
    }

    // This will not be entered if LogMessage(executed from A(), will call SaveChangesAsync())
    public string B()
    {
        // Do some work
        _payLoggerService.LogMessage(message).ConfigureAwait(false);
    }
}

public class PayLoggerService : GeneralService
{
    public PayLoggerService(PayingContext dbContext) : base(dbContext)
    {
    }

    public async Task LogMessage(string message)
    {
        // Irrelevant code

        var bettingHistory = new BettingHistory
                                 {
                                     // Irrelevant code
                                 };

        // DbScope is of type PayingContext, created in GeneralService.cs
        await DbScope.PayLogs.AddAsync(bettingHistory);
        await DbScope.SaveChangesAsync(); // This line causes an error. 
    }
}

Possible solutions I have thought of :

• Implement unit of work and call SaveChanges from there in Pay() endpoint
• Implement DbContextFactory and make a new instance of DbContext every time I need it

Is there anything better I could do?

• c#
• .net
• asynchronous
• entity-framework-core

• The "Async" naming convention (I.e. SaveChangesAsync vs SaveChanges) is there for a reason so that synchronous code does not accidentally call asynchronous methods. Definitely get into the habit of adopting that naming convention to avoid issues like this which can send you chasing gypsy wagons. – Steve Py Commented 2 days ago

1 Answer 1

Is there anything better I could do?

Yes. Make both DiscountService.A and DiscountService.B async. Until you do that almost everything is a hack.

If you can't go with async for now then use sync API for saving changes via DbContext.SaveChanges. There are two options here either creating two versions of the LogMessage - sync and async ones:

public void LogMessage(string message)
{
    // ...
    //DbScope is of type PayingContext. Made in GeneralService.cs
    DbScope.PayLogs.Add(bettingHistory);
    DbScope.SaveChanges(); //This line causes an error. 
}

public async Task LogMessageAsync(string message)
{
    // ...
    DbScope.PayLogs.Add(bettingHistory);
    await DbScope.SaveChangesAsync(); //This line causes an error. 
}

Or pass the parameter:

public async ValueTask LogMessageAsync(string message, bool useAsync)
{
    // ...
    DbScope.PayLogs.Add(bettingHistory);
    if (useAsync)
    {
        await DbScope.SaveChangesAsync();
    }
    else
    {
        DbScope.SaveChanges();
    }
}

If this approach won't work as last resort (though I would hugely recommend against it) you can look into How to call asynchronous method from synchronous method in C#?

P.S.

1. Note that using AddAsync is needed only when you are using the HiLo if you are not using it - there is no point in calling AddAsync over Add. From the docs:

   This method is async only to allow special value generators, such as the one used by Microsoft.EntityFrameworkCore.Metadata.SqlServerValueGenerationStrategy.SequenceHiLo, to access the database asynchronously. For all other cases the non async method should be used.

2. Disposal of context not the only problem here. You can have concurrency issues - when one method is not awaited then the following can try to start another operation on the same context. While this can be fixed with a separate context created, there are other issues - unobserved fire-and-forget tasks in general is not a approach for multiple reasons.