Disposable object created in AroundReceive gets disposed during the middle of ReceiveAsync

[psandler]

I'm not sure this hasn't been reported before, as I see it is referenced in the following article:

https://havret.io/akka-entity-framework-core

At least until you start playing with AroundReceive, which isn’t an async method and simply returns before your handler does its async work.

I find AroundReceive to be incredibly useful, and being able to use it with Actors that make async calls seems pretty important.

Quick repro in case the problem isn't clear from the article:

class Program
{
    static void Main(string[] args)
    {
        var actorSystem = ActorSystem.Create("tempActorSystem");
        var actorRef = actorSystem.ActorOf(Props.Create<MyActor>(), "myActor");
        actorRef.Tell("hello");

        Console.ReadLine();
    }
}

public class MyActor : ReceiveActor
{
    public MyActor()
    {
        ReceiveAsync<string>(async message => await DoAsyncWork(message));
    }

    private async Task DoAsyncWork(string message)
    {
        await Task.Delay(2000);
        Console.WriteLine($"Finished DoAsynchWork at {DateTime.Now}");
    }

    protected override bool AroundReceive(Receive receive, object message)
    {
        base.AroundReceive(receive, message);
        Console.WriteLine($"Finished AroundReceive at {DateTime.Now}");
        return true;
    }
}

[Aaronontheweb]

I'm sorry for the long delay in getting back to you on this issue. We'll take a look at it.

[eaba]

Okay @Aaronontheweb @psandler @havret, I have been able to investigate this issue with success in this branch.

The console worked first attempt, no issue whatsoever overriding AroundReceive.

ReceiveAsync and AroundReceive Worked Together before AroundReceive was overridden, so why did it not work with this?:

protected override bool AroundReceive(Receive receive, object message)
{
    using (IServiceScope serviceScope = Context.CreateScope())
    {
        _bookstoreContext = serviceScope.ServiceProvider.GetService<BookstoreContext>();
        return base.AroundReceive(receive, message);
    }
}

Well, this error was a guide:

{[ERROR][11/30/2020 1:45:14 PM][Thread 0009][akka://bookstore/user/$a] 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.
Object name: 'BookstoreContext'.
Cause: System.ObjectDisposedException: 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.
Object name: 'BookstoreContext'.
   at Microsoft.EntityFrameworkCore.DbContext.CheckDisposed()
   at Microsoft.EntityFrameworkCore.DbContext.get_DbContextDependencies()
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Issue3691Aspnet.Domain.BooksManagerActor.<.ctor>b__1_0(CreateBook command) in C:\Users\Technical\source\repos\akka.net\src\Issue3691Aspnet\Domain\BooksManagerActor.cs:line 28}

_bookstoreContext was being disposed before it could be used in ReceiveAsync. This worked:

protected override bool AroundReceive(Receive receive, object message)
        {
            //using var serviceScope = Context.CreateScope();
            var serviceScope = Context.CreateScope();
            _bookstoreContext = serviceScope.ServiceProvider.GetService<BookstoreContext>();
            return base.AroundReceive(receive, message);
        }

@Aaronontheweb in this kind of situation, can the error be propagated to the client?:

[HttpPatch("{id}")]
[Route("update")]
public IActionResult Patch(Guid id, [FromBody] JsonPatchDocument<Book> patch)
        {
            _booksManagerActor.Tell(new UpdateBook(id, patch));
            return Accepted();
        }

I found something else:

The Log was being initialized(am thinking maybe the ActorSystem is being created) with each call to either of the API:

[Route("bookstore")]
    [ApiController]
    public class BooksController : Controller
    {
        private readonly IActorRef _booksManagerActor;

        public BooksController(/*BooksManagerActorProvider booksManagerActorProvider*/)
        {
            _booksManagerActor = Worker.BooksManager;
        }

        [Route("books")]
        public async Task<IActionResult> Get()
        {
            var books = await _booksManagerActor.Ask<IEnumerable<BookDto>>(GetBooks.Instance);
            return Ok(books);
        }

        [HttpGet("{id}")]
        [Route("book")]
        public async Task<IActionResult> Get(Guid id)
        {
            var result = await _booksManagerActor.Ask(new GetBookById(id));
            switch (result)
            {
                case BookDto book:
                    return Ok(book);
                case BookNotFound _:
                    return NotFound();
                default:
                    return BadRequest();
            }
        }

        //[HttpPost]
        [Route("generate")]
        public IActionResult Post(/*[FromBody] CreateBook command*/)
        {
            //no time to create UI - take shortcut. :)
            var command = new CreateBook($"{DateTime.Now.DayOfWeek}-{DateTime.Today.Ticks}", $"Aaron-{DateTime.Today.Ticks}", 105, 130);

            _booksManagerActor.Tell(command);
            return Accepted();
        }

        [HttpPatch("{id}")]
        [Route("update")]
        public IActionResult Patch(Guid id, [FromBody] JsonPatchDocument<Book> patch)
        {
            _booksManagerActor.Tell(new UpdateBook(id, patch));
            return Accepted();
        }

        [HttpDelete("{id}")]
        [Route("delete")]
        public IActionResult Delete(Guid id)
        {
            _booksManagerActor.Tell(new DeleteBook(id));
            return Accepted();
        }
    }

So I changed this:

/* Register the ActorSystem*/
           services.AddSingleton(provider =>
            {
                var serviceScopeFactory = provider.GetService<IServiceScopeFactory>();
                var actorSystem = ActorSystem.Create("bookstore", ConfigurationLoader.Load());
                actorSystem.AddServiceScopeFactory(serviceScopeFactory);
                return actorSystem;
            });

            services.AddSingleton<BooksManagerActorProvider>(provider =>
            {
                var actorSystem = provider.GetService<ActorSystem>();
                var booksManagerActor = actorSystem.ActorOf(Props.Create(() => new BooksManagerActor()));
                return () => booksManagerActor;
            });

To this:

public class Worker : BackgroundService
    {
        private readonly ActorSystem _actorSystem;
        public static IActorRef BooksManager;
        private readonly ILogger _logger;
        public Worker(ILogger<Worker> logger, IServiceProvider provider)
        {
            var serviceScopeFactory = provider.GetService<IServiceScopeFactory>();
            _actorSystem = ActorSystem.Create("bookstore", ConfigurationLoader.Load());
            _actorSystem.AddServiceScopeFactory(serviceScopeFactory);
            _logger = logger;
            var booksManager = _actorSystem.ActorOf(Props.Create(() => new BooksManagerActor()));
            SetBookManager(booksManager);
        }
        protected override async Task ExecuteAsync(CancellationToken stoppingToken)
        {
            while (!stoppingToken.IsCancellationRequested)
            {
                _logger.LogInformation("Worker running at: {time}", DateTimeOffset.Now);                
                await Task.Delay(1000, stoppingToken);
            }
        }
        public override async Task StopAsync(CancellationToken cancellationToken)
        {
            await _actorSystem.Terminate();
            await base.StopAsync(cancellationToken);
        }
        private static void SetBookManager(IActorRef manager)
        {
            BooksManager = manager;
        }
    }

[Aaronontheweb]

That makes perfect sense - the AroundReceive gets busted up because of how the await gets invoked. It's actually two separate message processing attempts (one before, one after the await) and the context that's being created in the AroundReceive does get disposed, but that exception occurs inside the Task - we'd need to modify the Awaiter code that Akka.NET uses to propagate that exception back.

But this syntax would never work inside Akka.NET, the reason being that the dispatcher model doesn't allow it.

[Aaronontheweb]

Looks like we catch the Exception here:

https://github.com/akkadotnet/akka.net/blob/ec01db19f1c49aecc6e3e828d264156a628ebadf/src/core/Akka/Dispatch/ActorTaskScheduler.cs#L149-L162

And it looks like we propagate that message when we are able to capture it inside the inner task:

https://github.com/akkadotnet/akka.net/blob/ec01db19f1c49aecc6e3e828d264156a628ebadf/src/core/Akka/Actor/ActorCell.DefaultMessages.cs#L337-L351

Any idea how we missed that exception in this case @eaba ?

[eaba]

The job of a using is to dispose disposables automatically, it did just that, and there was nothing akka.net could have done about that - and that is not a bug in Akka.net. I think it is the responsibility of developers to handle this situation properly. Doing this would have sufficed, I think:

public BooksManagerActor()
{
     var serviceScope = Context.CreateScope();
      //setup once instead of doing so at every new message
     _bookstoreContext = serviceScope.ServiceProvider.GetService<BookstoreContext>();
   //Receive follows
}
//the disposable can be disposed here:
protected override void PostStop()
{
            base.PostStop();
            _bookstoreContext.Dispose();
 }
//With this, there wont be any need to override `AroundReceive`

This issue is similar to the stackoverflow question "Exception Handling with akka.net PipeTo() on Task" and @Horusiath answered that adequately.

[Havret]

I think that the issue that @psandler mentioned here has nothing to do with using statement. From what I understand it is an implicit feature request for you guys to provide an async version of AroundReceive. Just take a look at the repro listing.

[eaba]

What would you do with an AroundReceiveAsync?

[Havret]

Probably the same things you may do with AroundReceive for Receive handlers.

[eaba]

One gain of using an Actor is that you get to process messages sequentially and that is why when ReceiveAsync was implemented, the mailbox was suspended until the current message has been processed and so with AroundReceiveAsync it is likely the mailbox will need to be suspended also. I don't know if @Aaronontheweb can speak on the possibility of having AroundReceiveAsync

[Aaronontheweb]

No point in around a AroundReceiveAsync - it's irrelevant to the actor whether or not the underlying Receive<T> returns a task or not. AroundReceive is an atomic operation that gets invoked whenever the actor processes a message. In this case, looks like the issue is AroundReceive doesn't get called because it was, theoretically, already called at the start of this Receive<T>.

The using block is absolutely the problem - you need to call using from inside the async block in order for the objects to not be disposed while the await is running out of band.

[Aaronontheweb]

The await state machine needs to be atomic - in this instance the user isn't deploying it that way. Just move the using block to inside the actor's async receive to resolve the issue.

Propagating the exception correctly appears to be the bug, but otherwise this is a user error.

Was the exception thrown inside the continuation task after the await @eaba ?

[Havret]

I'm not sure but back in the days when I was writing this article, Receive handlers were called from AroundReceive, if I remember correctly. So this code worked fine for non asynchronous scenarios. For instance if one needed to add tracing to the Receive handlers. After moving to async handlers it all started to falling apart.

[eaba]

@Aaronontheweb it is a user error and the exception is there to be handled by the user. There is a stackoverflow answer by @Horusiath on that here: https://stackoverflow.com/questions/48803038/exception-handling-with-akka-net-pipeto-on-task