Skip to content
This repository was archived by the owner on Dec 19, 2018. It is now read-only.

SelfHostDeployer still produces AddressInUseException #1296

Closed
ryanbrandenburg opened this issue Dec 12, 2017 · 16 comments
Closed

SelfHostDeployer still produces AddressInUseException #1296

ryanbrandenburg opened this issue Dec 12, 2017 · 16 comments
Assignees
Milestone

Comments

@ryanbrandenburg
Copy link
Contributor

We run into this kind of thing still. I believe previously @anurse added extra logging to this scenario.

@muratg
Copy link

muratg commented Jan 31, 2018

@anurse what's the work proposed here?

@muratg muratg added the task label Jan 31, 2018
@analogrelay
Copy link
Contributor

Depends on if this is still a persistent problem on the CI. In order to figure out why it's still happening, we'd either need more logging or to make our tests way more defensive about addresses being in use.

@ryanbrandenburg
Copy link
Contributor Author

It is still happening on the CI. When you say more logging you mean in the SelfHostDeployer right?

@analogrelay
Copy link
Contributor

Yes

@muratg muratg added this to the 2.1.0 milestone Feb 1, 2018
@muratg
Copy link

muratg commented Feb 1, 2018

@JunTaoLuo Whenever you have some free time, can you take a look?

@muratg muratg modified the milestones: 2.1.0, 2.1.0-preview2 Feb 15, 2018
@ryanbrandenburg
Copy link
Contributor Author

Here's a non-exhaustive (but reasonably through) list of the builds that were failed by this. As you can see it happens about once or more per-day.

@ryanbrandenburg
Copy link
Contributor Author

Any movement on this? It fails 2+ times a day.

@JunTaoLuo
Copy link
Contributor

I'll make some time to take a look at it this week.

@mikeharder
Copy link
Contributor

mikeharder commented Mar 15, 2018

SelfHostDeployer currently works as follows:

  1. Bind a socket to port 0
  2. Query the socket to determine the assigned port (say 56789)
  3. Dispose the socket
  4. Start another process, and tell it to use the earlier assigned port 56789
  5. Other process binds to assigned port 56789

This design was based on the assumption that the OS will not reuse the assigned port between step 3 and step 5. This assumption is usually true, but sometimes the OS does reuse the port, which causes one of the external processes to fail to bind. See repro at https://github.com/mikeharder/GetNextPort, which fails after a few seconds on my machine.

The most reliable workaround is to always start the external process on port 0, then have the external process communicate back the assigned port. SelfHostDeployer already has logic for this:

But it also has logic to handle apps which don't print the assigned port:

return (url: actualUrl ?? hintUrl, hostExitToken: hostExitTokenSource.Token);

It appears the only apps which don't print the assigned port are tests specifically for this logic:

https://github.com/search?utf8=%E2%9C%93&q=org%3Aaspnet+statusmessagesenabled&type=Code

So, I think the most reliable fix:

  • Remove GetNextPort() and StatusMessagesEnabled from SelfHostDeployer
  • Always start external processes on port 0, and require them to print the assigned port as a status message

SignalR also uses GetNextPort():

https://github.com/aspnet/SignalR/blob/ff43390ed2ddd8ad9274fd2c99ae330fe31e743c/test/Common/ServerFixture.cs#L35

But I believe it can be changed to use port 0.

Kestrel also uses GetNextPort() in AddressRegistrationTests:

https://github.com/aspnet/KestrelHttpServer/blob/4afaa386db527e1ff168389bb189ddbecd8deaff/test/Kestrel.FunctionalTests/AddressRegistrationTests.cs#L998

This usage needs to stay, since it's specifically testing that Kestrel can bind to a non-zero port. There's still a race condition if AddressRegistrationTests is running in parallel with another process binding to port 0, but even if this happens, the frequency of this failure will be significantly reduced (likely to 0).

Will send a PR tomorrow.

@Tratcher
Copy link
Member

Keep in mind that HttpSys also uses the SelfHost deployer in ServerTests and MusicStore and doesn't support binding to port 0. I don't see it calling GetNextPort directly, but I don't recall how it's test port is allocated otherwise.

@ryanbrandenburg
Copy link
Contributor Author

This particular presentation of this error is new to me.

@mikeharder
Copy link
Contributor

@ryanbrandenburg: What is different about http://aspnetci/viewLog.html?buildId=432346&buildTypeId=XPlat_Windows_Win8_Universe? It looks like the typical error message:

| [2018-03-27T05:08:07] Microsoft.AspNetCore.Server.IntegrationTesting.SelfHostDeployer Warning:
dotnet stderr: Unhandled Exception: System.IO.IOException:
Failed to bind to address http://127.0.0.1:57896: address already in use. --->
Microsoft.AspNetCore.Connections.AddressInUseException: Only one usage of each socket address (protocol/network address/port) is normally permitted ---> 
System.Net.Sockets.SocketException: Only one usage of each socket address (protocol/network address/port) is normally permitted

@ryanbrandenburg
Copy link
Contributor Author

What was different to me was that it was throwing CSC 255 at the top of the exception and was showing a stack-trace out of CompilerServices.

@muratg muratg modified the milestones: 2.1.0-preview2, 2.1.0-rc1 Apr 3, 2018
@muratg
Copy link

muratg commented Apr 3, 2018

Updating the milestone as we're closing on preview2. If this is ready to checkin to release/2.1 branch, though, I think we should still be able to.

@muratg
Copy link

muratg commented Apr 6, 2018

@mikeharder Do you plan to do any work on this in RC1?

@mikeharder
Copy link
Contributor

@muratg: Yes, I will work on this tomorrow and Friday.

mikeharder added a commit that referenced this issue Apr 17, 2018
* Should significantly reduce flaky test failures due to AddressInUse exceptions
* Addresses #1296
pranavkm pushed a commit that referenced this issue Sep 19, 2018
* Should significantly reduce flaky test failures due to AddressInUse exceptions
* Addresses #1296
pranavkm pushed a commit that referenced this issue Sep 19, 2018
* Should significantly reduce flaky test failures due to AddressInUse exceptions
* Addresses #1296
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants