- use ReentrantReadWriteLock to protect entry cache maps against unsynchron...

abobrov

24.18.2008 c3bcbefc8d1dbd5ce9ff94b068ba5623dce1a173

- use ReentrantReadWriteLock to protect entry cache maps against unsynchronized concurrent structural modifications.

 opends/src/server/org/opends/server/extensions/FIFOEntryCache.java

@@ -38,7 +38,7 @@
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

import org.opends.server.admin.server.ConfigurationChangeListener;
import org.opends.server.admin.std.server.EntryCacheCfg;
@@ -61,7 +61,6 @@

import static org.opends.server.loggers.debug.DebugLogger.*;
import static org.opends.messages.ExtensionMessages.*;
import static org.opends.server.util.ServerConstants.*;



@@ -113,7 +112,9 @@

  // The lock used to provide threadsafe access when changing the contents of
  // the cache.
  private Lock cacheLock;
  private ReentrantReadWriteLock cacheLock;
  private Lock cacheWriteLock;
  private Lock cacheReadLock;

  // The maximum amount of memory in bytes that the JVM will be allowed to use
  // before we need to start purging entries.
@@ -154,8 +155,11 @@
    // Initialize the cache structures.
    idMap     = new HashMap<Backend,HashMap<Long,CacheEntry>>();
    dnMap     = new LinkedHashMap<DN,CacheEntry>();
    cacheLock = new ReentrantLock();

    // Initialize locks.
    cacheLock = new ReentrantReadWriteLock(true);
    cacheWriteLock = cacheLock.writeLock();
    cacheReadLock = cacheLock.readLock();

    // Read configuration and apply changes.
    boolean applyChanges = true;
@@ -186,7 +190,7 @@
   */
  public void finalizeEntryCache()
  {
    cacheLock.lock();
    cacheWriteLock.lock();

    try {
      registeredConfiguration.removeFIFOChangeListener(this);
@@ -202,7 +206,7 @@
        }
      }
    } finally {
      cacheLock.unlock();
      cacheWriteLock.unlock();
    }
  }

@@ -218,7 +222,12 @@
    }

    // Indicate whether the DN map contains the specified DN.
    cacheReadLock.lock();
    try {
    return dnMap.containsKey(entryDN);
    } finally {
      cacheReadLock.unlock();
    }
  }


@@ -229,19 +238,21 @@
  public Entry getEntry(DN entryDN)
  {
    // Simply return the entry from the DN map.
    cacheReadLock.lock();
    try {
    CacheEntry e = dnMap.get(entryDN);
    if (e == null)
    {
      if (e == null) {
      // Indicate cache miss.
      cacheMisses.getAndIncrement();
      return null;
    }
    else
    {
      } else {
      // Indicate cache hit.
      cacheHits.getAndIncrement();
      return e.getEntry();
    }
    } finally {
      cacheReadLock.unlock();
    }
  }


@@ -252,15 +263,17 @@
  public long getEntryID(DN entryDN)
  {
    // Simply return the ID from the DN map.
    cacheReadLock.lock();
    try {
    CacheEntry e = dnMap.get(entryDN);
    if (e == null)
    {
      if (e == null) {
      return -1;
    }
    else
    {
      } else {
      return e.getEntryID();
    }
    } finally {
      cacheReadLock.unlock();
    }
  }


@@ -271,6 +284,8 @@
  public DN getEntryDN(Backend backend, long entryID)
  {
    // Locate specific backend map and return the entry DN by ID.
    cacheReadLock.lock();
    try {
    HashMap<Long,CacheEntry> backendMap = idMap.get(backend);
    if (backendMap != null) {
      CacheEntry e = backendMap.get(entryID);
@@ -279,6 +294,9 @@
      }
    }
    return null;
    } finally {
      cacheReadLock.unlock();
    }
  }


@@ -295,7 +313,7 @@
    // Obtain a lock on the cache.  If this fails, then don't do anything.
    try
    {
      if (! cacheLock.tryLock(getLockTimeout(), TimeUnit.MILLISECONDS))
      if (!cacheWriteLock.tryLock(getLockTimeout(), TimeUnit.MILLISECONDS))
      {
        return;
      }
@@ -388,7 +406,7 @@
    }
    finally
    {
      cacheLock.unlock();
      cacheWriteLock.unlock();
    }
  }

@@ -406,7 +424,7 @@
    // Obtain a lock on the cache.  If this fails, then don't do anything.
    try
    {
      if (! cacheLock.tryLock(getLockTimeout(), TimeUnit.MILLISECONDS))
      if (!cacheWriteLock.tryLock(getLockTimeout(), TimeUnit.MILLISECONDS))
      {
        // We can't rule out the possibility of a conflict, so return false.
        return false;
@@ -514,7 +532,7 @@
    }
    finally
    {
      cacheLock.unlock();
      cacheWriteLock.unlock();
    }
  }

@@ -530,7 +548,7 @@
    // FIXME -- An alternate approach could be to block for a maximum length of
    // time and then if it fails then put it in a queue for processing by some
    // other thread before it releases the lock.
    cacheLock.lock();
    cacheWriteLock.lock();


    // At this point, it is absolutely critical that we always release the lock
@@ -575,7 +593,7 @@
    }
    finally
    {
      cacheLock.unlock();
      cacheWriteLock.unlock();
    }
  }

@@ -588,7 +606,7 @@
  {
    // Acquire a lock on the cache.  We should not return until the cache has
    // been cleared, so we will block until we can obtain the lock.
    cacheLock.lock();
    cacheWriteLock.lock();


    // At this point, it is absolutely critical that we always release the lock
@@ -612,7 +630,7 @@
    }
    finally
    {
      cacheLock.unlock();
      cacheWriteLock.unlock();
    }
  }

@@ -625,7 +643,7 @@
  {
    // Acquire a lock on the cache.  We should not return until the cache has
    // been cleared, so we will block until we can obtain the lock.
    cacheLock.lock();
    cacheWriteLock.lock();


    // At this point, it is absolutely critical that we always release the lock
@@ -656,9 +674,9 @@

        if ((entriesDeleted % 1000)  == 0)
        {
          cacheLock.unlock();
          cacheWriteLock.unlock();
          Thread.currentThread().yield();
          cacheLock.lock();
          cacheWriteLock.lock();
        }
      }
    }
@@ -673,7 +691,7 @@
    }
    finally
    {
      cacheLock.unlock();
      cacheWriteLock.unlock();
    }
  }

@@ -695,7 +713,7 @@

    // Acquire a lock on the cache.  We should not return until the cache has
    // been cleared, so we will block until we can obtain the lock.
    cacheLock.lock();
    cacheWriteLock.lock();


    // At this point, it is absolutely critical that we always release the lock
@@ -715,7 +733,7 @@
    }
    finally
    {
      cacheLock.unlock();
      cacheWriteLock.unlock();
    }
  }

@@ -762,9 +780,9 @@
      entriesExamined++;
      if ((entriesExamined % 1000) == 0)
      {
        cacheLock.unlock();
        cacheWriteLock.unlock();
        Thread.currentThread().yield();
        cacheLock.lock();
        cacheWriteLock.lock();
      }
    }

@@ -798,7 +816,7 @@
  public void handleLowMemory()
  {
    // Grab the lock on the cache and wait until we have it.
    cacheLock.lock();
    cacheWriteLock.lock();


    // At this point, it is absolutely critical that we always release the lock
@@ -843,7 +861,7 @@
    }
    finally
    {
      cacheLock.unlock();
      cacheWriteLock.unlock();
    }
  }

@@ -1048,7 +1066,7 @@

    // Grab cache lock to prevent any modifications
    // to the cache maps until a snapshot is taken.
    cacheLock.lock();
    cacheWriteLock.lock();
    try {
      // Examining the real maps will hold the lock and can cause map
      // modifications in case of any access order maps, make copies
@@ -1056,7 +1074,7 @@
      dnMapCopy = new LinkedHashMap<DN,CacheEntry>(dnMap);
      idMapCopy = new HashMap<Backend,HashMap<Long,CacheEntry>>(idMap);
    } finally {
      cacheLock.unlock();
      cacheWriteLock.unlock();
    }

    // Check dnMap first.