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 reason why my code just did what it wanted. He behaved in a non-deterministic way. It served one endpoint on the site and occasionally returned HTTP status 200, even though I ordered in writing to return the status 201. In the end, I found the error and fixed it. I always think about the cause of the mistakes so that I don't repeat the same mistakes over and over again. But I came to the conclusion that it would be necessary to either redesign half of the .NET Framework classes or a new keyword in C#.

Object-oriented programming is based on the fact that objects change, and the order in which the changes are made matters. In contrast, in functional programming, structures are created from which, once created, one can only read. It has its undeniable advantage. It does not matter in the order in which the individual commands are executed. The command can be easily 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. A suitable program design can prevent this, but the compiler will not warn you that you are asking for random output data to be generated.

Let me 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 method ProcessRequestAsync, which calls method FireAndWait, which calls the asynchronous lambda method declared as an argument. But immediately after calling it (more precisely, after starting waiting for Task.Delay), FireAndWait is completed, immediately after ProcessRequestAsync and HttpContext has StatusCode at the default 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 nowhere else in the rest of the code is a similar error repeated? Of course, the easiest thing would be if the HttpTaskAsyncHandler class wasn't implemented so stupidly. It would be better if all data for the HTTP response was returned by command return.

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

But that would have to change a lot of code that didn't take into account the asynchronous programming paradigm at the time of its creation. (Even though this one is.) When I used to create a new thread, I knew about it. It couldn't be created just by mistake. But now, if I add await somewhere, cascading all the synchronous methods that call this code, I 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 warn about 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 by its nature what to do with it next.

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 more code. A method can return only one pointer. So I have to create a special data type for everything, wrap all the data into 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. It is therefore natural that the 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 => {
        …
    });
}

Which approach you prefer, I'll leave it up to you. However, if you call a method and expect some data back, it is not good to do it in such a way that the method changes the object that you pass to it in the argument. You may not 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 point is that you can't tell it apart from the regular synchronous method.