0

I am writing automated tests for a large application. Some of these tests could easily be async methods, as they make use of an HttpClient, which offers only async methods for issuing requests.

Some crucial aspects of the application to test are the following:

  • It is a huge ASP.NET application (though the code is not running within an ASP.NET context when executed through unit tests).
  • In its core, it relies heavily on static objects that cache context information (e.g. the entire permission scheme of the active user) per thread.

Now, my problem is that when using await in an async method, the continuation might happen in a different thread than before (as described in answers such as this or this). Obviously, the other thread doesn't have the aforementioned cached context information and I'm immediately drowning in exceptions as soon as I invoke anything.

Now, if I use GetAwaiter().GetResult() (as suggested here) instead of await on the HTTP calls and make my test methods synchronous, everything appears to work fine for my test cases.

Will this solution reliably ensure my entire test case will run on the same thread? Or do I have to pick one of the seemingly more complex solutions, such as implementing a custom synchronization context?

EDIT: To be clear: Changing the architecture of the application core itself, as questionable as its design may or may not be, is not an option.

To be clear no. 2: The only async methods I would like to call are the ones on HttpClient for sending HTTP requests. Our own code does not contain any async methods (possibly, for the reasons described above).

EDIT2: Here's a simplified version of my test method, to give an idea of what kinds of calls are in there:

[Test]
public async Task TestWriteAsync()
{
    MyAppLogic.Initialize(TestConstants.TestUserId);
    MyAppLogic.CleanUp();

    using (var client = new HttpClient())
    {
        var content = ... // my input data
        var response = await client.PostAsync("http://my.web.app.com/application/write", content);
        Assert.IsTrue(response.IsSuccessStatusCode);

        Assert.AreEqual(..., MyAppLogic.GetValue());

        MyAppLogic.CleanUp();
    }
}

Tentative rewrite that appears to work:

[Test]
public void TestWrite()
{
    MyAppLogic.Initialize(TestConstants.TestUserId);
    MyAppLogic.CleanUp();

    using (var client = new HttpClient())
    {
        var content = ... // my input data
        var response = client.PostAsync("http://my.web.app.com/application/write", content).GetAwaiter().GetResult();
        Assert.IsTrue(response.IsSuccessStatusCode);

        Assert.AreEqual(..., MyAppLogic.GetValue());

        MyAppLogic.CleanUp();
    }
}
O. R. Mapper
  • 20,083
  • 9
  • 69
  • 114
  • Why do you have the context information on Thread level? Context might be the droid you should be looking for. A lot of the job of `Task` is abstracting Multitasking, so it does not mater wich specific model of Multitasking you use. – Christopher Nov 15 '19 at 21:30
  • 2
    this ("sync over async") is explicitly a terrible idea; please don't, but to be clear: if you do this, **almost nothing** is guaranteed about what will happen (deadlock is a very real possibility); if you want to call an async API, you **must go async** - all the way; there is no reliable middle ground here – Marc Gravell Nov 15 '19 at 21:32
  • @Christopher: I have no idea, all of that was decided and implemented before I started on this project. We register a user somewhere on some core functions, which in turn load much information, such as the user's access rights on various application functions, data localized for the user's locale, and loads of other settings into various static classes, where that information is kept on dictionaries where the current thread ID doubles as the key. – O. R. Mapper Nov 15 '19 at 21:32
  • @MarcGravell: So you're saying I do need a custom synchronization context or task scheduler to force everything to happen on the same thread? Note that the only `async` methods I am using are indeed the various HTTP call methods from the `HttpClient` class. None of our own code is `async`. – O. R. Mapper Nov 15 '19 at 21:34
  • Can't you just change teh Dictionary ID to be the user ID? Why on earth would any programmer of sound mind use the **Thread** ID instead of something like the UserID for UserData? – Christopher Nov 15 '19 at 21:41
  • @Christopher: No, changing anything about how those core classes work is not an option. We're talking about an application that has been under development for several years, and that is used in live systems with customers. I am not going to touch that code. As for the why - I *think* the assumption that all that cached context information is required for any given request (to the ASP.NET application, mind you), and that at any given time, one thread will handle one request, and vice-versa (parallel worker threads that do not need the context info notwithstanding) played a role, while the ... – O. R. Mapper Nov 15 '19 at 21:46
  • ... same user account could theoretically be running several requests with different priviledges (think several browser window, or automated tasks that run with limited priviledges) at the same time. – O. R. Mapper Nov 15 '19 at 21:47
  • 2
    @O.R.Mapper "same user account could theoretically be running several requests with different priviledges " then each request has a session ID and you use that instead of the UserID. Using teh ThreadID for the SessionID? That was the mistake. – Christopher Nov 15 '19 at 21:49
  • @Christopher: I appreciate your attempts to think out of the box, but we are talking about literally millions of business-critical lines of code here, which I am neither allowed nor willing to change at this point. Whatever issue I have in my automated test code, I need to fix in that automated test code, not in the actual application itself. I am not claiming the application itself is particularly well-designed, but that's what it is. I just want to send some HTTP requests from within a test class, without revamping the entire architecture :) – O. R. Mapper Nov 15 '19 at 21:52
  • @O.R.Mapper: The mistake seem definitely in the soruce code, not the test. *Asuming* a specific Thread will always work on one task is just a design mistake. One somewhere around catching boneheaded exceptions bad: https://blogs.msdn.microsoft.com/ericlippert/2008/09/10/vexing-exceptions/ | If nobody here can give you a solution, you can asume there is no solution - other then fixing that code. – Christopher Nov 15 '19 at 21:54
  • @Christopher: Sorry, I'm not giving up that early ;) Also, I have meanwhile found the [`HttpWebRequest` class](https://learn.microsoft.com/en-us/dotnet/api/system.net.httpwebrequest?redirectedfrom=MSDN&view=netframework-4.8), which has a synchronous `GetResponse` method, as well as an [MSDN blogpost](https://blogs.msdn.microsoft.com/jpsanders/2017/08/28/asp-net-do-not-use-task-result-in-main-context/) that describes an apparently deadlock-free way to synchronously call `HttpClient` - two promising leads, I would say. – O. R. Mapper Nov 15 '19 at 22:03
  • Rather than writing your own SynchronizationContext, why not use the one that's used in ASP.NET? That way you not only save yourself work, but you more correctly test the behavior (including performance) that you can expect while executing in production. – StriplingWarrior Nov 15 '19 at 22:45
  • @StriplingWarrior: Hmm, at least [that answer](https://stackoverflow.com/a/12661242/1430156) suggests that with the ASP.NET synchronization context, "continuations may execute on a different thread". Thus, it doesn't sound suitable for my case where I need to be sure the continuation happens on the same thread, unfortunately. – O. R. Mapper Nov 15 '19 at 22:49
  • @O.R.Mapper: There are different synchronization contexts used by different versions and configurations of ASP.NET. If you think of what you just said, if the one being used by your app didn't ensure things were executing on the same thread, that would imply that the whole architecture you've described for your application is not functioning, which is what you'd want your integration tests to find anyway. But since it sounds like it's been working, I'm guessing you just need to find which implementation your app is using in production. – StriplingWarrior Nov 15 '19 at 22:56
  • @StriplingWarrior: I think what you're missing is that my tests do something that the app never does - they use `await` and then access the regular application code, which might be continued in a different thread precisely due to the `await`. This simply doesn't happen in the regular app; the `await` keyword doesn't appear at all in all the thousands of source files, and any thread-related code that I could find was very explicit about running isolated procedures in worker threads apart from the main thread that handles the current request. – O. R. Mapper Nov 15 '19 at 23:23
  • 1
    "So you're saying I do need a custom synchronization context or task scheduler to force everything to happen on the same thread?" - I explicitly did not say that; if you are thinking "async", or using "async" APIs, frankly you need to stop thinking in terms of threads, and refactor any code that insists on specific threads; or: don't use "async" APIs – Marc Gravell Nov 16 '19 at 08:55
  • @MarcGravell: Ok, so, as rewriting the entire application is not an option for me, `HttpWebRequest` with its non-async `GetResponse` method seems to be the way to go for me. – O. R. Mapper Nov 16 '19 at 09:58
  • What are you expecting to see as an answer here? Judging by the comments, it appears that you’re hoping for a magic solution that works despite incompetent architecture of the functions under test. That seems unlikely to me. – theMayer Nov 16 '19 at 13:07
  • @theMayer: Nothing magical, just an explanation why the async methods on an HttpClient in particular cannot be synchronized/waited for, an explanation that they can actually be waited for (as has meanwhile arrived in noseratio's answer), or an alternative to those async methods (as I have meanwhile found with HttpWebRequest). Sorry for being a bit insistent here, I just can't believe something relatively simple like "issue an HTTP request - block thread until it has completed - read HTTP response in same thread" cannot be done. – O. R. Mapper Nov 16 '19 at 14:06
  • 1
    As an http request is going to be traversing the sockets and thus will not be sharing memory with your application, there are one of two possibilities here going on. (1) your application global context is not truly thread-safe, and it's pure luck that your users aren't encountering the same issues as your tests or (2) certain functions with certain arguments cannot be run in parallel with other functions, but this restriction has not been coded into the tests. – theMayer Nov 16 '19 at 14:09
  • Keep in mind that we have no idea (1) what code you're trying to test, (2) how you're testing it, or (3) what exceptions you're getting. All you've provided is some vague idea that "it works when I do this, and it doesn't work when I do that." So naturally, the answer is "then don't do what doesn't work." – theMayer Nov 16 '19 at 14:12
  • @theMayer: I'm not sure what you're searching for; the cause of the observed issues is not unknown at all (in fact, I have described it in the question): The global context is truly thread-safe (in that there are no race conditions or deadlocks), and all functions can be run in parallel with each other, BUT (here comes the underlying restriction) everything is designed under the assumption that, basically, the thread that a method is invoked in will be the thread that all lines of that method will be executed in. What exceptions I'm getting if the thread *does* change mid-way: ... – O. R. Mapper Nov 16 '19 at 20:51
  • ... `NullReferenceException`, `KeyNotFoundException` and the like, because the static settings containers that are expected to be loaded are simply not there. Not surprisingly, given that they are stored and retrieved by using the thread ID as a key. Hence, if the beginning of a method stores e.g. the loaded user permissions under the current thread ID and the end of a method needs to check a particular user permission by accessing those loaded permissions again, and the current thread's ID has changed in the meantime, accessing the loaded permissions will fail, of course. – O. R. Mapper Nov 16 '19 at 20:57
  • @O.R.Mapper: You're correct that I didn't understand the production code is entirely synchronous and only the test code uses `async`/`await`. – StriplingWarrior Nov 18 '19 at 03:52

1 Answers1

2

In its core, it relies heavily on static objects that cache context information (e.g. the entire permission scheme of the active user) per thread.

Perhaps, you can make those static object to be AsyncLocal<T> (versus ThreadLocal<T> or [ThreadStatic]). Then they will flow across different threads.

If though you're only allowed to touch the tests and not the code under test, then you'd have to stick with synchronous methods, as Marc mentioned in the comments. You should still be able to use HttpClient.PostAsync(..).GetAwaiter().GetResult(), instead of HttpWebRequest. If you're concerned that something inside HttpClient may end up in a deadlock, you can wrap the call with Task.Run:

Task.Run(() => client.PostAsync(..)).GetAwaiter().GetResult()

Note however, there's no thread affinity in ASP.NET environment, i.e. your ASP.NET controller methods would most likely be called each time on a different thread. So, those static per-thread objects might only be valid for a scope of a specific HTTP request/response.

Tanveer Badar
  • 5,438
  • 2
  • 27
  • 32
noseratio
  • 59,932
  • 34
  • 208
  • 486
  • 1
    Thanks a lot, I'm going to apply your answer for my tests. A few comments, for the sake of future reader and/or just in case you're curious: Unfortunately, the thread-specific data is not even marked as `[ThreadStatic]`, but inserted and retrieved within various methods by means of [`Thread.GetData`](https://learn.microsoft.com/en-us/dotnet/api/system.threading.thread.getdata?view=netframework-4.8#System_Threading_Thread_GetData_System_LocalDataStoreSlot_) and the like. Thus, replacing this (and not breaking anything on the way!) is not too easy. ... – O. R. Mapper Nov 18 '19 at 12:55
  • ... Concerning the proposed solution wrapped in `Task.Run`, am I correctly getting the idea that like that, the `async` call will be run on another thread and work as a regular `async` call there, and it's only the main thread that invoked `Task.Run` that will be blocked and wait for the results? Finally, concerning your remark on ASP.NET - yes, I think that's the principle the entire application is built upon; the context information for an HTTP request is stored on the thread and accessible there throughout the processing of the request, but no two requests need to know about one another. – O. R. Mapper Nov 18 '19 at 13:00
  • @O.R.Mapper, *and it's only the main thread that invoked Task.Run that will be blocked and wait for the results?* - yes, this is correct. Only the original thread will be blocked, and the pool thread that is acquired by `Task.Run` will be briefly released once the async call is in-flight. – noseratio Nov 18 '19 at 20:36
  • 1
    .. *inserted and retrieved within various methods by means of Thread.GetData and the like* - I see, but maybe you should rise the issue about code refactoring with your team to embrace `AsyncLocal` or some other alternatives, this shouldn't be terribly complicated. – noseratio Nov 18 '19 at 20:46