0

I have the following code. When deployed to a server, i m getting exception that , this object already exists in the dictionary. Even though , i did double locking, synchrony didnt work quite well. What s the best way to implement this? How can i best lock access to this section? Should i implement a singleton class and place the methods in there?

Should i lock the collection or mutex?

NO I M NOT ON .NET 4

As you can see, i m trying to do a simple cache based on datetime.

         // within the class.
         IDictionary<id, MyObj> _mydict = new Dictionary<string, MyObj>();
         object mutex = new object;

         //within some method comes the following

         if (_mydict.TryGetValue(id, out myobj))
         {
             DateTime now = DateTime.Now;
             DateTime expiry = myobj.Timestamp;
             TimeSpan span = now.Subtract(expiry);
             if (span.Minutes  0)
             {
                 lock (mutex)
                 {
                     myobj= DataAccess.GetMyObj(id);
                     _mydict[id] = myobj;
                 }

                 return myobj;
             }
             else
             {
                 return myobj;
             }
         }
         else
         {
             lock (mutex)
             {
                 if (_mydict.TryGetValue(id, out myobj))
                 {
                     return myobj;
                 }
                 else
                 {
                     lock (mutex)
                     {
                         myobj = DataAccess.GetMyObj(id);
                         _mydict[id] = myobj;
                     }

                     return myobj;
                 }                    
             }
         }
Jeff Sternal
  • 47,787
  • 8
  • 93
  • 120
DarthVader
  • 52,984
  • 76
  • 209
  • 300

4 Answers4

2

If you are using .Net 4 using the ConcurrentDictionary class should be quite helpful. It will take care of the locking for you.

x112358
  • 121
  • 4
  • Generally [System.Collections.Concurrent](http://msdn.microsoft.com/en-us/library/dd287108.aspx) namespace has a lot of cool datastructures, not many people use them. :( – Sanjeevakumar Hiremath Mar 01 '11 at 00:34
  • Should _mydict[name] = myobj; be _mydict[id] = myobj; in the first block of code? Should _internetServiceProviderCache[id] = myobj; be _mydict[id]? – rsbarro Mar 01 '11 at 00:59
2

Just use ASP.Net caching instead (you don't need to be creating a web application for this to work, just add a reference to System.Web):

void get(string key)
{
    return HttpRuntime.Cache[key];
}
void set(string key, object value, DateTime expires)
{
    HttpRuntime.Cache.Insert(
        key, 
        value, 
        null, 
        expires, 
        Cache.NoSlidingExpiration);
}

Simple, easy and also means that your cache now also has all the other features of the ASP.Net cache (A cap on cache size, caching dependencies etc...)

Justin
  • 84,773
  • 49
  • 224
  • 367
  • I'd agree, the built in ASP.NET caching in the System.Web namespace works very well and keeps you from having to write all of this stuff yourself. The Caching Application block also works quite well. – rsbarro Mar 01 '11 at 01:11
  • 1
    @user177883 Why? Its been part of the .Net framework since version 2.0 and is completely independent of the ASP.Net framework other than the reference to System.Web. – Justin Mar 01 '11 at 01:21
  • 1
    Check out this link. On .NET 4.0 you can use the ObjectCache instead of the ASP.NET cache. The Framework documentation recommends only using the HttpRuntime cache in ASP.NET applications. http://msdn.microsoft.com/en-us/library/system.runtime.caching.objectcache.aspx – rsbarro Mar 01 '11 at 03:54
  • @rsbarro Very useful looking - alas the poster and I are both stuck using antiquated technology (.Net 3.5) – Justin Mar 01 '11 at 04:05
2

Dictionary isn't threadsafe to read from, you should lock before even calling TryGetValue, basically if you are using a Dictionary only one thing can touch it at a time

BrandonAGr
  • 5,827
  • 5
  • 47
  • 72
  • dude, why would u lock the whole dictionary for a look up? then what s the point of using a dictionary on a multithreaded environment, if it will work like a singleton ??????? – DarthVader Mar 01 '11 at 01:26
  • This post has a good explanation of why double checked locking doesn't work with Dictionary. See http://stackoverflow.com/questions/2624301/how-to-show-that-the-double-checked-lock-pattern-with-dictionarys-trygetvalue-is – rsbarro Mar 01 '11 at 03:33
0

Why not try using a list, and insert an item that is a class with your value and the timestamp that you inserted the value. Then you can search for the latest timestamp and value, order the resulted list, and use the top item. You can then expire the old items as you see fit.

Ritch Melton
  • 11,498
  • 4
  • 41
  • 54
  • Right, using a dictionary confines you to searching for a simple key. If you use a list _and_ use the value and a timestamp to search, then you don't have to worry about multiple insertions. Your problem then becomes about how to expire the old entries in the cache that aren't being used. – Ritch Melton Mar 01 '11 at 03:46