Skip to content

Commit e4c6621

Browse files
authored
Use FirstPipeInstance setting in named pipes transport (#48609)
1 parent c2488ee commit e4c6621

File tree

2 files changed

+33
-17
lines changed

2 files changed

+33
-17
lines changed

src/Servers/Kestrel/Transport.NamedPipes/src/Internal/NamedPipeConnectionListener.cs

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,25 +27,24 @@ internal sealed class NamedPipeConnectionListener : IConnectionListener
2727
private readonly MemoryPool<byte> _memoryPool;
2828
private readonly PipeOptions _inputOptions;
2929
private readonly PipeOptions _outputOptions;
30-
private readonly Mutex _mutex;
30+
private readonly NamedPipeServerStreamPoolPolicy _poolPolicy;
3131
private Task? _completeListeningTask;
3232
private int _disposed;
3333

3434
public NamedPipeConnectionListener(
3535
NamedPipeEndPoint endpoint,
3636
NamedPipeTransportOptions options,
3737
ILoggerFactory loggerFactory,
38-
ObjectPoolProvider objectPoolProvider,
39-
Mutex mutex)
38+
ObjectPoolProvider objectPoolProvider)
4039
{
4140
_log = loggerFactory.CreateLogger("Microsoft.AspNetCore.Server.Kestrel.Transport.NamedPipes");
4241
_endpoint = endpoint;
4342
_options = options;
44-
_mutex = mutex;
4543
_memoryPool = options.MemoryPoolFactory();
4644
_listeningToken = _listeningTokenSource.Token;
4745
// Have to create the pool here (instead of DI) because the pool is specific to an endpoint.
48-
_namedPipeServerStreamPool = objectPoolProvider.Create(new NamedPipeServerStreamPoolPolicy(endpoint, options));
46+
_poolPolicy = new NamedPipeServerStreamPoolPolicy(endpoint, options);
47+
_namedPipeServerStreamPool = objectPoolProvider.Create(_poolPolicy);
4948

5049
// The OS maintains a backlog of clients that are waiting to connect, so the app queue only stores a single connection.
5150
// We want to have a queue plus a background task that populates the queue, rather than creating NamedPipeServerStream
@@ -77,6 +76,7 @@ public void Start()
7776
{
7877
// Start first stream inline to catch creation errors.
7978
var initialStream = _namedPipeServerStreamPool.Get();
79+
_poolPolicy.SetFirstPipeStarted();
8080

8181
listeningTasks[i] = Task.Run(() => StartAsync(initialStream));
8282
}
@@ -170,7 +170,6 @@ public async ValueTask DisposeAsync()
170170
}
171171

172172
_listeningTokenSource.Dispose();
173-
_mutex.Dispose();
174173
if (_completeListeningTask != null)
175174
{
176175
await _completeListeningTask;
@@ -185,6 +184,7 @@ private sealed class NamedPipeServerStreamPoolPolicy : IPooledObjectPolicy<Named
185184
{
186185
private readonly NamedPipeEndPoint _endpoint;
187186
private readonly NamedPipeTransportOptions _options;
187+
private bool _hasFirstPipeStarted;
188188

189189
public NamedPipeServerStreamPoolPolicy(NamedPipeEndPoint endpoint, NamedPipeTransportOptions options)
190190
{
@@ -196,6 +196,14 @@ public NamedPipeServerStream Create()
196196
{
197197
NamedPipeServerStream stream;
198198
var pipeOptions = NamedPipeOptions.Asynchronous | NamedPipeOptions.WriteThrough;
199+
if (!_hasFirstPipeStarted)
200+
{
201+
// The first server stream created should validate that no one else is listening with a given name.
202+
// Only the first server stream should make this test. The listener will almost always create multiple streams
203+
// to listen on multiple threads and to handle parallel requests. The pool policy must be updated that the
204+
// setting isn't needed after the first stream.
205+
pipeOptions |= NamedPipeOptions.FirstPipeInstance;
206+
}
199207
if (_options.CurrentUserOnly)
200208
{
201209
pipeOptions |= NamedPipeOptions.CurrentUserOnly;
@@ -228,5 +236,10 @@ public NamedPipeServerStream Create()
228236
}
229237

230238
public bool Return(NamedPipeServerStream obj) => !obj.IsConnected;
239+
240+
public void SetFirstPipeStarted()
241+
{
242+
_hasFirstPipeStarted = true;
243+
}
231244
}
232245
}

src/Servers/Kestrel/Transport.NamedPipes/src/Internal/NamedPipeTransportFactory.cs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -45,19 +45,22 @@ public ValueTask<IConnectionListener> BindAsync(EndPoint endpoint, CancellationT
4545
throw new NotSupportedException($@"Server name '{namedPipeEndPoint.ServerName}' is invalid. The server name must be ""{LocalComputerServerName}"".");
4646
}
4747

48-
// Creating a named pipe server with an name isn't exclusive. Create a mutex with the pipe name to prevent multiple endpoints
49-
// accidently sharing the same pipe name. Will detect across Kestrel processes.
50-
// Note that this doesn't prevent other applications from using the pipe name.
51-
var mutexName = "Kestrel-NamedPipe-" + namedPipeEndPoint.PipeName;
52-
var mutex = new Mutex(false, mutexName, out var createdNew);
53-
if (!createdNew)
48+
var listener = new NamedPipeConnectionListener(namedPipeEndPoint, _options, _loggerFactory, _objectPoolProvider);
49+
50+
// Start the listener and create NamedPipeServerStream instances immediately.
51+
// The first server stream is created with the FirstPipeInstance flag. The FirstPipeInstance flag ensures
52+
// that no other apps have a running pipe with the configured name. This check is important because:
53+
// 1. Some settings, such as ACL, are chosen by the first pipe to start with a given name.
54+
// 2. It's easy to run two Kestrel app instances. Want to avoid two apps listening on the same pipe and creating confusion.
55+
// The second launched app instance should immediately fail with an error message, just like port binding conflicts.
56+
try
5457
{
55-
mutex.Dispose();
56-
throw new AddressInUseException($"Named pipe '{namedPipeEndPoint.PipeName}' is already in use by Kestrel.");
58+
listener.Start();
59+
}
60+
catch (UnauthorizedAccessException ex)
61+
{
62+
throw new AddressInUseException($"Named pipe '{namedPipeEndPoint.PipeName}' is already in use.", ex);
5763
}
58-
59-
var listener = new NamedPipeConnectionListener(namedPipeEndPoint, _options, _loggerFactory, _objectPoolProvider, mutex);
60-
listener.Start();
6164

6265
return new ValueTask<IConnectionListener>(listener);
6366
}

0 commit comments

Comments
 (0)