Index: C:/Java/castor-1/codegen/src/main/java/org/exolab/castor/builder/factory/SourceFactory.java =================================================================== --- C:/Java/castor-1/codegen/src/main/java/org/exolab/castor/builder/factory/SourceFactory.java (revision 7131) +++ C:/Java/castor-1/codegen/src/main/java/org/exolab/castor/builder/factory/SourceFactory.java (working copy) @@ -1138,13 +1138,16 @@ // The hashCode method has no arguments jclass.addMethod(jMethod); - + JSourceCode jsc = jMethod.getSourceCode(); - - jsc.add("int result = 17;"); + if (jclass.getSuperClassQualifiedName() == null) { + jsc.add("int result = 17;"); + } else { + jsc.add("int result = super.hashCode();"); + } jsc.add(""); jsc.add("long tmp;"); - + for (int i = 0; i < fields.length; i++) { JField temp = fields[i]; // If the field is an object the hashCode method is called recursively @@ -1169,9 +1172,11 @@ jsc.add("result = 37 * result + (int)(tmp^(tmp>>>32));"); } } else { - // Calculates hashCode in a recursive manner. - jsc.add("if (" + name + " != null) {"); + // Calculates hashCode in an acyclic recursive manner + jsc.add("if (" + name + " != null"); + jsc.add(" && !org.castor.util.CycleBreaker.startingToCycle(" + name + ")) {"); jsc.add(" result = 37 * result + " + name + ".hashCode();"); + jsc.add(" org.castor.util.CycleBreaker.releaseCycleHandle(" + name + ");"); jsc.add("}"); } } @@ -1222,6 +1227,8 @@ jsc.append(" temp = ("); jsc.append(jclass.getLocalName()); jsc.append(")obj;"); + jsc.add("boolean thcycle;"); + jsc.add("boolean tmcycle;"); } for (int i = 0; i < fields.length; i++) { JField temp = fields[i]; @@ -1249,8 +1256,31 @@ jsc.indent(); jsc.append("return false;"); jsc.unindent(); - jsc.add("else if (!("); - + jsc.add("if (this."); + jsc.append(name); + jsc.append(" != temp."); + jsc.append(name); + jsc.append(") {"); + // This prevents string constants and improper DOM subtree self comparisons + // (where Q(A(B)) and Q'(C(B)) are compared) screwing up cycle detection + jsc.indent(); + jsc.add("thcycle=org.castor.util.CycleBreaker.startingToCycle(this." + name + ");"); + jsc.add("tmcycle=org.castor.util.CycleBreaker.startingToCycle(temp." + name + ");"); + // equivalent objects *will* cycle at the same time + jsc.add("if (thcycle!=tmcycle) {"); + jsc.indent(); + jsc.add("if (!thcycle) { org.castor.util.CycleBreaker.releaseCycleHandle(this." + + name + "); };"); + jsc.add("if (!tmcycle) { org.castor.util.CycleBreaker.releaseCycleHandle(temp." + + name + "); };"); + jsc.add("return false;"); + jsc.unindent(); + jsc.add("}"); // end of unequal cycle point test + jsc.add("if (!thcycle) {"); + + jsc.indent(); + + jsc.add("if (!"); // Special handling for comparing arrays if (temp.getType().isArray()) { jsc.append("java.util.Arrays.equals(this."); @@ -1266,13 +1296,24 @@ jsc.append(")"); } - jsc.append(")) "); + jsc.append(") {"); jsc.indent(); + + jsc.add("org.castor.util.CycleBreaker.releaseCycleHandle(this." + name + ");"); + jsc.add("org.castor.util.CycleBreaker.releaseCycleHandle(temp." + name + ");"); jsc.add("return false;"); jsc.unindent(); + jsc.add("}"); + + jsc.add("org.castor.util.CycleBreaker.releaseCycleHandle(this." + name + ");"); + jsc.add("org.castor.util.CycleBreaker.releaseCycleHandle(temp." + name + ");"); + jsc.unindent(); - jsc.add("}"); //end of != null - jsc.add("else if (temp."); + jsc.add("}"); // end of !thcycle + jsc.unindent(); + jsc.add("}"); // end of this.name != that.name object constant check + jsc.unindent(); + jsc.add("} else if (temp."); jsc.append(name); jsc.append(" != null)"); } Index: C:/Java/castor-1/src/doc/release-notes.xml =================================================================== --- C:/Java/castor-1/src/doc/release-notes.xml (revision 7131) +++ C:/Java/castor-1/src/doc/release-notes.xml (working copy) @@ -82,6 +82,26 @@ + + + Fixed infinite recursion with cyclid IDREFs at hashCode() and equals(). + + + Jim Procter + jimp@compbio.dundee.ac.uk + + + Ralf Joachim + ralf.joachim@syscon.eu + + + Jim Procter + jimp@compbio.dundee.ac.uk + + Enh. + XML + 20070804 + Fixed sealing violation occured when using castor-jdo. Index: C:/Java/castor-1/src/main/java/org/castor/util/CycleBreaker.java =================================================================== --- C:/Java/castor-1/src/main/java/org/castor/util/CycleBreaker.java (revision 0) +++ C:/Java/castor-1/src/main/java/org/castor/util/CycleBreaker.java (revision 0) @@ -0,0 +1,91 @@ +/* + * Copyright 2007 Jim Procter + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.castor.util; + +import java.util.IdentityHashMap; + +/** + *

+ * lightweight mechanism for thread-safe detection of cyclic calls to hashCode or equals in + * objects created by the XML CodeGenerator. + *

+ *

Usage

+ *
  1. + * startingToCycle is called on a particular object prior to recursing on it, and recursion + * should only occur if this call returns false. + *
    2. + *
  2. + * releaseCycleHandle is called after the recursive call returns in order to release the cycle + * lock on the object. + *
    3. + *
+ *

Note : Do not use this cycle breaking mechanism on object comparisons + * where two instances may share the same reference to some third object, such as a String constant. + *

+ * @author Jim Procter + */ +public class CycleBreaker { + /** Hash of threads and objects that we are keeping track of. */ + private static IdentityHashMap threadHash = new IdentityHashMap(); + + /** + * Test to see if we are about to begin cycling on a method call to beingHashed. + * + * @param beingHashed the object to check for a cycle. + * @return true if a cycle is about to occur on this non-null object. + */ + public static boolean startingToCycle(Object beingHashed) { + if (beingHashed==null) { + return false; + } + + Object hthr = threadHash.get(Thread.currentThread()); + if (hthr == null) { + threadHash.put(Thread.currentThread(), hthr = new IdentityHashMap()); + ((IdentityHashMap) hthr).put(beingHashed, beingHashed); + return false; // first call. no cycle detected + } + Object objhandle = ((IdentityHashMap) hthr).get(beingHashed); + if (objhandle == null) { + // this is the default for a hash value currently being computed. + ((IdentityHashMap) hthr).put(beingHashed, beingHashed); + return false; // first call. no cycle detected. + } + return true; + } + + /** + * Called to release Cycling lock for this object at the end of a routine + * where cycles are to be detected. + * + * @param beingHashed the object for which the cycle-lock will be released. + */ + public static void releaseCycleHandle(Object beingHashed) { + if (beingHashed == null) { + return; + } + Object hthr = threadHash.get(Thread.currentThread()); + if (hthr != null) { + if (((IdentityHashMap) hthr).containsKey(beingHashed)) { + ((IdentityHashMap) hthr).remove(beingHashed); + // release any references if we have no more CycleHandles + if (((IdentityHashMap) hthr).size() == 0) { + threadHash.remove(hthr); + } + } + } + } +} \ No newline at end of file