Add IMemoryPoolFactory to Connections.Abstractions

[BrennanConroy]

Background and Motivation

Kestrel (and other server impls) use the PinnedBlockMemoryPool for the core IO processing. The usage is purely internal today and also has the issue of never releasing its memory resulting in users thinking there is a memory leak. See: #55490

As part of wanting to reduce the memory when the pool is idle we want to use a timer, but creating a timer per pool is not ideal, so we want to put the cleanup on the kestrel heartbeat thread which requires sharing between Sockets, NamedPipes, and Kestrel Core. Ideally we want to avoid internals visible to, so in order to solve this we need a type that can be used by all dlls and shared via DI so we can wire up the pool usage everywhere.

Proposed API

Microsoft.AspNetCore.Connections.Abstractions.dll

namespace Microsoft.AspNetCore.Connections;

+ public interface IMemoryPoolFactory
+ {
+    MemoryPool<byte> CreatePool();
+ }

Usage Examples

var builder = WebApplication.CreateBuilder(args);

builder.Services.AddSingleton<IMemoryPoolFactory, MyMemoryPoolFactory>();

var app = builder.Build();

public sealed class MyMemoryPoolFactory : IMemoryPoolFactory
{
    public MemoryPool<byte> CreatePool()
    {
        return MemoryPool<byte>.Shared;
    }
}

Alternative Designs

Alternatively we can use [InternalsVisibleTo] on some of our assemblies.

Risks

N/A

[dotnet-policy-service]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[halter73]

API Review Notes:

• Does IMemoryPoolFactory exist today internally? Sort of as SocketTransportOptions.MemoryPoolFactory
  • Could it stay an option?
    • We want to access it outside of the transport in places like Kestrel's heartbeat and using the IMemoryPoolFeature is unwieldy for that.
• Should it be a property?
  • No because it's very likely to be expensive and return a new instance each time
• What about the name CreatePool?
  • GetPool?
  • Create?
  • GetOrCreate?
  • CreateMemoryPool?
  • Let's go with Create!
• Do we need to be able to return the pool?
  • No. The caller should dispose it.
• Would this be useful outside of Microsoft.AspNetCore.Connections?
  • We doubt they'd want this in System.Memory
  • We can't think of a better place.
  • This makes it clear that the consumer is Kestrel or at least ASP.NET Core.
• This is similar to ObjectPoolProvider<T>.
  • Should we use "provider" instead of "factory" since it's not necessarily new?
    • ILoggerFactory doesn't always give you new instances anyway
  • Should we make it generic?
    • Probably considering "Byte" is nowhere in the name.

API Approved!

Microsoft.AspNetCore.Connections.Abstractions.dll

namespace Microsoft.AspNetCore.Connections;

+ public interface IMemoryPoolFactory<T>
+ {
+    MemoryPool<T> Create();
+ }