Skip to content

Commit d506f79

Browse files
halter73halter73
committed
Make ServerAddressFeature thread safe
1 parent 97577a4 commit d506f79

File tree

5 files changed

+208
-10
lines changed

5 files changed

+208
-10
lines changed

‎src/Servers/Kestrel/Core/src/Internal/AddressBindContext.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,14 @@
44
using System;
55
using System.Collections.Generic;
66
using System.Threading.Tasks;
7-
using Microsoft.AspNetCore.Hosting.Server.Features;
87
using Microsoft.Extensions.Logging;
98

109
namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
1110
{
1211
internal class AddressBindContext
1312
{
14-
public IServerAddressesFeature ServerAddressesFeature { get; set; }
15-
public ICollection<string> Addresses => ServerAddressesFeature.Addresses;
13+
public ServerAddressesFeature ServerAddressesFeature { get; set; }
14+
public ICollection<string> Addresses => ServerAddressesFeature.InternalCollection;
1615

1716
public KestrelServerOptions ServerOptions { get; set; }
1817
public ILogger Logger { get; set; }

‎src/Servers/Kestrel/Core/src/Internal/ServerAddressesCollection.cs

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Collections;
6+
using System.Collections.Generic;
7+
using System.Diagnostics;
8+
using Microsoft.AspNetCore.Hosting.Server.Features;
9+
10+
namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
11+
{
12+
internal class ServerAddressesCollection : ICollection<string>
13+
{
14+
private readonly List<string> _addresses = new List<string>();
15+
private readonly PublicServerAddressesCollection _publicCollection;
16+
17+
public ServerAddressesCollection()
18+
{
19+
_publicCollection = new PublicServerAddressesCollection(this);
20+
}
21+
22+
public ICollection<string> PublicCollection => _publicCollection;
23+
24+
public bool IsReadOnly => false;
25+
26+
public int Count
27+
{
28+
get
29+
{
30+
lock (_addresses)
31+
{
32+
return _addresses.Count;
33+
}
34+
}
35+
}
36+
37+
public void PreventPublicMutation()
38+
{
39+
lock (_addresses)
40+
{
41+
_publicCollection.IsReadOnly = true;
42+
}
43+
}
44+
45+
public void Add(string item)
46+
{
47+
lock (_addresses)
48+
{
49+
_addresses.Add(item);
50+
}
51+
}
52+
53+
public bool Remove(string item)
54+
{
55+
lock (_addresses)
56+
{
57+
return _addresses.Remove(item);
58+
}
59+
}
60+
61+
public void Clear()
62+
{
63+
lock (_addresses)
64+
{
65+
_addresses.Clear();
66+
}
67+
}
68+
69+
public void InternalAdd(string item)
70+
{
71+
lock (_addresses)
72+
{
73+
_addresses.Add(item);
74+
}
75+
}
76+
77+
public bool InternalRemove(string item)
78+
{
79+
lock (_addresses)
80+
{
81+
return _addresses.Remove(item);
82+
}
83+
}
84+
85+
public void InternalClear()
86+
{
87+
lock (_addresses)
88+
{
89+
_addresses.Clear();
90+
}
91+
}
92+
93+
public bool Contains(string item)
94+
{
95+
lock (_addresses)
96+
{
97+
return _addresses.Contains(item);
98+
}
99+
}
100+
101+
public void CopyTo(string[] array, int arrayIndex)
102+
{
103+
lock (_addresses)
104+
{
105+
_addresses.CopyTo(array, arrayIndex);
106+
}
107+
}
108+
109+
public IEnumerator<string> GetEnumerator()
110+
{
111+
lock (_addresses)
112+
{
113+
// Copy inside the lock.
114+
return new List<string>(_addresses).GetEnumerator();
115+
}
116+
}
117+
118+
IEnumerator IEnumerable.GetEnumerator()
119+
{
120+
return GetEnumerator();
121+
}
122+
123+
private class PublicServerAddressesCollection : ICollection<string>
124+
{
125+
private readonly ServerAddressesCollection _addressesCollection;
126+
private readonly object _addressesLock;
127+
128+
public PublicServerAddressesCollection(ServerAddressesCollection addresses)
129+
{
130+
_addressesCollection = addresses;
131+
_addressesLock = addresses._addresses;
132+
}
133+
134+
public bool IsReadOnly { get; set; }
135+
136+
public int Count => _addressesCollection.Count;
137+
138+
public void Add(string item)
139+
{
140+
lock (_addressesLock)
141+
{
142+
ThrowIfReadonly();
143+
_addressesCollection.Add(item);
144+
}
145+
}
146+
147+
public bool Remove(string item)
148+
{
149+
lock (_addressesLock)
150+
{
151+
ThrowIfReadonly();
152+
return _addressesCollection.Remove(item);
153+
}
154+
}
155+
156+
public void Clear()
157+
{
158+
lock (_addressesLock)
159+
{
160+
ThrowIfReadonly();
161+
_addressesCollection.Clear();
162+
}
163+
}
164+
165+
public bool Contains(string item)
166+
{
167+
return _addressesCollection.Contains(item);
168+
}
169+
170+
public void CopyTo(string[] array, int arrayIndex)
171+
{
172+
_addressesCollection.CopyTo(array, arrayIndex);
173+
}
174+
175+
public IEnumerator<string> GetEnumerator()
176+
{
177+
return _addressesCollection.GetEnumerator();
178+
}
179+
180+
IEnumerator IEnumerable.GetEnumerator()
181+
{
182+
return _addressesCollection.GetEnumerator();
183+
}
184+
185+
[StackTraceHidden]
186+
private void ThrowIfReadonly()
187+
{
188+
if (IsReadOnly)
189+
{
190+
throw new InvalidOperationException($"{nameof(IServerAddressesFeature)}.{nameof(IServerAddressesFeature.Addresses)} cannot be modified after the server has started.");
191+
}
192+
}
193+
}
194+
}
195+
}

‎src/Servers/Kestrel/Core/src/Internal/ServerAddressesFeature.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal
88
{
99
internal class ServerAddressesFeature : IServerAddressesFeature
1010
{
11-
public ICollection<string> Addresses { get; } = new List<string>();
11+
public ServerAddressesCollection InternalCollection { get; } = new ServerAddressesCollection();
12+
13+
ICollection<string> IServerAddressesFeature.Addresses => InternalCollection.PublicCollection;
1214
public bool PreferHostingUrls { get; set; }
1315
}
1416
}

‎src/Servers/Kestrel/Core/src/KestrelServer.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
2222
{
2323
public class KestrelServer : IServer
2424
{
25-
private readonly IServerAddressesFeature _serverAddresses;
25+
private readonly ServerAddressesFeature _serverAddresses;
2626
private readonly TransportManager _transportManager;
2727
private readonly IConnectionListenerFactory _transportFactory;
2828
private readonly IMultiplexedConnectionListenerFactory _multiplexedTransportFactory;
@@ -71,7 +71,7 @@ internal KestrelServer(IEnumerable<IConnectionListenerFactory> transportFactorie
7171

7272
Features = new FeatureCollection();
7373
_serverAddresses = new ServerAddressesFeature();
74-
Features.Set(_serverAddresses);
74+
Features.Set<IServerAddressesFeature>(_serverAddresses);
7575

7676
_transportManager = new TransportManager(_transportFactory, _multiplexedTransportFactory, ServiceContext);
7777

@@ -260,7 +260,9 @@ private async Task BindAsync(CancellationToken cancellationToken)
260260

261261
IChangeToken reloadToken = null;
262262

263-
if (Options.ConfigurationLoader?.ReloadOnChange == true && (!_serverAddresses.PreferHostingUrls || _serverAddresses.Addresses.Count == 0))
263+
_serverAddresses.InternalCollection.PreventPublicMutation();
264+
265+
if (Options.ConfigurationLoader?.ReloadOnChange == true && (!_serverAddresses.PreferHostingUrls || _serverAddresses.InternalCollection.Count == 0))
264266
{
265267
reloadToken = Options.ConfigurationLoader.Configuration.GetReloadToken();
266268
}
@@ -309,7 +311,7 @@ private async Task RebindAsync()
309311
foreach (var listenOption in endpointsToStop)
310312
{
311313
Options.OptionsInUse.Remove(listenOption);
312-
_serverAddresses.Addresses.Remove(listenOption.GetDisplayName());
314+
_serverAddresses.InternalCollection.Remove(listenOption.GetDisplayName());
313315
}
314316
}
315317

‎src/Servers/Kestrel/Core/test/AddressBinderTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ public void ParseAddressIP(string address, string ip, int port, bool isHttps)
116116
public async Task WrapsAddressInUseExceptionAsIOException()
117117
{
118118
var addresses = new ServerAddressesFeature();
119-
addresses.Addresses.Add("http://localhost:5000");
119+
addresses.InternalCollection.Add("http://localhost:5000");
120120
var options = new KestrelServerOptions();
121121

122122
var addressBindContext = new AddressBindContext
@@ -139,7 +139,7 @@ public async Task FallbackToIPv4WhenIPv6AnyBindFails(string address)
139139
{
140140
var logger = new MockLogger();
141141
var addresses = new ServerAddressesFeature();
142-
addresses.Addresses.Add(address);
142+
addresses.InternalCollection.Add(address);
143143
var options = new KestrelServerOptions();
144144

145145
var ipV6Attempt = false;

0 commit comments

Comments
 (0)