Skip to content

Commit 0140321

Browse files
JamesNKJamesNK
committed
Add pool test
1 parent a9249f4 commit 0140321

File tree

5 files changed

+99
-17
lines changed

5 files changed

+99
-17
lines changed

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

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,17 @@ public NamedPipeConnectionListener(
3535
NamedPipeEndPoint endpoint,
3636
NamedPipeTransportOptions options,
3737
ILoggerFactory loggerFactory,
38+
ObjectPoolProvider objectPoolProvider,
3839
Mutex mutex)
3940
{
4041
_log = loggerFactory.CreateLogger("Microsoft.AspNetCore.Server.Kestrel.Transport.NamedPipes");
4142
_endpoint = endpoint;
4243
_options = options;
43-
_namedPipeServerStreamPool = new DefaultObjectPoolProvider().Create(new NamedPipeServerStreamPoolPolicy(this));
4444
_mutex = mutex;
4545
_memoryPool = options.MemoryPoolFactory();
4646
_listeningToken = _listeningTokenSource.Token;
47+
// 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));
4749

4850
// The OS maintains a backlog of clients that are waiting to connect, so the app queue only stores a single connection.
4951
// We want to have a queue plus a background task that populates the queue, rather than creating NamedPipeServerStream
@@ -176,43 +178,45 @@ public async ValueTask DisposeAsync()
176178

177179
// Dispose pool after listening tasks are complete so there is no chance a stream is fetched from the pool after the pool is disposed.
178180
// Important to dispose because this empties and disposes streams in the pool.
179-
((IDisposable)_namedPipeServerStreamPool).Dispose();
181+
(_namedPipeServerStreamPool as IDisposable)?.Dispose();
180182
}
181183

182184
private sealed class NamedPipeServerStreamPoolPolicy : IPooledObjectPolicy<NamedPipeServerStream>
183185
{
184-
public NamedPipeConnectionListener _listener;
186+
private readonly NamedPipeEndPoint _endpoint;
187+
private readonly NamedPipeTransportOptions _options;
185188

186-
public NamedPipeServerStreamPoolPolicy(NamedPipeConnectionListener listener)
189+
public NamedPipeServerStreamPoolPolicy(NamedPipeEndPoint endpoint, NamedPipeTransportOptions options)
187190
{
188-
_listener = listener;
191+
_endpoint = endpoint;
192+
_options = options;
189193
}
190194

191195
public NamedPipeServerStream Create()
192196
{
193197
NamedPipeServerStream stream;
194198
var pipeOptions = NamedPipeOptions.Asynchronous | NamedPipeOptions.WriteThrough;
195-
if (_listener._options.CurrentUserOnly)
199+
if (_options.CurrentUserOnly)
196200
{
197201
pipeOptions |= NamedPipeOptions.CurrentUserOnly;
198202
}
199203

200-
if (_listener._options.PipeSecurity != null)
204+
if (_options.PipeSecurity != null)
201205
{
202206
stream = NamedPipeServerStreamAcl.Create(
203-
_listener._endpoint.PipeName,
207+
_endpoint.PipeName,
204208
PipeDirection.InOut,
205209
NamedPipeServerStream.MaxAllowedServerInstances,
206210
PipeTransmissionMode.Byte,
207211
pipeOptions,
208212
inBufferSize: 0, // Buffer in System.IO.Pipelines
209213
outBufferSize: 0, // Buffer in System.IO.Pipelines
210-
_listener._options.PipeSecurity);
214+
_options.PipeSecurity);
211215
}
212216
else
213217
{
214218
stream = new NamedPipeServerStream(
215-
_listener._endpoint.PipeName,
219+
_endpoint.PipeName,
216220
PipeDirection.InOut,
217221
NamedPipeServerStream.MaxAllowedServerInstances,
218222
PipeTransmissionMode.Byte,

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Net;
66
using Microsoft.AspNetCore.Connections;
77
using Microsoft.Extensions.Logging;
8+
using Microsoft.Extensions.ObjectPool;
89
using Microsoft.Extensions.Options;
910

1011
namespace Microsoft.AspNetCore.Server.Kestrel.Transport.NamedPipes.Internal;
@@ -14,17 +15,20 @@ internal sealed class NamedPipeTransportFactory : IConnectionListenerFactory, IC
1415
private const string LocalComputerServerName = ".";
1516

1617
private readonly ILoggerFactory _loggerFactory;
18+
private readonly ObjectPoolProvider _objectPoolProvider;
1719
private readonly NamedPipeTransportOptions _options;
1820

1921
public NamedPipeTransportFactory(
2022
ILoggerFactory loggerFactory,
21-
IOptions<NamedPipeTransportOptions> options)
23+
IOptions<NamedPipeTransportOptions> options,
24+
ObjectPoolProvider objectPoolProvider)
2225
{
2326
ArgumentNullException.ThrowIfNull(loggerFactory);
2427

2528
Debug.Assert(OperatingSystem.IsWindows(), "Named pipes transport requires a Windows operating system.");
2629

2730
_loggerFactory = loggerFactory;
31+
_objectPoolProvider = objectPoolProvider;
2832
_options = options.Value;
2933
}
3034

@@ -52,7 +56,7 @@ public ValueTask<IConnectionListener> BindAsync(EndPoint endpoint, CancellationT
5256
throw new AddressInUseException($"Named pipe '{namedPipeEndPoint.PipeName}' is already in use by Kestrel.");
5357
}
5458

55-
var listener = new NamedPipeConnectionListener(namedPipeEndPoint, _options, _loggerFactory, mutex);
59+
var listener = new NamedPipeConnectionListener(namedPipeEndPoint, _options, _loggerFactory, _objectPoolProvider, mutex);
5660
listener.Start();
5761

5862
return new ValueTask<IConnectionListener>(listener);

‎src/Servers/Kestrel/Transport.NamedPipes/src/WebHostBuilderNamedPipeExtensions.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
using Microsoft.AspNetCore.Server.Kestrel.Transport.NamedPipes;
77
using Microsoft.AspNetCore.Server.Kestrel.Transport.NamedPipes.Internal;
88
using Microsoft.Extensions.DependencyInjection;
9+
using Microsoft.Extensions.DependencyInjection.Extensions;
10+
using Microsoft.Extensions.ObjectPool;
911

1012
namespace Microsoft.AspNetCore.Hosting;
1113

@@ -29,6 +31,7 @@ public static IWebHostBuilder UseNamedPipes(this IWebHostBuilder hostBuilder)
2931

3032
hostBuilder.ConfigureServices(services =>
3133
{
34+
services.TryAddSingleton<ObjectPoolProvider, DefaultObjectPoolProvider>();
3235
services.AddSingleton<IConnectionListenerFactory, NamedPipeTransportFactory>();
3336
});
3437
return hostBuilder;

‎src/Servers/Kestrel/Transport.NamedPipes/test/NamedPipeConnectionListenerTests.cs

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Microsoft.AspNetCore.Connections;
66
using Microsoft.AspNetCore.Testing;
77
using Microsoft.Extensions.Logging;
8+
using Microsoft.Extensions.ObjectPool;
89

910
namespace Microsoft.AspNetCore.Server.Kestrel.Transport.NamedPipes.Tests;
1011

@@ -24,11 +25,70 @@ public async Task AcceptAsync_AfterUnbind_ReturnNull()
2425
Assert.Null(await connectionListener.AcceptAsync().DefaultTimeout());
2526
}
2627

28+
private class TestObjectPoolProvider : ObjectPoolProvider
29+
{
30+
public List<ITestObjectPool> Pools { get; } = new List<ITestObjectPool>();
31+
32+
public override ObjectPool<T> Create<T>(IPooledObjectPolicy<T> policy)
33+
{
34+
var pool = new TestObjectPool<T>(policy);
35+
Pools.Add(pool);
36+
37+
return pool;
38+
}
39+
40+
private class TestObjectPool<T> : ObjectPool<T>, ITestObjectPool where T : class
41+
{
42+
private readonly IPooledObjectPolicy<T> _policy;
43+
44+
public TestObjectPool(IPooledObjectPolicy<T> policy)
45+
{
46+
_policy = policy;
47+
}
48+
49+
public int GetCount { get; private set; }
50+
public int ReturnSuccessCount { get; private set; }
51+
public int ReturnFailureCount { get; private set; }
52+
53+
public override T Get()
54+
{
55+
GetCount++;
56+
return _policy.Create();
57+
}
58+
59+
public override void Return(T obj)
60+
{
61+
if (_policy.Return(obj))
62+
{
63+
ReturnSuccessCount++;
64+
}
65+
else
66+
{
67+
ReturnFailureCount++;
68+
}
69+
}
70+
}
71+
}
72+
73+
private interface ITestObjectPool
74+
{
75+
int GetCount { get; }
76+
int ReturnSuccessCount { get; }
77+
int ReturnFailureCount { get; }
78+
}
79+
2780
[ConditionalFact]
2881
public async Task AcceptAsync_ClientCreatesConnection_ServerAccepts()
2982
{
3083
// Arrange
31-
await using var connectionListener = await NamedPipeTestHelpers.CreateConnectionListenerFactory(LoggerFactory);
84+
var testObjectPoolProvider = new TestObjectPoolProvider();
85+
var options = new NamedPipeTransportOptions
86+
{
87+
ListenerQueueCount = 2
88+
};
89+
await using var connectionListener = await NamedPipeTestHelpers.CreateConnectionListenerFactory(LoggerFactory, options: options, objectPoolProvider: testObjectPoolProvider);
90+
var pool = Assert.Single(testObjectPoolProvider.Pools);
91+
Assert.Equal(options.ListenerQueueCount, pool.GetCount);
3292

3393
// Stream 1
3494
var acceptTask1 = connectionListener.AcceptAsync();
@@ -40,6 +100,10 @@ public async Task AcceptAsync_ClientCreatesConnection_ServerAccepts()
40100
await serverConnection1.DisposeAsync().AsTask().DefaultTimeout();
41101
Assert.True(serverConnection1.ConnectionClosed.IsCancellationRequested, "Connection 1 should be closed");
42102

103+
Assert.Equal(options.ListenerQueueCount + 1, pool.GetCount);
104+
Assert.Equal(1, pool.ReturnSuccessCount);
105+
Assert.Equal(0, pool.ReturnFailureCount);
106+
43107
// Stream 2
44108
var acceptTask2 = connectionListener.AcceptAsync();
45109
await using var clientStream2 = NamedPipeTestHelpers.CreateClientStream(connectionListener.EndPoint);
@@ -49,6 +113,10 @@ public async Task AcceptAsync_ClientCreatesConnection_ServerAccepts()
49113
Assert.False(serverConnection2.ConnectionClosed.IsCancellationRequested, "Connection 2 should be open");
50114
await serverConnection2.DisposeAsync().AsTask().DefaultTimeout();
51115
Assert.True(serverConnection2.ConnectionClosed.IsCancellationRequested, "Connection 2 should be closed");
116+
117+
Assert.Equal(options.ListenerQueueCount + 2, pool.GetCount);
118+
Assert.Equal(2, pool.ReturnSuccessCount);
119+
Assert.Equal(0, pool.ReturnFailureCount);
52120
}
53121

54122
[ConditionalFact]

‎src/Servers/Kestrel/Transport.NamedPipes/test/NamedPipeTestHelpers.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
using Microsoft.AspNetCore.Testing;
1111
using Microsoft.Extensions.Logging;
1212
using Microsoft.Extensions.Logging.Abstractions;
13+
using Microsoft.Extensions.ObjectPool;
1314
using Microsoft.Extensions.Options;
1415

1516
namespace Microsoft.AspNetCore.Server.Kestrel.Transport.NamedPipes.Tests;
@@ -22,18 +23,20 @@ internal static class NamedPipeTestHelpers
2223

2324
public static NamedPipeTransportFactory CreateTransportFactory(
2425
ILoggerFactory loggerFactory = null,
25-
NamedPipeTransportOptions options = null)
26+
NamedPipeTransportOptions options = null,
27+
ObjectPoolProvider objectPoolProvider = null)
2628
{
2729
options ??= new NamedPipeTransportOptions();
28-
return new NamedPipeTransportFactory(loggerFactory ?? NullLoggerFactory.Instance, Options.Create(options));
30+
return new NamedPipeTransportFactory(loggerFactory ?? NullLoggerFactory.Instance, Options.Create(options), objectPoolProvider ?? new DefaultObjectPoolProvider());
2931
}
3032

3133
public static async Task<NamedPipeConnectionListener> CreateConnectionListenerFactory(
3234
ILoggerFactory loggerFactory = null,
3335
string pipeName = null,
34-
NamedPipeTransportOptions options = null)
36+
NamedPipeTransportOptions options = null,
37+
ObjectPoolProvider objectPoolProvider = null)
3538
{
36-
var transportFactory = CreateTransportFactory(loggerFactory, options);
39+
var transportFactory = CreateTransportFactory(loggerFactory, options, objectPoolProvider);
3740

3841
var endpoint = new NamedPipeEndPoint(pipeName ?? GetUniquePipeName());
3942

0 commit comments

Comments
 (0)