Mediatr: Problem with using Wait() method in ASP.NET MVC.

Created on 12 Feb 2017  Â·  7Comments  Â·  Source: jbogard/MediatR

The issue is clearly illustrated by this code:

public class TestCommand: IRequest
{ }
public class TestHandler : IAsyncRequestHandler<TestCommand>
{
    public Task Handle(TestCommand message)
    {
        return Task.Factory.StartNew(() =>
        {
            throw new ApplicationException();
        });
    }
}
public class HomeController : Controller
{
    private readonly IMediator _mediator;

    public HomeController(IMediator mediator)
    {
        _mediator = mediator;
    }

    public  ActionResult Index()
    {
        try
        {
            _mediator.Send(new TestCommand()).Wait();  // deadlock 
        }
        catch (Exception e)
        {
            // unreachable block 
        }

        return View();
    }
}

If i use await operator catch block is reachable:

```
try
{
await _mediator.Send(new TestCommand());
}
catch (Exception e)
{
// OK
}

but when use `Wait()` method  i have a problem, because `catch` block is unreachable.

 ```
try
{
    _mediator.Send(new TestCommand()).Wait(); // deadlock 
}
catch (Exception e)
{
     // unreachable block 
}

Most helpful comment

Perhaps I found a lines of code which can cause problems, for example the deadlock. I think we needs to use ConfigureAwait(false).

RequestHandler.cs

RequestHandler.cs

RequestPostProcessorBehavior.cs

RequestPostProcessorBehavior.cs

RequestPreProcessorBehavior.cs

RequestPreProcessorBehavior.cs

P.S. This can be useful Don't Block on Async Code

All 7 comments

Why are you waiting at all? Make your controller method async.

This isn't an issue specific to MediatR.

Yes, of course, you are right, i can make controller method async. But i want to understand why it doesn't work when i call Wait() method.

For example, if i use request/response scenarios with Wait() method, code works well.

public class Result
{  }

public class TestCommandWithResult : IRequest<Result>
{  }

public class TestHandlerWithResult : IAsyncRequestHandler<TestCommandWithResult, Result>
{
    public Task<Result> Handle(TestCommandWithResult message)
    {
        return Task.Factory.StartNew(() =>
        {
            throw new ApplicationException();
            return new Result();
        });
    }
}

public class HomeController : Controller
{
        public  ActionResult Index()
        {
               try
               {
                    _mediator.Send(new TestCommandWithResult()).Wait();
               }
               catch (Exception e)
               {
                    // OK 
               }

               return View();
        }
}

Perhaps I found a lines of code which can cause problems, for example the deadlock. I think we needs to use ConfigureAwait(false).

RequestHandler.cs

RequestHandler.cs

RequestPostProcessorBehavior.cs

RequestPostProcessorBehavior.cs

RequestPreProcessorBehavior.cs

RequestPreProcessorBehavior.cs

P.S. This can be useful Don't Block on Async Code

I agree about the ConfigureAwait(false). MediatR doesn't depend on the context, so there's no need to have continuations run on the calling context. If a higher level does depend on context, that'll still work as-is.

:thumbsup:

I'm not too strong on this async/await stuff as you can probably tell lol

On Tue, Feb 14, 2017 at 2:33 AM, Michael Croes notifications@github.com
wrote:

I agree about the ConfigureAwait(false). MediatR doesn't depend on the
context, so there's no need to have continuations run on the calling
context. If a higher level does depend on context, that'll still work as-is.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/jbogard/MediatR/issues/137#issuecomment-279640333,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGYMpAIHLy5bFG7BaZqowPReZ_NkON0ks5rcWbTgaJpZM4L-nYO
.

ConfigureAwait(false) is the way to go.

Unless a library specifically needs to run the callback on the captured context (i.e. the UI thread) ConfigureAwait(false) should be used on ALL awaits.

This is most unlikely, since most libraries are not aware of where/how they are used, so there's almost never a reason to capture the context, hence the use of ConfigureAwait(false) on all awaits to avoid unexpected dead-locks.

I was looking at applying ConfigureAwait(false) on all awaits, but I did notice that it has a theoretical impact on the pipeline. If one would perform IMediator.Send(...) on the UI thread, the pipeline is also on the UI thread (which I guess is not what you'd want, but that's my opinion). So if one would have a pipeline which would update some request counter in the UI, it would need additional code to switch to the UI SynchronizationContext. Fortunately that last part can be done with a custom awaiter, so in the pipeline one could write await myForm; or await myControl; to achieve this.

As such I'm going to apply ConfigureAwait(false) on all current awaits, I hope the explanation above provides enough reason to merge this.

Was this page helpful?
0 / 5 - 0 ratings