Micha Schopman Micha Schopman - 3 months ago 15
ASP.NET (C#) Question

C# async, execution stopped, what is going on?

Some of the moments I actually know what I am doing; this isn't one of them.

I try to understand why the execution of my code stops the moment I execute a specific async operation. It works when executed synchronously, it doesn't when executed asynchronously.

This all happens in an MVC application;


  • AccountController
    creates a new Account, via an
    AccountService
    to keep my controllers more or less clean.

  • In the
    AccountService
    we do some things like a user account (ASP.NET Identity) and this works fine. Nothing special.

  • Then, I try to send an email "Welcome to your new account" and everything goes wrong.



I am not especially looking for a solution, I am more curious as to what is happening.

The moment I execute the await
EmailSender.Send(message);
in the
CreatePrimaryUser
method, the
account.AccountOwnerId = userId;
line is never executed anymore. There is no exception. It looks like everything went fine but the method just stopped executing.

Where did I go wrong?

AccountController

[HttpPost]
[ValidateAntiForgeryToken]
[AllowAnonymous]
public ActionResult Register(AccountViewModel model)
{
if (ModelState.IsValid)
{
try
{
BusinessServiceFacade.GetAccountService(RavenMasterSession).Create(model.ToModel(), model.SubscriptionPlanId);
return Redirect(string.Format("http://{0}.localhost:6257/Admin/GettingStarted",model.Name));
}
catch(AccountTakenException)
{
ModelState.AddModelError("", "Account name already taken");
}
catch (CustomException e)
{
ModelState.AddModelError("", "Error:" + e);
}
}
return View(model);
}


GetAccountService Create

public async Task<Account> Create(Account account, string subscriptionPlanId)
{
if (IsAccountNameTaken(account.Name))
throw new AccountTakenException();

RavenMasterSession.Store(account); // register account in MasterDB
CreateTenantDatabase(account); // create tenantDB

var userId = await CreatePrimaryUser(account); // create account owner (first user)
account.AccountOwnerId = userId;

var stripeAccountResult = BusinessServiceFacade.GetStripeService(RavenMasterSession).CreateCustomer(account); // register account in Stripe
account.CustomerId = stripeAccountResult.CustomerId;

AddSubscription(account, subscriptionPlanId); // add subscription to account

return account;
}


CreatePrimaryUser

private async Task<string> CreatePrimaryUser(Account account)
{
var user = new User()
{
UserName = account.AccountOwner.Email,
FirstName = account.AccountOwner.FirstName,
LastName = account.AccountOwner.LastName,
Email = account.AccountOwner.Email
};
RavenContext.GetTenantSession().Store(user);

var result = await UserManager.CreateAsync(user);
if (result.Succeeded)
{
var message = new MailMessage();
message.To.Add("recipient@domain.com");
message.Subject = "Thank you for joining ... ";
message.Body = string.Format("Your account is available at http://{0}.localhost:6257/Admin/GettingStarted", account.Name);

await EmailSender.Send(message);
}
else
{
RavenContext.GetTenantSession().Delete(user);
var stringList = String.Concat(result.Errors.ToArray());
throw new CustomException(stringList);
}
return user.Id;
}


EmailSender

public static async Task SendSES(MailMessage message)
{
const string FROM = "from@domain.com";
message.From = new MailAddress(FROM);

const string SMTP_USERNAME = "user";
const string SMTP_PASSWORD = "pass";
const string HOST = "email-smtp.eu-west-1.amazonaws.com";
const int PORT = 587;

using (SmtpClient client = new SmtpClient(HOST, PORT))
{
client.Credentials = new System.Net.NetworkCredential(SMTP_USERNAME, SMTP_PASSWORD);
client.EnableSsl = true;

try
{
//client.Send(message);
await client.SendMailAsync(message);
}
catch (Exception ex)
{
throw ex;
}
}
}

Answer

This is a symptom of mixing async sync somewhere which will cause a deadlock exactly as you describe. Change your calling code like below so its async all the way otherwise there is no real benefit to using async. Changing it like this is no problem as you are using MVC which supports this for action signatures.

[HttpPost]
[ValidateAntiForgeryToken]
[AllowAnonymous]
// change to async
public async Task<ActionResult> Register(AccountViewModel model)
{
    if (ModelState.IsValid)
    {
        try
        {
            var service = BusinessServiceFacade.GetAccountService(RavenMasterSession);

            // broken apart so you can see the await call
            await service.Create(model.ToModel(), model.SubscriptionPlanId);

            return Redirect(string.Format("http://{0}.localhost:6257/Admin/GettingStarted",model.Name));
        }
        catch(AccountTakenException)
        {
            ModelState.AddModelError("", "Account name already taken");
        }
        catch (CustomException e)
        {
            ModelState.AddModelError("", "Error:" + e);
        }
    }
    return View(model);
}

Side note 1

To rethrow an exception write only throw; and not throw ex;. Doing the later resets the call stack in the exception making it difficult to debug later on. To preserve the exception state/call stack use the former (best practice and always recommended)!

try {
    // something that could throw exception
} catch(Exception ex) {
    // do something with the exception like logging or retry or whatever
    // if the code does nothing but a rethrow then remove the whole try/catch as it has no added value
    throw;
}

Side note 2

For non-action methods (those methods not defined on your Mvc or Web API controllers) you should suffix the method with Async as a naming convention if the method is an asynchronous method. This makes it easy to see where in your code you can make use of await or some other Task mechanism to execute the code. You can see that Microsoft does this as well with the async methods built into the FCL. It is not required (there is nothing to enforce it) but it is good practice because it makes for more readable code.

  • Create becomes CreateAsync
  • CreatePrimaryUser becomes CreatePrimaryUserAsync
  • SendSES becomes SendSESAsync
Comments