You are viewing a plain text version of this content. The canonical link for it is here.
Posted to torque-dev@db.apache.org by dl...@apache.org on 2003/06/20 02:41:18 UTC
cvs commit: db-torque/src/java/org/apache/torque/manager AbstractBaseManager.java MethodResultCache.java
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 <jm...@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;