2

I've been following Pluralsight course, authored by Simon Robinson on Concurrent Collections.

He uses AddOrUpdate in the following way in order to make it thread-safe:

public bool TrySellShirt(string code)
{
    bool success = false;

    _stock.AddOrUpdate(code, 
        (itemname) => { success = false; return 0; },
        (itemName, oldValue) =>
        {
            if (oldValue == 0)
            {
                success = false;
                return 0;
            }
            else
            {
                success = true;
                return oldValue - 1;
            }
        });

    if (success)
        Interlocked.Increment(ref _totalQuantitySold);

    return success;
}

So, I'm aware that AddOrUpdate isn't entirely atomic, as it says in the docs: " the addValueFactory and updateValueFactory delegates are called outside the locks to avoid the problems that can arise from executing unknown code under a lock. "

That to me is clear. What isn't clear is what's the point of setting success to false within the delegates. AddValueFactory argument is deliberately used as lambda so success = false can be set, rather than just returning 0. I somewhat understand/think that if the method/lambda is interrupted by another thread (and it can be interrupted because it's called outside of the lock), it will attempt to repeat itself and so we should set state of any corresponding values to their initial value to cleanly engage new iteration, hence setting success = false;.

Also from the docs: If you call AddOrUpdate simultaneously on different threads, addValueFactory may be called multiple times, but its key/value pair might not be added to the dictionary for every call.

If that's the case, I've been checking source code of AddOrUpdate on source.dot.net, I can't see any locks being used anywhere, I can see TryAddInternal and TryUpdateInternal.

Regardless, aforeposted method works but I don't understand why it works, and as soon as I remove seemingly unnecessary success = false assignments it doesn't work, there is mismatch. So I'm curious what makes these delegates repeat themselves after failure?

Questions I have are:

1. Is it safe to use AddOrUpdate as shown or should I just lock everything and forget about it?

2. What makes delegates repeat themselves after being interrupted? Does it have anything to do with 'Compare-and-swap'? (most curious about this one);

3. Are there any topics/concepts you would have me check out to better understand thread-safe environment?

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
Sha Kal
  • 199
  • 1
  • 1
  • 6

1 Answers1

3

Because the addValueFactory and updateValueFactory delegates are invoked by the ConcurrentDictionary without any locks, it is possible for another thread to change the contents of the dictionary whilst the add/updateValueFactory code is running. To deal with this scenario, if the addValueFactory was invoked (because the key didn't exist in the dictionary) it will only add the returned value if the key still doesn't exist in the dictionary. Similarly if the updateValueFactory was invoked, it will only update the value for the key if the current value is still oldValue.

If there's a mismatch as a result of another thread adding/updating/deleting the same key while the add/updateValueFactory code was running, it will simply try invoking the appropriate delegate again based on the latest contents of the dictionary (the delegates aren't "interrupted", and it's the dictionary itself that calls them again the value for the key being added/updated has changed). This explains why you still need the success = false assignments within the lambdas, even though success is initialized to false. The following example might help visualize the behavior:

Initial dictionary state: _stock["X"] = 1

Step Thread 1 Thread 2
1 Calls _stock.AddOrUpdate("X", ...)
2 updateValueFactory invoked (oldValue = 1)
3 Calls _stock.AddOrUpdate("X", ...)
4 updateValueFactory invoked (oldValue = 1)
5 Sets success = true, returns oldValue - 1 = 0
6 Dictionary checks that the value for key "X" is still = 1 (true)
7 Value for key "X" is updated to 0
8 Sets success = true, returns oldValue - 1 = 0
9 Dictionary checks that the value for key "X" is still = 1 (false)
10 updateValueFactory invoked again (oldValue = 0)
11 Sets success = false, returns 0
12 Dictionary checks that the value for key "X" is still = 0 (true)
13 Value for key "X" is updated to 0
14 Final value of success is false Final value of success is true

Note that without the explicit setting of success = false within the oldValue == 0 branch of the if, Thread 1 would've thought that it had still successfully sold the item, even though there was no longer any stock because Thread 2 sold the last one.

The technique in your question therefore works as intended.

Iridium
  • 23,323
  • 6
  • 52
  • 74
  • Thanks for clarification, *1. Is it safe to use AddOrUpdate as shown or should I just lock everything and forget about it?* – Sha Kal Apr 27 '21 at 16:05
  • 1
    @ShaKal - yes, it appears safe to use AddOrUpdate in this way, providing you're careful to implement it as in your example (being sure not to omit any `success = false` assignments). – Iridium Apr 27 '21 at 18:00