HHH-7747 - Fixes CNFE regression for runtime field level class

enhancing. Also removes the usage of ClassPool.getDefault() and
creates a new ClassPool on each usage.

HHH-7747 - Add the entity class to the ClassPool to support modular
classloading like OSGI. Remove unused import in FieldTransformer.

HHH-7747 - Enhanced test to ensure that class enhancement sees all
classes of an entity. Added test to ensure that StackMapTables are
non-null for Javassist added methods.

hibernate-core/src/main/java/org/hibernate/bytecode/internal/javassist/FieldTransformer.java

@@ -32,6 +32,7 @@ import java.util.Iterator;
 import java.util.List;
 
 import javassist.CannotCompileException;
+import javassist.ClassPool;
 import javassist.bytecode.AccessFlag;
 import javassist.bytecode.BadBytecode;
 import javassist.bytecode.Bytecode;
@@ -43,6 +44,8 @@ import javassist.bytecode.Descriptor;
 import javassist.bytecode.FieldInfo;
 import javassist.bytecode.MethodInfo;
 import javassist.bytecode.Opcode;
+import javassist.bytecode.StackMapTable;
+import javassist.bytecode.stackmap.MapMaker;
 
 /**
  * The thing that handles actual class enhancement in regards to
@@ -50,6 +53,7 @@ import javassist.bytecode.Opcode;
  *
  * @author Muga Nishizawa
  * @author Steve Ebersole
+ * @author Dustin Schultz
  */
 public class FieldTransformer {
 
@@ -80,12 +84,19 @@ public class FieldTransformer {
 
 	private FieldFilter filter;
 	
+	private ClassPool classPool;
+
 	public FieldTransformer() {
-		this(null);
+		this(null, null);
 	}
 
-	public FieldTransformer(FieldFilter f) {
+	public FieldTransformer(FieldFilter f, ClassPool c) {
 		filter = f;
+		classPool = c;
+	}
+	
+	public void setClassPool(ClassPool c) {
+		classPool = c;
 	}
 
 	public void setFieldFilter(FieldFilter f) {
@@ -130,7 +141,7 @@ public class FieldTransformer {
 	}
 
 	private void addGetFieldHandlerMethod(ClassFile classfile)
-			throws CannotCompileException {
+			throws CannotCompileException, BadBytecode {
 		ConstPool cp = classfile.getConstPool();
 		int this_class_index = cp.getThisClassInfo();
 		MethodInfo minfo = new MethodInfo(cp, GETFIELDHANDLER_METHOD_NAME,
@@ -148,11 +159,13 @@ public class FieldTransformer {
 		code.addOpcode(Opcode.ARETURN);
 		minfo.setCodeAttribute(code.toCodeAttribute());
 		minfo.setAccessFlags(AccessFlag.PUBLIC);
+		StackMapTable smt = MapMaker.make(classPool, minfo);
+		minfo.getCodeAttribute().setAttribute(smt);
 		classfile.addMethod(minfo);
 	}
 
 	private void addSetFieldHandlerMethod(ClassFile classfile)
-			throws CannotCompileException {
+			throws CannotCompileException, BadBytecode {
 		ConstPool cp = classfile.getConstPool();
 		int this_class_index = cp.getThisClassInfo();
 		MethodInfo minfo = new MethodInfo(cp, SETFIELDHANDLER_METHOD_NAME,
@@ -172,6 +185,8 @@ public class FieldTransformer {
 		code.addOpcode(Opcode.RETURN);
 		minfo.setCodeAttribute(code.toCodeAttribute());
 		minfo.setAccessFlags(AccessFlag.PUBLIC);
+		StackMapTable smt = MapMaker.make(classPool, minfo);
+		minfo.getCodeAttribute().setAttribute(smt);
 		classfile.addMethod(minfo);
 	}
 
@@ -185,7 +200,7 @@ public class FieldTransformer {
 	}
 
 	private void addReadWriteMethods(ClassFile classfile)
-			throws CannotCompileException {
+			throws CannotCompileException, BadBytecode {
 		List fields = classfile.getFields();
 		for (Iterator field_iter = fields.iterator(); field_iter.hasNext();) {
 			FieldInfo finfo = (FieldInfo) field_iter.next();
@@ -205,7 +220,7 @@ public class FieldTransformer {
 	}
 
 	private void addReadMethod(ClassFile classfile, FieldInfo finfo)
-			throws CannotCompileException {
+			throws CannotCompileException, BadBytecode {
 		ConstPool cp = classfile.getConstPool();
 		int this_class_index = cp.getThisClassInfo();
 		String desc = "()" + finfo.getDescriptor();
@@ -254,11 +269,13 @@ public class FieldTransformer {
 
 		minfo.setCodeAttribute(code.toCodeAttribute());
 		minfo.setAccessFlags(AccessFlag.PUBLIC);
+		StackMapTable smt = MapMaker.make(classPool, minfo);
+		minfo.getCodeAttribute().setAttribute(smt);
 		classfile.addMethod(minfo);
 	}
 
 	private void addWriteMethod(ClassFile classfile, FieldInfo finfo)
-			throws CannotCompileException {
+			throws CannotCompileException, BadBytecode {
 		ConstPool cp = classfile.getConstPool();
 		int this_class_index = cp.getThisClassInfo();
 		String desc = "(" + finfo.getDescriptor() + ")V";
@@ -320,11 +337,13 @@ public class FieldTransformer {
 
 		minfo.setCodeAttribute(code.toCodeAttribute());
 		minfo.setAccessFlags(AccessFlag.PUBLIC);
+		StackMapTable smt = MapMaker.make(classPool, minfo);
+		minfo.getCodeAttribute().setAttribute(smt);
 		classfile.addMethod(minfo);
 	}
 
 	private void transformInvokevirtualsIntoPutAndGetfields(ClassFile classfile)
-			throws CannotCompileException {
+			throws CannotCompileException, BadBytecode {
 		List methods = classfile.getMethods();
 		for (Iterator method_iter = methods.iterator(); method_iter.hasNext();) {
 			MethodInfo minfo = (MethodInfo) method_iter.next();
@@ -341,15 +360,13 @@ public class FieldTransformer {
 			}
 			CodeIterator iter = codeAttr.iterator();
 			while (iter.hasNext()) {
-				try {
 				int pos = iter.next();
 				pos = transformInvokevirtualsIntoGetfields(classfile, iter, pos);
 				pos = transformInvokevirtualsIntoPutfields(classfile, iter, pos);
-				} catch ( BadBytecode e ){
-					throw new CannotCompileException( e );
 			}
 			
-			}
+			StackMapTable smt = MapMaker.make(classPool, minfo);
+			minfo.getCodeAttribute().setAttribute(smt);
 		}
 	}
 

hibernate-core/src/main/java/org/hibernate/bytecode/internal/javassist/JavassistClassTransformer.java

@@ -30,6 +30,8 @@ import java.io.DataOutputStream;
 import java.io.IOException;
 import java.security.ProtectionDomain;
 
+import javassist.ClassClassPath;
+import javassist.ClassPool;
 import javassist.bytecode.ClassFile;
 import org.jboss.logging.Logger;
 
@@ -44,6 +46,7 @@ import org.hibernate.internal.CoreMessageLogger;
  *
  * @author Emmanuel Bernard
  * @author Steve Ebersole
+ * @author Dustin Schultz
  */
 public class JavassistClassTransformer extends AbstractClassTransformerImpl {
 
@@ -70,7 +73,17 @@ public class JavassistClassTransformer extends AbstractClassTransformerImpl {
 			LOG.unableToBuildEnhancementMetamodel( className );
 			return classfileBuffer;
 		}
-		FieldTransformer transformer = getFieldTransformer( classfile );
+		// This is the same as ClassPool.getDefault() but ensures a new ClassPool per
+		ClassPool cp = new ClassPool();
+		cp.appendSystemPath();
+		cp.appendClassPath(new ClassClassPath(this.getClass()));
+		cp.appendClassPath(new ClassClassPath(classfile.getClass()));
+		try {
+			cp.makeClassIfNew(new ByteArrayInputStream(classfileBuffer));
+		} catch (IOException e) {
+			throw new RuntimeException(e.getMessage(), e);
+		}
+		FieldTransformer transformer = getFieldTransformer( classfile, cp );
 		if ( transformer != null ) {
 			LOG.debugf( "Enhancing %s", className );
 			DataOutputStream out = null;
@@ -97,7 +110,7 @@ public class JavassistClassTransformer extends AbstractClassTransformerImpl {
 		return classfileBuffer;
 	}
 
-	protected FieldTransformer getFieldTransformer(final ClassFile classfile) {
+	protected FieldTransformer getFieldTransformer(final ClassFile classfile, final ClassPool classPool) {
 		if ( alreadyInstrumented( classfile ) ) {
 			return null;
 		}
@@ -118,7 +131,7 @@ public class JavassistClassTransformer extends AbstractClassTransformerImpl {
 					public boolean handleWriteAccess(String fieldOwnerClassName, String fieldName) {
 						return fieldFilter.shouldTransformFieldAccess( classfile.getName(), fieldOwnerClassName, fieldName );
 					}
-				}
+				}, classPool
 		);
 	}
 

hibernate-entitymanager/src/test/java/org/hibernate/ejb/test/instrument/InstrumentedClassLoader.java

@@ -10,6 +10,7 @@ import org.hibernate.ejb.instrument.InterceptFieldClassFileTransformer;
 
 /**
  * @author Emmanuel Bernard
+ * @author Dustin Schultz
  */
 public class InstrumentedClassLoader extends ClassLoader {
 	private List<String> entities;
@@ -20,9 +21,28 @@ public class InstrumentedClassLoader extends ClassLoader {
 
 	@Override
 	public Class<?> loadClass(String name) throws ClassNotFoundException {
-		if ( name != null && name.startsWith( "java.lang." ) ) return getParent().loadClass( name );
+		// Do not instrument the following packages
+		if (name != null
+				&& (name.startsWith("java.lang.") || 
+					name.startsWith("java.util.")))
+			return getParent().loadClass(name);
 		Class c = findLoadedClass( name );
 		if ( c != null ) return c;
+
+		byte[] transformed = loadClassBytes(name);
+		
+		return defineClass( name, transformed, 0, transformed.length );
+	}
+	
+	/**
+	 * Specialized {@link ClassLoader#loadClass(String)} that returns the class
+	 * as a byte array.
+	 * 
+	 * @param name
+	 * @return
+	 * @throws ClassNotFoundException
+	 */
+	public byte[] loadClassBytes(String name) throws ClassNotFoundException {
 		InputStream is = this.getResourceAsStream( name.replace( ".", "/" ) + ".class" );
 		if ( is == null ) throw new ClassNotFoundException( name );
 		byte[] buffer = new byte[409600];
@@ -67,7 +87,7 @@ public class InstrumentedClassLoader extends ClassLoader {
 			throw new ClassNotFoundException( name + " not found", e );
 		}
 		
-		return defineClass( name, transformed, 0, transformed.length );
+		return transformed;
 	}
 
 	public void setEntities(List<String> entities) {

hibernate-entitymanager/src/test/java/org/hibernate/ejb/test/instrument/InterceptFieldClassFileTransformerTest.java

@@ -1,18 +1,42 @@
 //$Id$
 package org.hibernate.ejb.test.instrument;
 
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.List;
 
+import javassist.ClassPool;
+import javassist.CtClass;
+import javassist.CtMethod;
+import javassist.bytecode.AttributeInfo;
+import javassist.bytecode.StackMapTable;
+
+import org.hibernate.testing.TestForIssue;
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.Test;
 
 /**
  * @author Emmanuel Bernard
  * @author Hardy Ferentschik
+ * @author Dustin Schultz
  */
 public class InterceptFieldClassFileTransformerTest {
+	
+	private List<String> entities = new ArrayList<String>();
+	private InstrumentedClassLoader loader = null;
+	
+	@Before
+	public void setup() {
+		entities.add( "org.hibernate.ejb.test.instrument.Simple" );
+		// use custom class loader which enhances the class
+		InstrumentedClassLoader cl = new InstrumentedClassLoader( Thread.currentThread().getContextClassLoader() );
+		cl.setEntities( entities );
+		this.loader = cl;
+	}
+	
 	/**
 	 * Tests that class file enhancement works.
 	 * 
@@ -20,9 +44,6 @@ public class InterceptFieldClassFileTransformerTest {
 	 */
     @Test
 	public void testEnhancement() throws Exception {
-		List<String> entities = new ArrayList<String>();
-		entities.add( "org.hibernate.ejb.test.instrument.Simple" );
-
 		// sanity check that the class is unmodified and does not contain getFieldHandler()
 		try {
 			org.hibernate.ejb.test.instrument.Simple.class.getDeclaredMethod( "getFieldHandler" );
@@ -31,14 +52,42 @@ public class InterceptFieldClassFileTransformerTest {
 			// success
 		}
 		
-		// use custom class loader which enhances the class
-		InstrumentedClassLoader cl = new InstrumentedClassLoader( Thread.currentThread().getContextClassLoader() );
-		cl.setEntities( entities );
-		Class clazz = cl.loadClass( entities.get( 0 ) );
+		Class clazz = loader.loadClass( entities.get( 0 ) );
 		
 		// javassist is our default byte code enhancer. Enhancing will eg add the method getFieldHandler()
 		// see org.hibernate.bytecode.internal.javassist.FieldTransformer
 		Method method = clazz.getDeclaredMethod( "getFieldHandler" );
 		Assert.assertNotNull( method );
 	}
+    
+	/**
+	 * Tests that methods that were enhanced by javassist have
+	 * StackMapTables for java verification. Without these,
+	 * java.lang.VerifyError's occur in JDK7.
+	 * 
+	 * @throws ClassNotFoundException
+	 * @throws InstantiationException
+	 * @throws IllegalAccessException
+	 * @throws IOException
+	 */
+	@Test
+	@TestForIssue(jiraKey = "HHH-7747")
+	public void testStackMapTableEnhancment() throws ClassNotFoundException,
+			InstantiationException, IllegalAccessException, IOException {
+		byte[] classBytes = loader.loadClassBytes(entities.get(0));
+		ClassPool classPool = new ClassPool();
+		CtClass ctClass = classPool.makeClass(new ByteArrayInputStream(
+				classBytes));
+		for (CtMethod ctMethod : ctClass.getMethods()) {
+			//Only check methods that were added by javassist
+			if (ctMethod.getName().startsWith("$javassist_")) {
+				AttributeInfo attributeInfo = ctMethod
+						.getMethodInfo().getCodeAttribute()
+						.getAttribute(StackMapTable.tag);
+				Assert.assertNotNull(attributeInfo);
+				StackMapTable smt = (StackMapTable)attributeInfo;
+				Assert.assertNotNull(smt.get());
+			}
+		}
+	}
 }

hibernate-entitymanager/src/test/java/org/hibernate/ejb/test/instrument/Simple.java

@@ -1,13 +1,24 @@
 //$Id$
 package org.hibernate.ejb.test.instrument;
 
+import java.util.Collection;
+
+import org.hibernate.ejb.instrument.InterceptFieldClassFileTransformer;
+
 
 /**
+ * A simple entity to be enhanced by the {@link InterceptFieldClassFileTransformer}
+ * 
  * @author Emmanuel Bernard
+ * @author Dustin Schultz
  */
 public class Simple {
 	private String name;
 	
+	// Have an additional attribute that will ensure that the enhanced classes
+	// will see all class attributes of an entity without CNFEs
+	private Collection<SimpleRelation> relations;
+
 	public String getName() {
 		return name;
 	}
@@ -15,4 +26,12 @@ public class Simple {
 	public void setName(String name) {
 		this.name = name;
 	}
+
+	public Collection<SimpleRelation> getRelations() {
+		return relations;
+	}
+
+	public void setRelations(Collection<SimpleRelation> relations) {
+		this.relations = relations;
+	}
 }

hibernate-entitymanager/src/test/java/org/hibernate/ejb/test/instrument/SimpleRelation.java

@@ -0,0 +1,21 @@
+package org.hibernate.ejb.test.instrument;
+
+/**
+ * A simple entity relation used by {@link Simple} to ensure that enhanced
+ * classes load all classes.
+ * 
+ * @author Dustin Schultz
+ */
+public class SimpleRelation {
+
+	private String blah;
+
+	public String getBlah() {
+		return blah;
+	}
+
+	public void setBlah(String blah) {
+		this.blah = blah;
+	}
+
+}