Skip to content

Commit 0c3cbd2

Browse files
committed
Address frequent PortReserver failures
- related to aspnet#11 (3 of 4 additions) - reduce work done within inner loop - add messages for `Exception`s this class `throw`s - release acquired `Mutex`es - reduce timeout for acquiring global `Mutex`
1 parent 2e98bc0 commit 0c3cbd2

File tree

1 file changed

+38
-15
lines changed

1 file changed

+38
-15
lines changed

test/Microsoft.TestCommon/PortReserver.cs

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
using System.Collections.Generic;
66
using System.Diagnostics;
77
using System.Globalization;
8-
using System.Linq;
98
using System.Net;
109
using System.Net.NetworkInformation;
1110
using System.Threading;
@@ -14,18 +13,19 @@ namespace Microsoft.TestCommon
1413
{
1514
/// <summary>
1615
/// This class allocates ports while ensuring that:
17-
/// 1. Ports that are permanentaly taken (or taken for the duration of the test) are not being attempted to be used.
16+
/// 1. Ports that are permanently taken (or taken for the duration of the test) are not being attempted to be used.
1817
/// 2. Ports are not shared across different tests (but you can allocate two different ports in the same test).
19-
///
18+
///
2019
/// Gotcha: If another application grabs a port during the test, we have a race condition.
2120
/// </summary>
2221
[DebuggerDisplay("Port: {PortNumber}, Port count for this app domain: {_appDomainOwnedPorts.Count}")]
2322
public class PortReserver : IDisposable
2423
{
2524
private Mutex _portMutex;
25+
private Thread _acquiredOn;
2626

2727
// We use this list to hold on to all the ports used because the Mutex will be blown through on the same thread.
28-
// Theoretically we can do a thread local hashset, but that makes dispose thread dependant, or requires more complicated concurrency checks.
28+
// Theoretically we can do a thread local HashSet, but that makes dispose thread-dependent, or requires more complicated concurrency checks.
2929
// Since practically there is no perf issue or concern here, this keeps the code the simplest possible.
3030
private static HashSet<int> _appDomainOwnedPorts = new HashSet<int>();
3131

@@ -39,16 +39,21 @@ public PortReserver(int basePort = 50231)
3939
{
4040
if (basePort <= 0)
4141
{
42-
throw new InvalidOperationException();
42+
throw new ArgumentOutOfRangeException("basePort", "Argument must be greater than 0.");
4343
}
4444

4545
// Grab a cross appdomain/cross process/cross thread lock, to ensure only one port is reserved at a time.
4646
using (Mutex mutex = GetGlobalMutex())
4747
{
4848
try
4949
{
50-
int port = basePort - 1;
50+
var usedTCPPorts = new HashSet<int>();
51+
foreach (var endPoint in ListUsedTCPPort())
52+
{
53+
usedTCPPorts.Add(endPoint.Port);
54+
}
5155

56+
int port = basePort - 1;
5257
while (true)
5358
{
5459
port++;
@@ -60,18 +65,19 @@ public PortReserver(int basePort = 50231)
6065

6166
// AppDomainOwnedPorts check enables reserving two ports from the same thread in sequence.
6267
// ListUsedTCPPort prevents port contention with other apps.
63-
if (_appDomainOwnedPorts.Contains(port) ||
64-
ListUsedTCPPort().Any(endPoint => endPoint.Port == port))
68+
if (_appDomainOwnedPorts.Contains(port) || usedTCPPorts.Contains(port))
6569
{
6670
continue;
6771
}
6872

69-
string mutexName = "WebStack-Port-" + port.ToString(CultureInfo.InvariantCulture); // Create a well known mutex
73+
// Create a well known mutex
74+
string mutexName = "WebStack-Port-" + port.ToString(CultureInfo.InvariantCulture);
7075
_portMutex = new Mutex(initiallyOwned: false, name: mutexName);
7176

7277
// If no one else is using this port grab it.
7378
if (_portMutex.WaitOne(millisecondsTimeout: 0))
7479
{
80+
_acquiredOn = Thread.CurrentThread;
7581
break;
7682
}
7783

@@ -108,26 +114,43 @@ public void Dispose()
108114

109115
using (Mutex mutex = GetGlobalMutex())
110116
{
111-
_portMutex.Dispose();
112-
_appDomainOwnedPorts.Remove(PortNumber);
113-
PortNumber = -1;
117+
try
118+
{
119+
using (_portMutex)
120+
{
121+
if (_acquiredOn == Thread.CurrentThread)
122+
{
123+
_portMutex.ReleaseMutex();
124+
}
125+
126+
_portMutex = null;
127+
}
128+
129+
_appDomainOwnedPorts.Remove(PortNumber);
130+
PortNumber = -1;
131+
}
132+
finally
133+
{
134+
mutex.ReleaseMutex();
135+
}
114136
}
115137
}
116138

117139
private static Mutex GetGlobalMutex()
118140
{
141+
const int timeoutInSeconds = 20;
142+
119143
Mutex mutex = new Mutex(initiallyOwned: false, name: "WebStack-RandomPortAcquisition");
120-
if (!mutex.WaitOne(30000))
144+
if (!mutex.WaitOne(TimeSpan.FromSeconds(timeoutInSeconds)))
121145
{
122-
throw new InvalidOperationException();
146+
throw new InvalidOperationException($"Unable to reserve global Mutex within {timeoutInSeconds} seconds.");
123147
}
124148

125149
return mutex;
126150
}
127151

128152
private static IPEndPoint[] ListUsedTCPPort()
129153
{
130-
var usedPort = new HashSet<int>();
131154
IPGlobalProperties ipGlobalProperties = IPGlobalProperties.GetIPGlobalProperties();
132155

133156
return ipGlobalProperties.GetActiveTcpListeners();

0 commit comments

Comments
 (0)