Using the ConcurrentBag as an object pool for an ASP.NET application

Question

Is there anything wrong with this implementation of an object pool that will be called from an ASP.NET application?

The object I want to pool has extremely expensive initialization, so currently I am storing one instance of it in a static variable and using C# lock() to avoid race conditions. I want to change to an object pool so that more that one asp.net thread can make use of it simultaneously.

The object pool must have a maximum size restriction, so that callers of GetObject wait in line to get an instance of the pooled object. For example, if I have a maximum size of 10, and suddenly 20 asp.net threads need an instance, 10 threads should get an instance immediately, and the other 10 threads should wait patiently (and in turn) for one of the 10 instances to be freed up.

The object pool class below is stolen nearly in-toto from this MSDN article, except that it incorporates Scott Chamberlain's suggestion to use a BlockingCollection:

public class ObjectPool<T>
{
 private ConcurrentBag<T> _objects;
 private Func<T> _objectGenerator;
 public ObjectPool(Func<T> objectGenerator, 
 int boundedCapacity,
 int initializeCount = 0)
 {
 if (objectGenerator == null) throw new ArgumentNullException("objectGenerator");
 _objects = new BlockingCollection<T>(new ConcurrentBag<T>(), boundedCapacity);
 _objectGenerator = objectGenerator;
 if (initializeCount > 0)
 {
 int counter = 0;
 while (counter++ < initializeCount)
 {
 _objects.Add(_objectGenerator());
 }
 }
 }
 public T GetObject()
 {
 T item;
 if (_objects.TryTake(out item)) return item;
 return _objectGenerator();
 }
 public void PutObject(T item)
 {
 _objects.Add(item);
 }
}

From my ASP.NET code, I'm instantiating the pool as a static variable:

static ObjectPool<MyClass> _pool = new ObjectPool<MyClass>(() => {
 var myClass = new MyClass();
 myClass.DataSource = something;
 return myClass;
 }, 10, 5); 

And then using it like this, in a try-finally block so that the PutObject method is always called. The ConcurrentBag is called "thread safe" so I am not locking access to the GetObject or PutObject methods.

MyClass myClass = _pool.GetObject();
try
 {
 myClass.Input = "123 Main, Anytown, MI";
 var result = myClass.ValidateAddress();
 }
finally
 {
 _pool.PutObject(myClass);
 }

• Scott Chamberlain: To handle your "limit to 10 items" use a BlockingCollection backed by a ConcurrentBag, then replace GetObject with T GetObject() { return _blockingCollection.Take();}

Answer 1

1. Instead of

    if (initializeCount > 0)
   {
    int counter = 0;
    while (counter++ < initializeCount)
    {
    _objects.Add(_objectGenerator());
    }
   }
   

   I would prefer a for loop for initial initialization:

   for (int i = 0; i < initializeCount; ++i)
   {
    _object.Add(_objectGenerator());
   }
   

2. The biggest problem however is that GetObject will return a new object anyway - even if the pool is empty. This violates your requirement:

   The object pool must have a maximum size restriction, so that callers of GetObject wait in line to get an instance of the pooled object.

   As mentioned by Scott in his comment to your question you should use Take to retrieve an item as it will block until one is available.

3. You might want to consider providing some sort of "object resetter" action which gets called whenever you have retrieved an item from the pool which puts the item into a clean initial state automatically.