locking only when modifying vs entire method

When should locks be used? Only when modifying data or when accessing it as well?

public class Test {
    static Dictionary<string, obj开发者_开发百科ect> someList = new Dictionary<string, object>();

    static object syncLock = new object();

    public static object GetValue(string name) {
        if (someList.ContainsKey(name)) {
            return someList[name];
        } else {
            lock(syncLock) {
                object someValue = GetValueFromSomeWhere(name);
                someList.Add(name, someValue);
            }
        }
    }
}

Should there be a lock around the the entire block or is it ok to just add it to the actual modification? My understanding is that there still could be some race condition where one call might not have found it and started to add it while another call right after might have also run into the same situation - but I'm not sure. Locking is still so confusing. I haven't run into any issues with the above similar code but I could just be lucky so far. Any help above would be appriciated as well as any good resources for how/when to lock objects.

You have to lock when reading too, or you can get unreliable data, or even an exception if a concurrent modification physically changes the target data structure.

In the case above, you need to make sure that multiple threads don't try to add the value at the same time, so you need at least a read lock while checking whether it is already present. Otherwise multiple threads could decide to add, find the value is not present (since this check is not locked), and then all try to add in turn (after getting the lock)

You could use a ReaderWriterLockSlim if you have many reads and only a few writes. In the code above you would acquire the read lock to do the check and upgrade to a write lock once you decide you need to add it. In most cases, only a read lock (which allows your reader threads to still run in parallel) would be needed.

There is a summary of the available .Net 4 locking primitives here. Definitely you should understand this before you get too deep into multithreaded code. Picking the correct locking mechanism can make a huge performance difference.

You are correct that you have been lucky so far - that's a frequent feature of concurrency bugs. They are often hard to reproduce without targeted load testing, meaning correct design (and exhaustive testing, of course) is vital to avoid embarrassing and confusing production bugs.

Lock the whole block before you check for the existence of name. Otherwise, in theory, another thread could add it between the check, and your code that adds it.

Actually locking just when you perform the Add really doesn't do anything at all. All that would do is prevent another thread from adding something simultaneously. But since that other thread would have already decided it was going to do the add, it would just try to do it anyway as soon as the lock was released.

If a resource can only be accessed by multiple threads, you do not need any locks.

If a resource can be accessed by multiple threads and can be modified, then all accesses/modifications need to be synchronized. In your example, if GetValueFromSomeWhere takes a long time to return, it is possible for a second call to be made with the same value in name, but the value has not been stored in the Dictionary.

ReaderWriterLock or the slim version if you under 4.0.

You will aquire the reader lock for the reads (will allow for concurrent reads) and upgrade the lock to the writer lock when something is to write (will allow only one write at the time and will block all the reads until is done, as well as the concurrent write-threads).

Make sure to release your locks with the pattern to avoid deadlocking:

            void Write(object[] args) 
            {


                   this.ReaderWriterLock.AquireWriteLock(TimeOut.Infinite);

                    try 
                    {
                      this.myData.Write(args);

                    }
                    catch(Exception ex) 
                    {

                    }
                    finally 
                    {

                        this.ReaderWriterLock.RelaseWriterLock();
                    }

            }