db-torque-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From d..@apache.org
Subject cvs commit: db-torque/src/java/org/apache/torque/manager AbstractBaseManager.java MethodResultCache.java
Date Fri, 20 Jun 2003 00:41:18 GMT
dlr         2003/06/19 17:41:18

  Modified:    src/java/org/apache/torque/manager AbstractBaseManager.java
                        MethodResultCache.java
  Log:
  Corrected deadly multi-CPU thread deadlock problem discovered by Ed
  Korthof <ed@apache.org> and John McNally <jmcnally@apache.org>.  The
  problem was due to emulation of synchronization using an int counter
  (to improve performance by avoiding Java "synchronized" keyword).
  Post-increment and decrement operators compile to three op codes (with
  Sun's JDK 1.3.1 for Linux), unsafe on a multi-CPU box.
  
  * src/java/org/apache/torque/manager/AbstractBaseManager.java
    lockCache, inGet, cacheGet(), removeInstanceImpl(),
    putInstanceImpl(): Removed use of lockeCache and inGet instance
    fields, replaced by consistent use of Java's "synchronized" keyword
    (on the current instance, "this").
  
    getMethodResultCache(), addCacheListenerImpl(), createSubsetList(),
    readObject(): Added JavaDoc.
  
  * src/java/org/apache/torque/manager/MethodResultCache.java
    lockCache, getImpl(), putImpl(), get(): Removed use of lockeCache
    instance fields, replaced by consistent use of Java's "synchronized"
    keyword (on the current instance, "this").
  
    remove(): Added error messages to several method overloads.
  
  Reviewed by: John McNally
  Issue: PCN19237
  
  Revision  Changes    Path
  1.15      +27 -54    db-torque/src/java/org/apache/torque/manager/AbstractBaseManager.java
  
  Index: AbstractBaseManager.java
  ===================================================================
  RCS file: /home/cvs/db-torque/src/java/org/apache/torque/manager/AbstractBaseManager.java,v
  retrieving revision 1.14
  retrieving revision 1.15
  diff -u -u -r1.14 -r1.15
  --- AbstractBaseManager.java	18 May 2003 12:27:24 -0000	1.14
  +++ AbstractBaseManager.java	20 Jun 2003 00:41:18 -0000	1.15
  @@ -63,6 +63,7 @@
   import java.util.Iterator;
   import java.io.Serializable;
   import java.io.IOException;
  +import java.io.ObjectInputStream;
   
   import org.apache.commons.collections.FastArrayList;
   import org.apache.jcs.JCS;
  @@ -103,8 +104,6 @@
   
       private String region;
   
  -    private boolean lockCache;
  -    private int inGet;
       private boolean isNew = true;
   
       protected Map validFields;
  @@ -219,18 +218,9 @@
           Persistent om = null;
           if (cache != null)
           {
  -            if (lockCache)
  -            {
  -                synchronized (this)
  -                {
  -                    om = (Persistent) cache.get(key);
  -                }
  -            }
  -            else
  +            synchronized (this)
               {
  -                inGet++;
                   om = (Persistent) cache.get(key);
  -                inGet--;
               }
           }
           return om;
  @@ -271,29 +261,19 @@
           Persistent oldOm = null;
           if (cache != null)
           {
  -            synchronized (this)
  +            try
               {
  -                lockCache = true;
  -                try
  +                synchronized (this)
                   {
                       oldOm = (Persistent) cache.get(key);
  -                    while (inGet > 0)
  -                    {
  -                        Thread.yield();
  -                    }
                       cache.remove(key);
                   }
  -                catch (CacheException ce)
  -                {
  -                    lockCache = false;
  -                    throw new TorqueException(
  -                    "Could not remove from cache due to internal JCS error.",
  -                        ce);
  -                }
  -                finally
  -                {
  -                    lockCache = false;
  -                }
  +            }
  +            catch (CacheException ce)
  +            {
  +                throw new TorqueException
  +                    ("Could not remove from cache due to internal JCS error",
  +                     ce);
               }
           }
           return oldOm;
  @@ -334,28 +314,18 @@
           Persistent oldOm = null;
           if (cache != null)
           {
  -            synchronized (this)
  +            try
               {
  -                lockCache = true;
  -                try
  +                synchronized (this)
                   {
                       oldOm = (Persistent) cache.get(key);
  -                    while (inGet > 0)
  -                    {
  -                        Thread.yield();
  -                    }
                       cache.put(key, om);
                   }
  -                catch (CacheException ce)
  -                {
  -                    lockCache = false;
  -                    throw new TorqueException(
  -                        "Could not cache due to internal JCS error.", ce);
  -                }
  -                finally
  -                {
  -                    lockCache = false;
  -                }
  +            }
  +            catch (CacheException ce)
  +            {
  +                throw new TorqueException
  +                    ("Could not cache due to internal JCS error", ce);
               }
           }
           return oldOm;
  @@ -517,6 +487,9 @@
           }
       }
   
  +    /**
  +     * @return The cache instance.
  +     */
       public MethodResultCache getMethodResultCache()
       {
           if (isNew)
  @@ -543,7 +516,7 @@
   
       /**
        * 
  -     * @param listener
  +     * @param listener A new listener for cache events.
        */
       public void addCacheListenerImpl(CacheListener listener)
       {
  @@ -591,7 +564,7 @@
       /**
        * 
        * @param key
  -     * @return
  +     * @return A subset of the list identified by <code>key</code>.
        */
       private synchronized List createSubsetList(String key)
       {
  @@ -664,13 +637,13 @@
       }
   
       /**
  -     * helper methods for the Serializable interface
  +     * Helper methods for the <code>Serializable</code> interface.
        * 
  -     * @param in
  +     * @param in The stream to read a <code>Serializable</code> from.
        * @throws IOException
        * @throws ClassNotFoundException
        */
  -    private void readObject(java.io.ObjectInputStream in)
  +    private void readObject(ObjectInputStream in)
           throws IOException, ClassNotFoundException
       {
           in.defaultReadObject();
  @@ -685,7 +658,7 @@
           catch (Exception e)
           {
               log.error("Cache could not be initialized for region: "
  -                    + region + "after deserialization");
  +                    + region + " after deserialization");
           }
       }
   }
  
  
  
  1.17      +19 -57    db-torque/src/java/org/apache/torque/manager/MethodResultCache.java
  
  Index: MethodResultCache.java
  ===================================================================
  RCS file: /home/cvs/db-torque/src/java/org/apache/torque/manager/MethodResultCache.java,v
  retrieving revision 1.16
  retrieving revision 1.17
  diff -u -u -r1.16 -r1.17
  --- MethodResultCache.java	14 May 2003 19:38:05 -0000	1.16
  +++ MethodResultCache.java	20 Jun 2003 00:41:18 -0000	1.17
  @@ -70,7 +70,8 @@
   import org.apache.torque.TorqueException;
   
   /**
  - * This class provides a cache for convenient storage of method results
  + * This class provides a cache for convenient storage of method
  + * results.
    *
    * @author <a href="mailto:jmcnally@collab.net">John McNally</a>
    * @version $Id$
  @@ -79,7 +80,6 @@
   {
       private ObjectPool pool;
       private GroupCacheAccess jcsCache;
  -    private boolean lockCache;
       private int inGet;
       private Map groups;
   
  @@ -125,18 +125,9 @@
           Object result = null;
           if (jcsCache != null)
           {
  -            if (lockCache)
  -            {
  -                synchronized (this)
  -                {
  -                    result = jcsCache.getFromGroup(key, key.getGroupKey());
  -                }
  -            }
  -            else
  +            synchronized (this)
               {
  -                inGet++;
                   result = jcsCache.getFromGroup(key, key.getGroupKey());
  -                inGet--;
               }
           }
   
  @@ -161,28 +152,18 @@
           Object old = null;
           if (jcsCache != null)
           {
  -            synchronized (this)
  +            try
               {
  -                lockCache = true;
  -                try
  +                synchronized (this)
                   {
                       old = jcsCache.getFromGroup(key, group);
  -                    while (inGet > 0)
  -                    {
  -                        Thread.yield();
  -                    }
                       jcsCache.putInGroup(key, group, value);
                   }
  -                catch (CacheException ce)
  -                {
  -                    lockCache = false;
  -                    throw new TorqueException(
  -                        "Could not cache due to internal JCS error.", ce);
  -                }
  -                finally
  -                {
  -                    lockCache = false;
  -                }
  +            }
  +            catch (CacheException ce)
  +            {
  +                throw new TorqueException
  +                    ("Could not cache due to internal JCS error", ce);
               }
           }
           return old;
  @@ -196,27 +177,8 @@
           {
               synchronized (this)
               {
  -                lockCache = true;
  -                try
  -                {
  -                    old = jcsCache.getFromGroup(key, key.getGroupKey());
  -                    while (inGet > 0)
  -                    {
  -                        Thread.yield();
  -                    }
  -                    jcsCache.remove(key, key.getGroupKey());
  -                }
  -                // jcs does not throw an exception here, might remove this
  -                catch (Exception ce)
  -                {
  -                    lockCache = false;
  -                    throw new TorqueException(
  -                        "Could not cache due to internal JCS error.", ce);
  -                }
  -                finally
  -                {
  -                    lockCache = false;
  -                }
  +                old = jcsCache.getFromGroup(key, key.getGroupKey());
  +                jcsCache.remove(key, key.getGroupKey());
               }
           }
           return old;
  @@ -521,7 +483,7 @@
               }
               catch (Exception e)
               {
  -                log.error("", e);
  +                log.error("Error removing element", e);
               }
           }
           return result;
  @@ -545,12 +507,12 @@
                   catch (Exception e)
                   {
                       log.warn(
  -                        "Nonfatal error.  Could not return key to pool", e);
  +                        "Nonfatal error: Could not return key to pool", e);
                   }
               }
               catch (Exception e)
               {
  -                log.error("", e);
  +                log.error("Error removing element from cache", e);
               }
           }
           return result;
  @@ -580,7 +542,7 @@
               }
               catch (Exception e)
               {
  -                log.error("", e);
  +                log.error("Error removing element from cache", e);
               }
           }
           return result;
  @@ -603,12 +565,12 @@
                   catch (Exception e)
                   {
                       log.warn(
  -                        "Nonfatal error.  Could not return key to pool", e);
  +                        "Nonfatal error: Could not return key to pool", e);
                   }
               }
               catch (Exception e)
               {
  -                log.error("", e);
  +                log.error("Error removing element from cache", e);
               }
           }
           return result;
  
  
  

Mime
View raw message