Improved Thread locking advice needed

Question

I have the following scenario:

I'm trying to lock a thread in place, if that threads 'custom' id matches the one that has already entered the locked section off code, but not if the id differs.

I created some sample code to explain the behaviour I want

class A
{        
    private static Dictionary<int, object> _idLocks = new Dictionary<int, object>();
    private static readonly object _DictionaryLock = new object();
    private int _id;

    private void A (int id)
    {
        _id = id;
    }

    private object getObject()
    {
        lock (_DictionaryLock)
        {
            if (!_idLocks.ContainsKey(_id))
                _idLocks.Add(_id, new object());
        }
        lock (_idLocks[_id])
        {
            if (TestObject.Exists(_id))
                return TestObject(_id);
            else
                return CreateTestObject(_id);
        }
    }
}

Now this works 100% for what I extended, where id example 1 does not check to see if its object has been created while another thread with id 1 is already busy creating that object.

But having two locks and a static dictionary does not seem correct way of doing it at all, so I'm hoping someone can show me an improved method of stopping a thread from accessing code only if that thread was created with the same id as the one already busy executing the code in the locked section.

I was looking at the ReaderWriterLockSlim class but to me it didn't really make sense to be used cause I don't want object TestObject(id) to be read at all while it's still being created.

I don't care about locking the thread from accessing a dictionary. What I'm trying to avoid at all cost is the _id which that thread runs should not be used inside CreateTestObject(_id) while there is already one busy, because files are being created and deleted with that id which will throw exceptions if two threads are trying to access the same files

Which is fixable with just a normal lock, but in this case I still want a thread whose _id is not currently running inside the CreateTestObject(_id) method to be able to enter the code within the lock.

This is all because what happens inside CreateTestObject takes time and performance will be impacted if a thread is waiting to access it.

Answer 1

It looks like you're using this code to populate a dictionary in a thread-safe manner - could you use a ConcurrentDictionary instead?

class A {
  private static ConcurrentDictionary<int, object> _dictionary = new ConcurrentDictionary<int, object>();

  private int _id;

  private object GetObject() {
    object output = null;
    if(_dictionary.TryGetValue(_id, output)) {
      return output;
    } else {
      return _dictionary.GetOrAdd(_id, CreateTestObject(_id));
    }
  }
}

---

Edit: If you want to completely eliminate the possibility of invoking duplicate CreateTestObject methods then you can store a wrapper in _dictionary that lazily sets object

class Wrapper {
  private volatile object _obj = null;

  public object GetObj() {
    while(_obj == null) {
      // spin, or sleep, or whatever
    }
    return _obj;
  }

  public void SetObj(object obj) {
    _obj = obj;
  } 
}

class A {
  private static ConcurrentDictionary<int, Wrapper> _dictionary = new ConcurrentDictionary<int, Wrapper>();

  private int _id;

  private object GetObject() {
    Wrapper wrapper = null;
    if(_dictionary.TryGetValue(_id, wrapper)) {
      return wrapper.GetObj();
    } else {
      Wrapper newWrapper = new Wrapper();
      wrapper = _dictionary.GetOrAdd(_id, newWrapper);
      if(wrapper == newWrapper) {
        wrapper.SetObj(CreateTestObject(_id));
      }
      return wrapper.GetObj();
    }
  }
}

Only one thread will be able to put a new Wrapper in _dictionary at the specified _id - that thread will initialize the object inside of the wrapper == newWrapper conditional. Wrapper#GetObj spins until the object is set, this can be rewritten to block instead.