0

We have a .Net Core 3.1 Web API that, towards the end of the request processing flow, deep in the services layer, needs to complete a fire-and-forget task.

Disclaimer: I am aware that the approach taken is generally considered less than ideal, but it has been accepted by the team, along with its risks.

Below is the current solution in a nutshell:

public class MyService
{
    public readonly IServiceA _serviceA;
    public readonly IServiceB _serviceB;
    
    public MyService(IServiceA serviceA, IServiceB serviceB)
    {
        _serviceA = serviceA;
        _serviceB = serviceB;
    }

    public void FireTheTask(MyData data)
    {
         _ = Task.Run(() => Submit(data));
    }

    public async Task Submit(MyData data)
    {
        try {
            var result = await _serviceA.CallSomeService(data); // uses HttpClientFactory

            if(!result.Success)
            {
                _serviceB.SaveData(data); // for offline processing
            }
        }
        catch (Exception ex)
        {
            // Log error to file here.
        }
    }
}

Both serviceA and serviceB are registered as transient and injected through DI.

There's technically a risk that the main request will complete sooner than Submit(), and serviceA and serviceB will no longer be available.

However, my consistent observation is that even if Submit() starts with, e.g., await Task.Delay(30000), it still successfully completes, 30 seconds after the main request has completed, and a response is returned from the API.

  1. Can anyone please comment on / explain this observation?

  2. If there's a risk of the above code not working reliably, would the solution below be correct (assuming no objects from the main request scope need to be referenced by the services)?

public class MyService
{
    public readonly IServiceScopeFactory _scopeFactory;
    
    public MyService(IServiceScopeFactory scopeFactory)
    {
        _scopeFactory = scopeFactory;
    }

    public void FireTheTask(MyData data)
    {
         _ = Task.Run(() => Submit(data));
    }

    public async Task Submit(MyData data)
    {
        try {
            using var scope = _scopeFactory.CreateScope();
            
            var serviceA = scope.ServiceProvider.GetRequiredService<IServiceA>();
            var serviceB = scope.ServiceProvider.GetRequiredService<IServiceB>();
            
            var result = await serviceA.CallSomeService(data); // uses HttpClientFactory

            if(!result.Success)
            {
                serviceB.SaveData(data); // for offline processing
            }
        }
        catch (Exception ex)
        {
            // Log error to file here.
        }
    }
}
  1. Would any transient services that serviceA and serviceB depend on (i.e., have injected via their constructors) be taken care of by this solution?

  2. Could this solution be further improved or simplified, and if so, how?

Thank you.

pikkabird
  • 125
  • 1
  • 9
  • 3
    “Both serviceA and serviceB are registered as transient” are they disposable? If not, then nothing happens, when request is processed, the Submit has references to the services and they are still alive – Viktor Arsanov Aug 28 '22 at 20:07
  • 1
    As an aside, I don't think you need to wrap the `Task` returned from `Submit` in another `Task`. Could you not simply call `Submit` and not `await` the returned `Task`? I.e `var _ = Submit(data);`. – WBuck Aug 28 '22 at 20:08
  • 1
    The solution with scope looks good to me – Viktor Arsanov Aug 28 '22 at 20:10
  • 1
    And also when they are not disposable, and dont have references to an object, which might have been disposed, eg DbContext – Viktor Arsanov Aug 28 '22 at 20:11
  • 1
    `Task.Run` should be pointless based on provided code and comments. – Guru Stron Aug 28 '22 at 20:33
  • 1
    @ViktorArsanov Thank you. The services, A and B, are not marked as disposable. From your two replies I understand that both solutions are technically acceptable, correct? – pikkabird Aug 28 '22 at 22:03
  • @WBuck @GuruStron Yes, I suppose I could just use ```var _ = Submit(data)```, thanks. Wasn't aware of this construct at the time of coding. Will likely switch to it when I get a chance. For the time being, though, do you think ```Task.Run()``` would cause an issue, or add significant overhead? – pikkabird Aug 28 '22 at 22:09
  • 1
    Right now you're just offloading the call to `Submit` to the thread pool. This **may** not be necessary. I say **may** because `Submit` could have a bunch of synchronous code that could impact UI responsiveness for example (but this does not appear to be the case with what you've showed). So queuing work on the thread pool is just a waste of time and resources. – WBuck Aug 29 '22 at 00:54

1 Answers1

1

As you mentioned in comments, the services referenced MyService are not disposable.

That is, when the DI scope is closed, nothing happens with those instances, and the code which has references to those services can access them.

However, you’re worrying right, that there might be issues with the services when they are transient and they are used like in your fire-and-forget example. You solution is right - the Submit method lifetime doesn’t depend on the Request lifetime, and vice versa, therefore yes, you create a separate scope for this.

I understand that both solutions are technically acceptable, correct

I see here only one solution - the one with scope. If you mean the original code, then it is a bad idea, even if it works now. It will break once you add any dependency to your service, which can be disposed(or its dependency, or … and so on).

So, answering the questions:

  1. See this answer
  2. Yes
  3. Yes
  4. I don’t see improvements here apart from syntax sugar, which everyone puts depending on the taste.

P.S. you can see this answer it may help to understand di lifecycle

Viktor Arsanov
  • 1,285
  • 1
  • 9
  • 17