TestServer causes a deadlock

[maxcherednik]

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:

aspnetcore/src/Hosting/TestHost/src/TestServer.cs

Line 66 in 54722a5

host.StartAsync().GetAwaiter().GetResult();

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

[davidfowl]

We should fix this and potentially backport to 3.1

cc @Tratcher

[Tratcher]

We have already addressed this in 3.0+ as part of the move to IHostBuilder.

aspnetcore/src/Hosting/TestHost/test/TestServerTests.cs

Lines 67 to 79 in 54722a5

	public async Task GenericCreateAndStartHost_GetTestClient()
	{
	using var host = await new HostBuilder()
	.ConfigureWebHost(webBuilder =>
	{
	webBuilder
	.UseTestServer()
	.Configure(app => { });
	})
	.StartAsync();
	var response = await host.GetTestClient().GetAsync("/");
	Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);

[maxcherednik]

@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:

aspnetcore/src/Hosting/TestHost/src/WebHostBuilderExtensions.cs

Line 16 in f3f9a1c

public static IWebHostBuilder UseTestServer(this IWebHostBuilder builder)

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

[Tratcher]

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

[maxcherednik]

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?

[davidfowl]

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

[Tratcher]

we should fix the blocking code in the constructor

How? And why bother when WebHostBuilder is scheduled for obsoletion in 5.0 (#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. dotnet/AspNetCore.Docs#16953

[davidfowl]

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...

[maxcherednik]

@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.

[Tratcher]

@maxcherednik that doesn't follow. The stack trace should look like this:

aspnetcore/src/Mvc/Mvc.Testing/src/WebApplicationFactory.cs

Line 412 in c7a2c06

EnsureServer();

aspnetcore/src/Mvc/Mvc.Testing/src/WebApplicationFactory.cs

Line 157 in c7a2c06

_server = (TestServer)_host.Services.GetRequiredService<IServer>();

aspnetcore/src/Hosting/TestHost/src/TestServer.cs

Line 26 in 9cecd08

public TestServer(IServiceProvider services)

DI shouldn't be able to call the TestServer.Ctor(IWebHostBuilder) constructor, IWebHostBuilder isn't in the DI container. You only end up in the blocking constructor if you start with a WebHostBuilder, not a HostBuilder.

aspnetcore/src/Mvc/Mvc.Testing/src/WebApplicationFactory.cs

Lines 161 to 164 in c7a2c06

	var builder = CreateWebHostBuilder();
	SetContentRoot(builder);
	_configuration(builder);
	_server = CreateServer(builder);

aspnetcore/src/Mvc/Mvc.Testing/src/WebApplicationFactory.cs

Line 362 in c7a2c06

protected virtual TestServer CreateServer(IWebHostBuilder builder) => new TestServer(builder);

[maxcherednik]

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

Just a reminder - I am running 2.1:

[Tratcher]

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

[analogrelay]

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?

[maxcherednik]

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.

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();
            }
}

[jbogard]

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?

[Tratcher]

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.

[analogrelay]

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