CODEWITHSUNDEEP
c#async-awaitc#-5.0
zam6ak
zam6ak

Reputation: 7259

Is it safe to call .ConfigureAwait(false) in finally block?

I have a "rest client" that wraps HttpClient and whose methods are async. Besides other reasons, I need to control signin/signout process with my rest client so that number of sessions is not exceeded.

The rest client implements IDisposable and upon disposing the client I need to check if the client is "still signed in" and sign out if it is. Since doing any kind of external calls in Dispose method is considered bad practice, I have something as following

public class MappingsController : RestController
{
    [HttpGet]
    public async Task<HttpResponseMessage> GetYears()
    {
        return await ProcessRestCall(async rc => await rc.GetYearsAsync());
    }
}

public class RestController : ApiController
{
    protected async Task<HttpResponseMessage> ProcessRestCall<T>(Func<RestClient, Task<T>> restClientCallback)
    {
        RestClient restClient = null;
        try
        {
            var credentials = GetCredentialsFromRequestHeader();
            if (credentials == null)
            {
                return Request.CreateErrorResponse(HttpStatusCode.Unauthorized, "Missing credentials from header!");
            }
            var username = credentials["Username"];
            var password = credentials["Password"];

            restClient = new RestClient(username, password);
            var authenticated = await restClient.SignInAsync();
            if (!authenticated)
            {
                return CreateErrorResponseWithRestStatus(HttpStatusCode.Unauthorized, restClient);
            }
            var result = await restClientCallback(restClient);
            // Following works, but since I need to do it in finally block in case exception happens, perhaps It should be done in finally anyways...
            //await restClient.SignOutAsync(); 
            var response = Request.CreateResponse(HttpStatusCode.OK, result);
            return response;
        }
        catch (Exception e)
        {
            return CreateErrorResponseWithRestStatus(HttpStatusCode.BadRequest, restClient, e);
        }
        finally
        {
            if (restClient != null)
            {
                if (restClient.IsSignedIn)
                {
                    //var signedOutOk = restClient.SignOutAsync();//.Result; //<-- problem - this blocks!!!
                    restClient.SignOutAsync().ConfigureAwait(false); // seems to work, but I am not sure if this is kosher + I can't get return var

                    //Logger.Warn(CultureInfo.InvariantCulture, m => m("Client was still signed in! Attempt to to sign out was {0}", signedOutOk ? "successful" : "unsuccessful"));
                }
                restClient.Dispose();
            }
        }
    }
}

Upvotes: 1

Views: 830

Answers (1)

Servy
Servy

Reputation: 203836

The use of .ConfigureAwait(false) is a non-issue. You aren't awaiting on the task at all. Since you don't await it, it doesn't matter what await is configured to do.

What you're doing is just basic fire and forget (which may or may not be acceptable for you).

You should remove the ConfigureAwait(false) no matter what, just because it does nothing and is confusing to the reader. If it's okay for you to send the request to sign out but not actually sign out, then this is okay.

If you need to ensure that restClient.Dispose(); isn't called until the sign out request returns, then you have a bit of a...problem. The problem stems from the fact that the sign out request might be unsuccessful, or much worse, it might not respond at all. You'd need some way of dealing with that.

You can't use await in a finally block, but you can more or less mimic its behavior through continuations. You may need to do something like this:

public static async Task DoStuff()
{
    IDisposable disposable = null;
    try { }
    finally
    {
        var task = GenerateTask();
        var continuation = Task.WhenAny(task, Task.Delay(5000))
            .ContinueWith(t =>
            {
                if (task.IsCompleted) //if false we timed out or it threw an exception
                {
                    var result = task.Result;
                    //TODO use result
                }

                disposable.Dispose();
            });
    }
}

Note that since you aren't using await the task returned from DoStuff will indicate that it is "done" as soon as it hits the finally block for the first time; not when the continuation fires and the object is disposed. That may or may not be acceptable.

Upvotes: 4

Related Questions