views:

205

answers:

2

I have developed an "object pool" and cannot seem to do it without using Thread.Sleep() which is "bad practice" I believe.

This relates to my other question "Is there a standard way of implementing a proprietary connection pool in .net?". The idea behind the object pool is similar to the one behind the connection pool used for database connections. However, in my case I am using it to share a limited resource in a standard ASP.NET Web Service (running in IIS6). This means that many threads will be requesting access to this limited resource. The pool would dish out these objects (a "Get) and once all the available pool objects have been used, the next thread requesting one would simply waits a set amount of time for one of these object to become available again (a thread would do a "Put" once done with the object). If an object does not become available in this set time, a timeout error will occur.

Here is the code:

public class SimpleObjectPool
{
    private const int cMaxGetTimeToWaitInMs = 60000;
    private const int cMaxGetSleepWaitInMs = 10;
    private object fSyncRoot = new object();
    private Queue<object> fQueue = new Queue<object>();

    private SimpleObjectPool()
    {
    }

    private static readonly SimpleObjectPool instance = new SimpleObjectPool();
    public static SimpleObjectPool Instance
    {
        get
        {
            return instance;
        }
    }

    public object Get()
    {
        object aObject = null;
        for (int i = 0; i < (cMaxGetTimeToWaitInMs / cMaxGetSleepWaitInMs); i++)
        {
            lock (fSyncRoot)
            {
                if (fQueue.Count > 0)
                {
                    aObject = fQueue.Dequeue();
                    break;
                }
            }
            System.Threading.Thread.Sleep(cMaxGetSleepWaitInMs);
        }
        if (aObject == null)
            throw new Exception("Timout on waiting for object from pool");
        return aObject;
    }

    public void Put(object aObject)
    {
        lock (fSyncRoot)
        {
            fQueue.Enqueue(aObject);
        }
    }
}

To use use it, one would do the following:

        public void ExampleUse()
        {
            PoolObject lTestObject = (PoolObject)SimpleObjectPool.Instance.Get();
            try
            {
                // Do something...
            }
            finally
            {
                SimpleObjectPool.Instance.Put(lTestObject);
            }
        }

Now the question I have is: How do I write this so I get rid of the Thread.Sleep()?

(Why I want to do this is because I suspect that it is responsible for the "false" timeout I am getting in my testing. My test application has a object pool with 3 objects in it. It spins up 12 threads and each thread gets an object from the pool 100 times. If the thread gets an object from the pool, it holds on to if for 2,000 ms, if it does not, it goes to the next iteration. Now logic dictates that 9 threads will be waiting for an object at any point in time. 9 x 2,000 ms is 18,000 ms which is the maximum time any thread should have to wait for an object. My get timeout is set to 60,000 ms so no thread should ever timeout. However some do so something is wrong and I suspect its the Thread.Sleep)

+2  A: 

Since you are already using lock, consider using Monitor.Wait and Monitor.Pulse

In Get():

lock (fSyncRoot)
{
   while (fQueue.Count < 1)
     Monitor.Wait(fSyncRoot);

   aObject = fQueue.Dequeue();
}

And in Put():

lock (fSyncRoot)
{
   fQueue.Enqueue(aObject);
   if (fQueue.Count == 1)
         Monitor.Pulse(fSyncRoot);
}
Henk Holterman
An excellent, simple solution that does not change my existing code much. I should be using a semaphore as the others have explained but this will do nicely as my testing has shown. Thank you Henk.
VinceJS
It is a (kind of) semaphore. Monitor is fully managed code and should be the first choice, only use something 'heavier' when needed.
Henk Holterman
A: 

you should be using a semaphore.

http://msdn.microsoft.com/en-us/library/system.threading.semaphore.aspx

UPDATE: Semaphores are one of the basic constructs of multi-threaded programming. A semaphore can be used different ways, but the basic idea is when you have a limited resource and many clients who want to use that resource, you can limit the number of clients that can access the resource at any given time.

below is a very crude example. I didn't add any error checking or try/finally blocks but you should.

You can also check: http://en.wikipedia.org/wiki/Semaphore_(programming)

Say you have 10 buckets and 100 people who want to use those buckets. We can represent the buckets in a queue.

At the start, add all of your buckets to the queue

for(int i=0;i<10;i++) 
{
    B.Push(new Bucket());
}

Now create a semaphore to guard your bucket queue. This semaphore is created with no items triggered and a capacity of 10.

Semaphore s = new Semaphore(0, 10);

All clients should check the semaphore before accessing the queue. You might have 100 threads running the thread method below. The first 10 will pass the semaphore. All others will wait.

void MyThread()
{
    while(true)
    {
        // thread will wait until the semaphore is triggered once
        // there are other ways to call this which allow you to pass a timeout
        s.WaitOne();

        // after being triggered once, thread is clear to get an item from the queue
        Bucket b = null;

        // you still need to lock because more than one thread can pass the semaphore at the sam time.
        lock(B_Lock)
        {
            b = B.Pop();
        }

        b.UseBucket();

        // after you finish using the item, add it back to the queue
        // DO NOT keep the queue locked while you are using the item or no other thread will be able to get anything out of it            
        lock(B_Lock)
        {
            B.Push(b);
        }

        // after adding the item back to the queue, trigger the semaphore and allow
        // another thread to enter
        s.Release();
    }
}
Zack
Yes, I see from the documentation that you are right, however I cannot seem to get my brain around it.
VinceJS