-1

I have some c# code which looks like this:

if (cache[filename] != null) {
    return (AppSettings)cache[filename];
}

lock (thisLock)
{
    using (StreamReader sr = new StreamReader(filename))
    {
        instance = (AppSettings)serial.Deserialize(sr);
        cache.Insert(filename, instance, null, Cache.NoAbsoluteExpiration, Cache.NoSlidingExpiration);
    }

}

return (AppSettings)cache[filename];

so, my understanding of a lock, is that once it has become "Unlocked", then the code block will be executes. So in the case of the code above, I'm assuming I'll need another check in the lock code block to see if the object has already been created?

Also how would I check for a deadlock?

Inisheer
  • 20,376
  • 9
  • 50
  • 82
binks
  • 1,001
  • 2
  • 10
  • 26
  • possible duplicate http://stackoverflow.com/questions/6029804/how-does-lock-work-exactly?rq=1 – Ramy M. Mousa Jun 10 '14 at 18:29
  • Note that your code is still completely unsafe. – SLaks Jun 10 '14 at 18:30
  • @SLaks The `Cache` object is specifically designed to be manipulated from multiple threads at the same time, so the fact that one thread may be reading a value while another is writing to that same value isn't a problem (so long as you don't care which order those conceptual operations happen in). The only reason to use a `lock` at all here is to avoid reading from the file using multiple threads (which this code does correctly), and possibly to also avoid reading from the file more than once when there is a cache miss (that part it fails to do, but can easily be modified to do). – Servy Jun 10 '14 at 18:53

2 Answers2

0

The lock instruction will stop any other thread from entering whilst another thread is processing. You can use it to protected resources that are not thread safe.

In your case the dictionary cache is what you want to protect because if multiple threads are adding, changing and deleting elements inside it your program will almost certainly crash.

To protect cache you need to wrap the whole operation in a lock statement like this:

AppSettings result = null;

lock (thisLock)
{
    if (cache[filename] != null) {
        result = (AppSettings)cache[filename];
    }
    else
    {
        using (StreamReader sr = new StreamReader(filename))
        {
            instance = (AppSettings)serial.Deserialize(sr);
            cache.Insert(filename, instance, null, Cache.NoAbsoluteExpiration, Cache.NoSlidingExpiration);
        }

        result = (AppSettings)cache[filename];
    }
}

return result;
Slade
  • 2,311
  • 3
  • 21
  • 25
  • The `Cache` object, as per its documentation, is specifically designed to be manipulated from multiple threads at the same time, as such the OP's code is actually entirely safe. – Servy Jun 10 '14 at 18:55
  • In that case, I agree, there's no need to protect it. – Slade Jun 10 '14 at 19:26
0

So in the case of the code above, I'm assuming I'll need another check in the lock code block to see if the object has already been created?

Yes. If two threads both check and see that the cache item is missing, you want just one of them to actually do the expensive work of creating the item in the cache, not both of them, so you need to apply the same check again inside of the lock. This is the famous "double checked locking" pattern.

Also how would I check for a deadlock?

The only way I see your code has of deadlocking would be if some other thread is holding onto a lock on the file you want to access, and it's waiting on some thread that is inside your lock block for something else it wants to do. That would be a deadlock. If you ensure that this code is the only code that would ever access that file, or that anything accessing this file wouldn't be dependent on this thread, then (given the code you've shown) you'll be fine.

Servy
  • 202,030
  • 26
  • 332
  • 449