Sign in

Dajbych.net


One antipattern that can make asynchronous programming really unpleasant

, 5 minutes to read

net2015 logo

I spent half a day looking for the cause of why my code was simply doing what it wanted. It behaved non-deterministically. It served one endpoint on the web and occasionally returned HTTP status 200, even though I had written the order to return state 201. In the end, I found the error and fixed it. I always think about the cause of the errors so that I don't repeat the same errors over and over again. But I came to the conclusion that it would require either a redesign of half of the .NET Framework classes or a new keyword in C#.

Object-oriented programming is based on the fact that objects are changed and it depends on the order in which the changes are made. In comparison, functional programming creates structures from which, once created, it can only be read. This has its indisputable advantage. It does not matter in the order in which the individual commands are executed. A command can simply be executed when all its input data has been calculated.

Asynchronous programming, as useful and valued as it is in C#, has the disadvantage that it can very easily obscure the order in which instructions are executed. Appropriate program design can prevent this, but the compiler will not warn you that you are asking about generating random output data.

I'll explain this with an example. ASP.NET has a class HttpTaskAsyncHandler from which it inherits a generic handler that overloads the method:

async Task ProcessRequestAsync(HttpContext c)

So nothing prevents you from writing this code:

public override async Task ProcessRequestAsync(HttpContext c) { 
    await FireAndWait(c, async p => { 
        await Task.Delay(1000); 
        p.Response.StatusCode = (int)HttpStatusCode.Created; 
    }); 
}

private async Task FireAndWait(HttpContext c, Action<HttpContext> fireAndForget) {
    fireAndForget(c); 
}

The program starts with the ProcessRequestAsync method, which calls the FireAndWait method, which calls the asynchronous lambda method declared as an argument. However, as soon as it is called (or more precisely, after the waiting for Task.Delay begins), FireAndWait is completed, and immediately after that, ProcessRequestAsync and HttpContext have StatusCode at the initial value of 200 at that moment. The client has had the server’s HTTP response with it for a long time when the asynchronous lambda method wakes up and remembers that it wants to change StatusCode to 201.

Of course, the error can be easily corrected:

private async Task FireAndWait(HttpContext c, Func<HttpContext, Task> fireAndForget)

But is this correct? Where can I get the certainty that a similar error is not repeated anywhere else in the rest of the code? Of course, it would be easiest if the HttpTaskAsyncHandler class was not implemented so stupidly. It would be better if all data for the HTTP response was returned with the return command.

public override async Task<HttpResponse> ProcessRequestAsync(HttpRequest r)

But that would have to change a lot of code that didn't think about the asynchronous programming paradigm at the time of its creation. (Even though this one did.) When I used to create a new thread, I knew about it. It couldn't have been created just by accident. But now, when I add await somewhere, I cascade all the synchronous methods that call this code, just forget to change Action<T> to Func<T, Task> and a new thread is born.

You may also think of writing a code analyzer to point out this problem, but the problem is that it would have to have knowledge of those classes that serve as read/write data carriers, which are potentially all of them.

One of the variants is to program less functionally – not to branch the code by calling one or the other lambda method passed as an argument, but to honestly return the data and decide what will happen to it next from its nature.

Task<object> ReadData(HttpContext c) {
    if (c.Request.ContentType == "multipart/form-data") {
         return new List<object>();
    } else {
         return String.Empty;
    }
}

async Task ProcessRequestAsync(HttpContext c) {
    var retval = await ReadData(c);
    if (retval is List<object>) {
        …
    } else if (retval is string) {
        …
    }
}

But that’s a lot of extra code. A method can only return one pointer. So I have to create a special data type for everything, wrap all the data in it, then ask what actually came back and decide again what to do with the data. In the above example, I actually ask about the nature of the data twice. So it’s natural that a programmer tends to relieve both his keyboard and the processor:

Task ReadData(HttpContext c, Func<List<object>, Task> a, Func<string, Task> b) {
    if (c.Request.ContentType == "multipart/form-data") {
         await a(new List<object>());
    } else {
         await b(String.Empty);
    }
}

async Task ProcessRequestAsync(HttpContext c) {
    await ReadData(c, async a => {
        …
    }, async b => {
        …
    });
}

I'll leave it up to you which approach you prefer. But if you call a method and expect some data back, it’s not a good idea to do it by having the method change the object you pass to it in the argument. You don't have to wait for that change. And at that point, it’s good to at least think about whether you'll actually see them. Each async void method is a potential fork and the arguments you pass to it should be read-only. The joke is that you can't tell it apart from a regular synchronous method.