[ 
http://issues.apache.org/jira/browse/IBATISNET-134?page=comments#action_12360397
 ] 

Gilles Bayon commented on IBATISNET-134:
----------------------------------------

Better think, that we must add a lock in CacheModel when we retrieve the object 
as in Java V2, something like

public object this [object key] 
{
        get
        {
                lock(this) 
                {
                        if (_lastFlush != NO_FLUSH_INTERVAL
                                && (DateTime.Now.Ticks - _lastFlush > 
_flushInterval.Interval)) 
                        {
                                Flush();
                        }
                }

// ----------------------------- New Code --------------------------
                object value = null;
                lock(this) 
                {
                        value = _controller[key];
                }
// ----------------------------- End n ew Code --------------------------

                lock(_statLock) 
                {
                        _requests++;
                        if (value != null) 
                        {
                                _hits++;
                        }
                }
                return value;
        }

> 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

>
> 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

Reply via email to