0

I'm implementing a resource manager class for my application that should handle multiple threads.

The requirement for this manager is that it will be given a class and a limit of instances it will be allowed to create of that class, and threads will call "Request" and "Return" methods in order to give and take instances from the manager, if the manager finds that the maximum number of instances are already lying in the application, I want it to block the requesting thread until another thread returns an instance of the resource, hence the code:

public ResourceManager<T> where T : new(){

    private object _locker = new object();

    public T RequestResource(){

        lock (_locker){
            while (!IsAnyAvailable()){
                Monitor.Wait(_locker);
            }

            return GetResource();
        }
    }

    public void ReturnResource(T instance){

        lock (_locker){
            ReturnResourceLogic(instance);
            Monitor.Pulse(_locker);
        }       

    }
}

While this code works for me, after reading this post on thread synchronization, I realized that because the threads in the waiting queue go to the back of the ready queue (instead of the front), a hypothetical situation where several threads keep requesting the resources could starve an "unlucky" thread that happens to keep going back and forth from ready queue to waiting queue while other threads "cut the line" when it's in the waiting queue.

How could this code be changed in order to make this resource manager "fair" and respect the order of requests from the threads?

EDIT: Here's a failing test that creates the "hypothetical situation" where either thread 1 or 2 will always starve

[TestClass]
public class UnitTest2
{
    private class ResourceManager<T> where T : new()
    {
        private int n=0;
        private object _locker = new object();

        public T RequestResource()
        {

            lock (_locker)
            {
                while (!IsAnyAvailable())
                {
                    Monitor.Wait(_locker);
                }

                return GetResource();
            }
        }

        private T GetResource()
        {
            n++;
            return new T();
        }

        private bool IsAnyAvailable()
        {
            return n < 1;
        }

        public void ReturnResource(T instance)
        {
            lock (_locker)
            {
                ReturnResourceLogic(instance);
                Monitor.Pulse(_locker);
            }

        }

        private void ReturnResourceLogic(T instance)
        {
            //Simulates some work
            Thread.Sleep(500);
            n--;
        }
    }

    class DummyClass { }

    [TestMethod]
    public void TestMethod1()
    {
        bool fair1 = false;
        bool fair2 = false;
        bool fair3 = false;
        var manager = new ResourceManager<DummyClass>();

        Stopwatch watch = new Stopwatch();

        //string builder for debugging purposes
        StringBuilder builder = new StringBuilder();

        Thread th1 = new Thread(() =>
        {
            while (true)
            {
                builder.AppendLine(watch.Elapsed + " 1 requesting");
                var temp = manager.RequestResource();
                builder.AppendLine(watch.Elapsed + " 1 got resource");
                fair1 = true;
                Thread.Sleep(500);
                builder.AppendLine(watch.Elapsed + " 1 returning");
                manager.ReturnResource(temp);
                builder.AppendLine(watch.Elapsed + " 1 returned");
                Thread.Sleep(750);
            }
        });

        Thread th2 = new Thread(() =>
        {
            while (true)
            {
                builder.AppendLine(watch.Elapsed + " 2 requesting");
                var temp = manager.RequestResource();
                builder.AppendLine(watch.Elapsed + " 2 got resource");
                fair2 = true;
                Thread.Sleep(500);
                builder.AppendLine(watch.Elapsed + " 2 returning");
                manager.ReturnResource(temp);
                builder.AppendLine(watch.Elapsed + " 2 returned");
                Thread.Sleep(750);
            }
        });

        Thread th3 = new Thread(() =>
        {
            Thread.Sleep(750);
            while (true)
            {
                builder.AppendLine(watch.Elapsed + " 3 requesting");
                var temp = manager.RequestResource();
                builder.AppendLine(watch.Elapsed + " 3 got resource");
                fair3 = true;
                Thread.Sleep(500);
                builder.AppendLine(watch.Elapsed + " 3 returning");
                manager.ReturnResource(temp);
                builder.AppendLine(watch.Elapsed + " 3 returned");
                Thread.Sleep(750);
            }
        });

        th1.Name = "Thread1";
        th2.Name = "Thread2";
        th3.Name = "Thread3";

        watch.Start();
        th1.Start();
        th2.Start();
        th3.Start();

        Thread.Sleep(60000);

        var result = builder.ToString();

        Assert.IsTrue(fair1);
        Assert.IsTrue(fair2);
        Assert.IsTrue(fair3);
    }
}
Eduardo Wada
  • 2,606
  • 19
  • 31
  • 1
    Your question makes little sense, you work from an assumption that the waiting threads are not waiting for the `_locker` monitor so they don't actually do anything useful when you Pulse(). That is not the case, the only way they got into the wait queue in the first place was by waiting for the monitor. The queue was *meant* to provide fairness. Your problem is imaginary, don't change the code. – Hans Passant Aug 26 '15 at 00:36
  • 1
    "Your problem is imaginary". Right, now tell that to the failing test. – Eduardo Wada Aug 26 '15 at 03:52
  • The queues are fair. Threads cannot cut in line and all of them will be processed. See http://stackoverflow.com/questions/26970783/why-second-example-on-msdn-for-lock-keyword-works-properly-with-monitor-wait/26971293#26971293 – Eli Algranti Aug 26 '15 at 04:01
  • @EliAlgranti the problem is that there are two queues, not just one, threads that come in the ready queue will have priority, abusing this concept is what allowed me to write a failing test – Eduardo Wada Aug 26 '15 at 04:05
  • 1
    Doesn't answer your question, but instead of reinventing the wheel, perhaps it would be easiest/fastest to just reuse code like http://dcutilities.codeplex.com/SourceControl/latest#Bcl/Concurrency/FifoSemaphore.cs since it appears you are remaking a counting semaphore with guaranteed order. – Robert McKee Aug 26 '15 at 04:58
  • @RobertMcKee Actually, I think it does answer the question, as it would make the manager fair, maybe its my question's title that needs to be changed but the FifoSemaphore seems to be exactly what I need – Eduardo Wada Aug 26 '15 at 11:01

0 Answers0