jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r1155431 - in /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache: AbstractCache.java ConcurrentCache.java
Date Tue, 09 Aug 2011 16:43:20 GMT
Author: jukka
Date: Tue Aug  9 16:43:19 2011
New Revision: 1155431

URL: http://svn.apache.org/viewvc?rev=1155431&view=rev
Log:
JCR-3013: ArrayIndexOutOfBoundsException: ConcurrentCache

The previous fix had a potential concurrency issue where another thread could still access
an overflown access count through getAccessCount() right before the accessCount.set(0) call
was made.

I prevented that problem by switching to a long counter which should make overflows highly
unlikely (you'd need to have multiple processors just repeatedly accessing the cache for at
least decades to get an overflow), and by using Math.abs() in the ConcurrentCache.shrinkIfNeeded()
where the actual problem would then occur.

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/AbstractCache.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/ConcurrentCache.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/AbstractCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/AbstractCache.java?rev=1155431&r1=1155430&r2=1155431&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/AbstractCache.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/AbstractCache.java
Tue Aug  9 16:43:19 2011
@@ -18,7 +18,6 @@ package org.apache.jackrabbit.core.cache
 
 import static org.apache.jackrabbit.core.cache.CacheAccessListener.ACCESS_INTERVAL;
 
-import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 
@@ -57,8 +56,13 @@ public abstract class AbstractCache impl
      * {@link CacheAccessListener#cacheAccessed()} events once every
      * {@link CacheAccessListener#ACCESS_INTERVAL} calls to the protected
      * {@link #recordCacheAccess()} method.
+     * <p>
+     * A long counter is used to prevent integer overflow. Even if the cache
+     * was accessed once every nanosecond, an overflow would only occur in
+     * about 300 years. See
+     * <a href="https://issues.apache.org/jira/browse/JCR-3013">JCR-3013</a>.
      */
-    private final AtomicInteger accessCount = new AtomicInteger();
+    private final AtomicLong accessCount = new AtomicLong();
 
     /**
      * Cache access listener. Set in the
@@ -94,12 +98,7 @@ public abstract class AbstractCache impl
      * interval has passed since the previous listener call.
      */
     protected void recordCacheAccess() {
-        int count = accessCount.incrementAndGet();
-        // guard against integer overflow
-        if (count < 0) {
-            accessCount.set(0);
-            count = accessCount.incrementAndGet();
-        }
+        long count = accessCount.incrementAndGet();
         if (count % ACCESS_INTERVAL == 0) {
             CacheAccessListener listener = accessListener.get();
             if (listener != null) {

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/ConcurrentCache.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/ConcurrentCache.java?rev=1155431&r1=1155430&r2=1155431&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/ConcurrentCache.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/cache/ConcurrentCache.java
Tue Aug  9 16:43:19 2011
@@ -236,7 +236,7 @@ public class ConcurrentCache<K, V> exten
      */
     private void shrinkIfNeeded() {
         // Semi-random start index to prevent bias against the first segments
-        int start = (int) getAccessCount() % segments.length;
+        int start = (int) Math.abs(getAccessCount() % segments.length);
         for (int i = start; isTooBig(); i = (i + 1) % segments.length) {
             synchronized (segments[i]) {
                 Iterator<Map.Entry<K, E<V>>> iterator =



Mime
View raw message