[ http://issues.apache.org/jira/browse/IBATISNET-134?page=comments#action_12360437 ]
Jeremy Gray commented on IBATISNET-134: --------------------------------------- Adding locks to the getter just adds overhead and reduces performance without addressing the race condition. As for the race condition, you'd still be open to it as soon as the getter releases whatever lock(s) it spins up, so you can't address it with changes only to the getter. As for performance, the lock statement isn't meant for high read:write ratio synchronization. You'd need to look at reader/writer locks to maximize performance, and if used correctly they can even simplify the elimination of the race condition. > lock mechanism doesn't prevent duplicate cache keys from being inserted into > IList persistence > ---------------------------------------------------------------------------------------------- > > Key: IBATISNET-134 > URL: http://issues.apache.org/jira/browse/IBATISNET-134 > Project: iBatis for .NET > Type: Bug > Components: DataMapper > Versions: DataMapper 1.1 > Environment: .NET Framework 1.1, Windows XP Professional SP1, SQL Server > 2000. > Reporter: Jeff Ziolkowski > Assignee: Gilles Bayon > > Every so often, our application would encounter a "Cannot insert key. Cause: > Item has already been added." exception. > Upon further investigation, we found we were able to regularly reproduce this > exception by having two users concurrently perform an action in our > application that would call a SQL map to retrieve a list of items to populate > a drop down list. Previous to these actions, the caches for the SQL map were > cleared, meaning the map had to go to the SQL Server to retrieve a fresh > listing. > It seems the request generated by one user would find that the key for the > list wasn't present in the IList collection and would proceed to go to the > SQL Server to retrieve the list, but before the thread has the opportunity to > retrieve the data from the SQL Server and place it in the IList, the request > generated by the second user comes along and finds the key isn't present in > the IList collection and ALSO proceeds to retrieve the data from the SQL > Server. In the end, the requests from both users ends up retrieving data > from the SQL Server and attempts to place both entries in the IList which > causes the collection to throw the duplicate key not allowed exeception. > Here's the property in question. It comes from the LRUCacheController class. > /// <summary> > /// Adds an item with the specified key and value into cached > data. > /// Gets a cached object with the specified key. > /// </summary> > /// <value>The cached object or <c>null</c></value> > public object this [object key] > { > get > { > lock (this) > { > _keyList.Remove(key); > _keyList.Add(key); > return _cache[key]; > } > } > set > { > lock (this) > { > _cache.Add(key, value); > _keyList.Add(key); > if (_keyList.Count > > _cacheSize) > { > object oldestKey = > _keyList[0]; > _keyList.Remove(0); > > _cache.Remove(oldestKey); > } > } > } > } > Microsoft has noted that the lock(this) statement shouldn't be used on > non-static members, as multiple instances of the object can exist and > therefore cause synchronization issues. In a multi-threaded scenario, with > multiple requests for the same map coming in on different threads, this seems > like a problem. In addition, within the set section of this code... there > should be a check to see if the key already exists in the collection just to > be safe. -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
