1

I have a private collection (like an ISet<int>) that can be accessed or modified by several threads.

I have logic such that this thread only needs to mutate the ISet in certain states, whereas the other thread always needs to mutate it. The other thread will always take a lock, implemented the lock keyword on the ISet like this:

lock (this._set)
{
    // some mutating operation on this._set
}

For this thread, I only need to perform an operation that mutates the thread if the ISet contains an int called currentValue, which in this case is rare. The operation in the other thread is frequent. Therefore the question is whether I can check to see if the ISet contains currentValue before taking the lock, to minimize the time that this thread blocks the other thread, like this:

if (this._set.Contains(currentValue))
{
    lock (this._set)
    {
            if (this._set.Contains(currentValue))
            {
                // some mutating operation on this._set
            }
    }
}

The question is whether the behavior of the check outside the lock is undefined, or if it can result in the ISet being put into an undefined or unexpected state. Of course the actual value returned cannot be trusted to use, which is why it's checked again inside the lock, but is this a valid optimization that I can expect to work, or will it mess up some internal state of the set? The goal is to take the lock in this thread as rarely as possible.

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
ms5991
  • 201
  • 2
  • 9
  • Would a `ConcurrentDictionary` maybe be an option that tailors to your needs? –  Apr 10 '20 at 02:35
  • When you say that the collection *can be accessed or modified by several threads*, do you mean that the collection is thread safe? – Theodor Zoulias Apr 10 '20 at 11:20

1 Answers1

0

Just reading the collection values won't throw an exception or change the values of the collection if that's what you mean. It may return dirty data, but you already stated that you expect that possibility. I believe reading from a non-thread safe collection like this can even give you incomplete data, or data that is half old state, half new state (see this response to C# multi-threading: Acquire read lock necessary?

Let's assume the first test for _set.Contains(currentValue) returns true and another thread updates the collection immediately after, removing currentValue. You then enter the lock, but now when you test _set.Contains(currentValue), it returns false.

The question is how critical is it for this thread to mutate the collection? Your code may miss out on mutating the collection every time. Your optimization may end up having the opposite effect - slowing the code down by unnecessarily locking.

It would be difficult to provide you a good alternative without knowing other requirements, but I think I've answered your questions and shown that the code is really not optimized at all, but by itself won't break anything.

SeanOB
  • 752
  • 5
  • 24