Add policy factory overloads for pooled instance registration

[zms9110750]

Hi, this was created with AI assistance to address issue #6. However, I'm not very familiar with this project, so I'd appreciate a review to confirm everything is correct.

---

Add new PooledInstancePerLifetimeScope(Func<IComponentContext, IPooledRegistrationPolicy<TLimit>>) and PooledInstancePerMatchingLifetimeScope(Func<IComponentContext, IPooledRegistrationPolicy<TLimit>>, params object[]) overloads that accept a factory delegate, allowing the pooling policy to be resolved from the container at registration time rather than supplied as an already-built instance.

This enables policies with container-managed dependencies (e.g., a cache-backed policy that resolves IMemoryCache from DI).

Design

The factory is called once when the pool is built, and the resulting policy instance is shared between the pool's create/return path and the get-side resolve path via a new internal PooledInstanceContext<T> type. This preserves the behavior of the existing IPooledRegistrationPolicy<T> overloads — a single policy object handles both Get and Return.

The factory is passed as a typed constructor argument to the activator (not via registration metadata), consistent with the existing overloads.

Changes

• PoolActivator<T>: new constructor accepting (Service, Func<IComponentContext, IPooledRegistrationPolicy<TLimit>>); pipeline calls factory once and wraps the pool + policy in PooledInstanceContext<T>
• PoolGetActivator<T>: new constructor accepting only PoolService; extracts shared policy from PooledInstanceContext<T> at resolve time (no per-resolve factory invocation)
• PooledInstanceContext<T>: new internal type that bundles the resolved pool and policy for sharing between activators
• RegistrationExtensions: 2 new public overloads — PooledInstancePerLifetimeScope(policyFactory) and PooledInstancePerMatchingLifetimeScope(policyFactory, tags); 1 private RegisterPooled(policyFactory, tags) method

Tests

9 new tests covering:

• Happy path registration and resolution
• Policy resolved from container (dependency injection)
• Cross-scope instance reuse
• Factory receives IComponentContext with access to registered services
• Matching lifetime scope tag support
• Non-singleton policy sharing (factory called exactly once)
• Null argument guards for both overloads
• Concurrent usage

[tillig]

Thank you for putting this together — and for sticking with the conversation while we worked out exactly what you were after. You were right that there's a real gap here, and I appreciate the effort you put into the implementation and tests.

I'd like to help get this merged, but I think we'll get there faster if we narrow the scope a bit. Here's how I'm seeing it:

The part I'd love to take

The core of your request — being able to supply the pooling policy lazily from the container instead of as an already-built instance — is a great fit. It's the same "give me the recipe, not the instance" pattern we use throughout Autofac, and it solves your cache-backed scenario cleanly. Concretely, that's a deferred overload of the existing methods:

PooledInstancePerLifetimeScope(
    this IRegistrationBuilder<...> registration,
    Func<IComponentContext, IPooledRegistrationPolicy<TLimit>> policyFactory)

(plus the matching PooledInstancePerMatchingLifetimeScope variant). Keeping the same method names and the IPooledRegistrationPolicy vocabulary lets this slot right in next to the overloads that are already there.

A few implementation notes when you get to it:

• Let's invoke the factory once when the pool is built and capture the result, rather than per-resolve — keeps the allocation/perf profile in line with the existing path.
• Pass the factory through as a constructor argument to the activator (the way the current policy is) rather than via registration metadata, so it doesn't leak into the public component metadata.
• Worth a test that proves the policy can resolve another registered dependency (your cache) — that's the heart of the scenario and the thing I most want to see locked down.

The part I'd like to set aside for now

The fully custom ObjectPool<T> path (ConfigurePool) is trickier, and I don't think we should rush it. When the pool itself is user-supplied, Autofac is no longer the thing constructing the pooled instances, which means activation and the IPooledComponent get/return hooks quietly stop applying. That's a real behavioral shift, and I'd want to think through whether it's something we want to own long-term before we commit to supporting it forever. If you have a concrete need there, let's discuss it in the original issue so we can design it deliberately rather than bolt it on.

Example: your cache-backed scenario, no custom pool needed

The key insight is that the policy is the right place for cache-backed behavior, and as a constructor dependency, just like any other component.

Your policy decides what Get and Return mean, and it can lean on the cache for idle/pressure-based eviction:

  public sealed class CacheBackedPolicy<T> : DefaultPooledRegistrationPolicy<T>
      where T : class
  {
      private readonly IMemoryCache _cache;

      public CacheBackedPolicy(IMemoryCache cache) => _cache = cache;

      public override T Get(IComponentContext context, IEnumerable<Parameter> parameters, Func<T> getFromPool)
      {
          // Hand back a cached instance if one is available; otherwise pull from the pool.
          if (_cache.TryGetValue(typeof(T), out T cached) && cached is not null)
          {
              _cache.Remove(typeof(T));
              return cached;
          }

          return getFromPool();
      }

      public override bool Return(T pooledObject)
      {
          // Park the instance in the cache with whatever eviction policy you like
          // (sliding expiration for idle time, size limits for pressure, etc.).
          _cache.Set(typeof(T), pooledObject, new MemoryCacheEntryOptions
          {
              SlidingExpiration = TimeSpan.FromMinutes(5),
              Size = 1,
          });

          // Returning false means "I've taken ownership of this instance" — the pool
          // won't retain it, because the cache is now managing its lifetime.
          return false;
      }
  }

And registration ties it together — note the cache is registered normally, and the policy factory resolves it at build time, which is exactly the ordering problem you ran into:

  builder.RegisterType<MemoryCache>()
         .As<IMemoryCache>()
         .SingleInstance();

  builder.RegisterType<Hello>()
         .PooledInstancePerLifetimeScope(ctx =>
             new CacheBackedPolicy<Hello>(ctx.Resolve<IMemoryCache>()));

That gives you a generic, cache-driven pooling strategy that's resolved from the container, without Autofac having to hand off the whole ObjectPool<T>. The policy keeps Autofac in charge of constructing instances (so constructor injection and the IPooledComponent hooks keep working), while you stay in charge of retention and eviction.

Suggested path forward

Could we trim this PR down to just the policyFactory overloads (with the existing method names) and a couple of focused tests? That's something I can review and merge quickly. The custom-pool idea can live as its own discussion, and if we land on a good design it becomes a clean follow-up PR.

None of this is a reflection on the work — it's solid, and the underlying idea is one we want. I'd just rather ship the high-value piece now and give the bigger idea the consideration it deserves. Thanks again for contributing!

[zms9110750]

Actually, I still want the Func<IComponentContext, ObjectPool<T>> overload.

In my view, the semantics of an object pool should be the same as a cache. I'm not satisfied with Microsoft's default ObjectPool<T> implementation itself.

If you only need the Func<IComponentContext, IPooledRegistrationPolicy<T>> part, the very first version of this commit was exactly that.

The problem was that my English isn't good, so I relied on AI to analyze the project. I didn't realize that IPooledRegistrationPolicy is a policy abstraction, not the pool itself. That's why in the first commit, I didn't wrap the policy with a default pool.

If you don't accept the Func<IComponentContext, ObjectPool<T>> overload, I'll be disappointed. But I can still help you get the Func<IComponentContext, IPooledRegistrationPolicy<T>> part merged. As a DI container, the beauty of it is that you can register open generics for anything — including this policy.

Still, the same issue remains: the content was created by AI and needs review.

[tillig]

That's really helpful context — thank you for being so candid about how the PR came together. And to be clear up front: I'm not saying no to Func<IComponentContext, ObjectPool<T>>. I'm saying it needs more design than this PR worked through, and I'd rather get there deliberately than merge something we'll all have to live with forever.

Your motivation is completely legitimate. "I don't want Microsoft's default ObjectPool<T>, I want pool semantics that behave like a cache" is a real, reasonable goal, and "register it as an open generic" is exactly the kind of thing a DI container should make easy. So the idea has my interest.

The reason I want to slow down is that handing Autofac a fully custom ObjectPool<T> changes some load-bearing assumptions, and the current implementation doesn't yet have...

• Instance construction. Today Autofac builds the pooled instances (constructor injection, parameters, the registration's activator). A user-supplied pool calls its own Get(), so who constructs the instance — and how does it get its dependencies injected? Right now that path quietly bypasses Autofac's activation.
• IPooledComponent hooks. OnGetFromPool / OnReturnToPool stop firing on the custom-pool path. That's a silent behavioral change for anyone relying on them.
• Ownership and disposal. With the default provider we know who disposes instances that aren't retained. With an external pool, the ownership contract is undefined — we need to decide and document who disposes what, and when.
• Pool lifetime. The pool is registered at the root scope. If it's resolved from the container, we need to be explicit about its lifetime, disposal, and thread-safety guarantees.
• Open generics. This is the scenario you actually care about, and it's the one the PR doesn't exercise at all — there are no tests proving an open-generic custom pool resolves and behaves correctly.
• Test coverage. The ConfigurePool path currently has no tests, so none of the above is pinned down.

None of these are dealbreakers — they're just open questions, and I don't think it's fair to you or to the project to merge the custom-pool API before we've answered them. That's design work worth doing in its own issue, where we can talk through the contract before writing more code.

So here's what I'd suggest, and I think it's genuinely good news for you:

1. Let's land the Func<IComponentContext, IPooledRegistrationPolicy<T>> overload now. You've offered to help get that part merged — that would be fantastic, and it's immediately useful. It's a clean, low-risk first step.
2. Let's keep the custom-pool idea alive in #6 (or a fresh issue), where we work through the bullets above together. Once we agree on the contract, the implementation becomes a focused follow-up PR — and you'll have shipped the foundation it builds on.

This isn't the door closing on the custom pool; it's us taking it one solid step at a time so each piece is something we're confident owning long-term. I'd really like to get your name on both. And no worries at all about the AI-assisted parts — that's exactly what review is for. Thanks for pushing on this.

[zms9110750]

Thank you so much for your detailed response.

The thorough context translated your statements with remarkable precision.
Your kindness and courtesy also felt genuinely warm.

I believe this commit should now be exactly what you're looking for.
I'm excited to think through the next steps with you.

[tillig]

This is a big step in the right direction — thank you for reworking it. Dropping the custom ObjectPool<T> path and scoping down to policyFactory overloads on the existing PooledInstancePerLifetimeScope / PooledInstancePerMatchingLifetimeScope methods is exactly the shape we were hoping for. The API naming and vocabulary are consistent with what's already there, the metadata leak is gone, the factory is passed as a typed constructor argument, and there's no dead code this time. The direction is right and this is close.

I'm requesting changes on the following — I'd like all of these addressed before merge:

1. Resolve the policy once and share that single instance between pool creation (PoolActivator) and the get-side (PoolGetActivator). Right now the factory is invoked per-resolve on the get-side and separately when the pool is built, so a non-singleton policy ends up with Get and Return running on different instances. A stateful policy would break, and the current tests don't catch it because they all use RegisterInstance.
2. Revert (or split out) the OnReturnToPool move between AutofacPooledObjectPolicy and PooledInstanceTracker — it's unrelated to this feature, touches the shared return path, and may quietly change when the hook fires.
3. Add a non-singleton/stateful policy test (this locks down #1), plus null-argument tests, and a concurrency case. Give PolicyFactory_PolicyAcceptsAllReturns a real assertion or remove it.
4. Clean up the double blank line and confirm a green Release build via dotnet msbuild ./default.proj.
5. Update the PR title and description — both still describe the removed ObjectPool<T> design. The title ("Add ObjectPool factory overload...") should reflect what the PR now does, e.g. "Add policy factory overloads for pooled instance registration", since it becomes the squash-merge commit message and release-note entry. The body references a DelegatingPooledRegistrationPolicy<T> type and an ObjectPool<TLimit> factory overload, neither of which exists in the current diff — please update it to describe the Func<IComponentContext, IPooledRegistrationPolicy<TLimit>> overloads that are actually here.

Once the policy-instance handling is sorted and the unrelated refactor is out, I'm happy to merge. Really appreciate how you've engaged with the feedback on this — the design is in good shape now. And as discussed, the custom-pool idea stays alive in #6 as a deliberate follow-up.

[tillig]

This looks great. Thanks for the contribution! We can continue the custom object pool design discussion in #6 .

[codecov]

Codecov Report

❌ Patch coverage is 87.12121% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.18%. Comparing base (a8982d9) to head (4ffb101).
⚠️ Report is 12 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/Autofac.Pooling/RegistrationExtensions.cs	83.33%	8 Missing and 8 partials ⚠️
src/Autofac.Pooling/PoolActivator.cs	96.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop      #14      +/-   ##
===========================================
- Coverage    86.19%   86.18%   -0.01%     
===========================================
  Files            8        9       +1     
  Lines          210      333     +123     
  Branches        19       32      +13     
===========================================
+ Hits           181      287     +106     
- Misses          18       26       +8     
- Partials        11       20       +9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.