Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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);
    }
share|improve this question
2  
To handle your "limit to 10 items" use a BlockingCollection backed by a ConcurrentBag, then replace GetObject with T GetObject() { return _blockingCollection.Take();} – Scott Chamberlain yesterday

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Browse other questions tagged or ask your own question.