Aspnetcore: TestServer causes a deadlock

Created on 26 Apr 2020  路  17Comments  路  Source: dotnet/aspnetcore

Describe the bug

We have a lot of tests which are written in a way described here:
https://docs.microsoft.com/en-us/aspnet/core/test/integration-tests?view=aspnetcore-3.1#basic-tests-with-the-default-webapplicationfactory

Sometimes WebApplicationFactory + TestServer cause a deadlock while running under xUnit.

This particular line causes this:
https://github.com/dotnet/aspnetcore/blob/54722a52b6b609124f94d8c7f5801b8bc7cd447e/src/Hosting/TestHost/src/TestServer.cs#L66

Proposition
Can we have an explicit Start/Stop methods both on WebApplicationFactory and TestServer which could be called from the xunit lifetime interfaces of the fixture?

To Reproduce

You need to have a lot of integration tests starting lots(more than the cores you have) of TestServers in parallel.

Further technical details

  • ASP.NET Core 2.1
  • Rider, VS
Needs area-hosting

All 17 comments

We should fix this and potentially backport to 3.1

cc @Tratcher

@Tratcher hm, I did not quite get how it was solved. The blocking call is still in the constructor of the TestServer and the UseTestServer extension is just a container registration:

https://github.com/dotnet/aspnetcore/blob/f3f9a1cdbcd06b298035b523732b9f45b1408461/src/Hosting/TestHost/src/WebHostBuilderExtensions.cs#L16

Hence, it will still be invoked the same way it is invoked now.

When using IHostBuilder and the container registration, that affected IWebHostBuilder constructor is not used anymore.

Ah, I see. Ok, another question then.
Is it a short term solution or? Cause the documentation for the integration tests writing is still old and the constructor of the TestServer does still have blocking call. Are you going to remove this blocking call or it is there forever for backwards compatibility?

What are the plans for this?

@Tratcher we should fix the blocking code in the constructor and update the docs..

we should fix the blocking code in the constructor

How? And why bother when WebHostBuilder is scheduled for obsoletion in 5.0 (https://github.com/dotnet/aspnetcore/issues/20964)? People need to move to IHostBuilder regardless.

@maxcherednik can you give a specific example from the docs that's out of date? That doc primarily covers WebApplicationFactory and barely mentions TestServer. The doc for TestServer is still in progress. https://github.com/dotnet/AspNetCore.Docs/issues/16953

How?

Start initialization in the constructor and finish it when the first incoming request is made.

And why bother when WebHostBuilder is scheduled for obsoletion in 5.0 (#20964)? People need to move to IHostBuilder regardless.

That issue is reasonable until the related issues are fixed.

Most of our own test code still uses WebHostBuilder, so we haven't even fixed the issue for our own tests...

@Tratcher in the documentation I mentioned:

https://docs.microsoft.com/en-us/aspnet/core/test/integration-tests?view=aspnetcore-3.1#basic-tests-with-the-default-webapplicationfactory

WebApplicationFactory is used. Not much happening during the the construction of the factory. The actual thing is delayed till the first call to the CreateClient method, where through the chain of invocations it is calling this guy. Which calls the EnsureServer.

So anyway - the TestSever is blocking.

@davidfowl there is no need to actually start anything in the constructor. We can use the constructor for cheap wiring and the actual start could be a separate method. Btw, it can still be lazy, then CreateClient should be async as well.

I don't know what I am doing wrong, but here I am:
image

Just a reminder - I am running 2.1:
image

Right, we fixed this in 3.0. I don't think we can fix it in 2.1 without breaking a lot of people.

I would agree. Backporting to 2.1 is a very high bar and if it's even remotely breaking it's going to be a non-starter. Are there workarounds?

I was not specifically asking to fix this in 2.1, but thank you for the validation.

Btw, for the ones who are facing similar issue we came up with a workaround - reduce the number of concurrently blocking things to 1.

```c#

private static readonly SemaphoreSlim s_asyncLock = new SemaphoreSlim(1, 1);

private async Task StartInternalTestServer()
{
// Aspnet core test server built by wep application factory blocks async operation in the test server constructor
// which means it blocks a worker thread in xunit thread pool. Throttling test server creation to one at a time
// ensures at most one threads will be blocked. Otherwise we have to increase xunit config maxParallelThreads
// which then ends up in maximum CPU and high IOPs resulting in sql timeouts
await s_asyncLock.WaitAsync();

        try
        {
            // Before this web server is not started yet
            _appFactory.CreateClient();
        }
        finally
        {
            s_asyncLock.Release();
        }

}
```

At the end of all this, even in 3.1 WebApplicationFactory still blocks, no? What's the guidance for folks using WebApplicationFactory today with hosts that use a generic host and call GetWebHostDefaults? Use TestServer directly and copy some of the functionality over to a custom fixture class?

At the end of all this, even in 3.1 WebApplicationFactory still blocks, no?

Only if you're using the legacy WebHostBuilder rather than the new HostBuilder.

Closing as we don't plan to backport this to 2.1 and there are resolutions in 3.0+.

Was this page helpful?
0 / 5 - 0 ratings