Semaphore race condition?

Question

I have created a "Manager" class that contains a limited set of resources. The resources are stored in the "Manager" as a Queue. I initialize the Queue and a Semaphore to the same size, using the semaphore to block a thread if there are no resources available. I have multiple threads calling into this class to request a resource. Here is the psuedo code:

public IResource RequestResource()
{
     IResource resource = null;
     _semaphore.WaitOne();

     lock (_syncLock)
     {
         resource = _resources.Dequeue();
     }
     return resource;
}
public void ReleaseResource(IResource resource)
{
    lock (_syncLock)
    {
        _resources.Enqueue(resource);
    }
    _semaphore.Release();
}

While running this application, it seems to run fine for a while. Then, it seems like my Queue is giving out the same object. Does this seem like its possible? I'm pulling my hair out over here, and any help would be greatly appreciated. Feel free to ask for more information if you need it. Thanks!

Answer 1

The semaphore is really incidental here; that just limits the number of consumers that can acquire resources at the same time. The synchronization is actually coming from the lock statement (critical section).

As far as I can tell, that code is thread-safe. When you say it's "giving out the same object" - maybe I'm misunderstanding the question, but it should be giving out the same object, because when callers invoke the Release method, they put back the same resource they got originally, so the same object will be in the queue at several points during execution.

If what you mean is that the RequestResource method is returning resources that have already been acquired but not released, there are only three possible reasons that I can think of:

1. The queue contained duplicate resources from the beginning;

2. A consumer called the Release method twice. In your Release method, you don't actually check to see whether or not the resource is already back in the queue; you might want to alter this code to check for this and throw an exception, which would help you catch where Release calls are being duplicated (if this is in fact the case).

3. Some other code is accessing the _resources queue without a lock.

My suspicion would be #2, if this is indeed what you mean by duplicates.

Answer 2

In addition to Aaronaught's answer:

It also looks entirely possible for two threads to acquire the semaphore, then one to enter the critical section in RequestResource(), and the the other block, followed by the first thread completing all its work and returning the object to the queue with ReleaseResource() before the second thread waking up and having a chance to dequeue. That would give an impression of two threads obtaining the same object.