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.
This commit is contained in:
Dustin Schultz 2012-12-03 15:35:09 -07:00 committed by Scott Marlow
parent 8463057b85
commit 58978a486f
6 changed files with 169 additions and 30 deletions

View File

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

View File

@ -30,6 +30,8 @@ import java.io.DataOutputStream;
import java.io.IOException; import java.io.IOException;
import java.security.ProtectionDomain; import java.security.ProtectionDomain;
import javassist.ClassClassPath;
import javassist.ClassPool;
import javassist.bytecode.ClassFile; import javassist.bytecode.ClassFile;
import org.jboss.logging.Logger; import org.jboss.logging.Logger;
@ -44,6 +46,7 @@ import org.hibernate.internal.CoreMessageLogger;
* *
* @author Emmanuel Bernard * @author Emmanuel Bernard
* @author Steve Ebersole * @author Steve Ebersole
* @author Dustin Schultz
*/ */
public class JavassistClassTransformer extends AbstractClassTransformerImpl { public class JavassistClassTransformer extends AbstractClassTransformerImpl {
@ -70,7 +73,17 @@ public class JavassistClassTransformer extends AbstractClassTransformerImpl {
LOG.unableToBuildEnhancementMetamodel( className ); LOG.unableToBuildEnhancementMetamodel( className );
return classfileBuffer; 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 ) { if ( transformer != null ) {
LOG.debugf( "Enhancing %s", className ); LOG.debugf( "Enhancing %s", className );
DataOutputStream out = null; DataOutputStream out = null;
@ -97,7 +110,7 @@ public class JavassistClassTransformer extends AbstractClassTransformerImpl {
return classfileBuffer; return classfileBuffer;
} }
protected FieldTransformer getFieldTransformer(final ClassFile classfile) { protected FieldTransformer getFieldTransformer(final ClassFile classfile, final ClassPool classPool) {
if ( alreadyInstrumented( classfile ) ) { if ( alreadyInstrumented( classfile ) ) {
return null; return null;
} }
@ -118,7 +131,7 @@ public class JavassistClassTransformer extends AbstractClassTransformerImpl {
public boolean handleWriteAccess(String fieldOwnerClassName, String fieldName) { public boolean handleWriteAccess(String fieldOwnerClassName, String fieldName) {
return fieldFilter.shouldTransformFieldAccess( classfile.getName(), fieldOwnerClassName, fieldName ); return fieldFilter.shouldTransformFieldAccess( classfile.getName(), fieldOwnerClassName, fieldName );
} }
} }, classPool
); );
} }

View File

@ -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;
}
}

View File

@ -10,6 +10,7 @@ import org.hibernate.jpa.internal.instrument.InterceptFieldClassFileTransformer;
/** /**
* @author Emmanuel Bernard * @author Emmanuel Bernard
* @author Dustin Schultz
*/ */
public class InstrumentedClassLoader extends ClassLoader { public class InstrumentedClassLoader extends ClassLoader {
private List<String> entities; private List<String> entities;
@ -20,9 +21,28 @@ public class InstrumentedClassLoader extends ClassLoader {
@Override @Override
public Class<?> loadClass(String name) throws ClassNotFoundException { 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 ); Class c = findLoadedClass( name );
if ( c != null ) return c; 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" ); InputStream is = this.getResourceAsStream( name.replace( ".", "/" ) + ".class" );
if ( is == null ) throw new ClassNotFoundException( name ); if ( is == null ) throw new ClassNotFoundException( name );
byte[] buffer = new byte[409600]; byte[] buffer = new byte[409600];
@ -66,8 +86,8 @@ public class InstrumentedClassLoader extends ClassLoader {
catch (IllegalClassFormatException e) { catch (IllegalClassFormatException e) {
throw new ClassNotFoundException( name + " not found", e ); throw new ClassNotFoundException( name + " not found", e );
} }
return defineClass( name, transformed, 0, transformed.length ); return transformed;
} }
public void setEntities(List<String> entities) { public void setEntities(List<String> entities) {

View File

@ -1,18 +1,42 @@
//$Id$ //$Id$
package org.hibernate.jpa.test.instrument; package org.hibernate.jpa.test.instrument;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.lang.reflect.Method; import java.lang.reflect.Method;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; 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.Assert;
import org.junit.Before;
import org.junit.Test; import org.junit.Test;
/** /**
* @author Emmanuel Bernard * @author Emmanuel Bernard
* @author Hardy Ferentschik * @author Hardy Ferentschik
* @author Dustin Schultz
*/ */
public class InterceptFieldClassFileTransformerTest { 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. * Tests that class file enhancement works.
* *
@ -20,9 +44,6 @@ public class InterceptFieldClassFileTransformerTest {
*/ */
@Test @Test
public void testEnhancement() throws Exception { public void testEnhancement() throws Exception {
List<String> entities = new ArrayList<String>();
entities.add( "org.hibernate.jpa.test.instrument.Simple" );
// sanity check that the class is unmodified and does not contain getFieldHandler() // sanity check that the class is unmodified and does not contain getFieldHandler()
try { try {
Simple.class.getDeclaredMethod( "getFieldHandler" ); Simple.class.getDeclaredMethod( "getFieldHandler" );
@ -30,15 +51,43 @@ public class InterceptFieldClassFileTransformerTest {
} catch ( NoSuchMethodException nsme ) { } catch ( NoSuchMethodException nsme ) {
// success // success
} }
// use custom class loader which enhances the class Class clazz = loader.loadClass( entities.get( 0 ) );
InstrumentedClassLoader cl = new InstrumentedClassLoader( Thread.currentThread().getContextClassLoader() );
cl.setEntities( entities );
Class clazz = cl.loadClass( entities.get( 0 ) );
// javassist is our default byte code enhancer. Enhancing will eg add the method getFieldHandler() // javassist is our default byte code enhancer. Enhancing will eg add the method getFieldHandler()
// see org.hibernate.bytecode.internal.javassist.FieldTransformer // see org.hibernate.bytecode.internal.javassist.FieldTransformer
Method method = clazz.getDeclaredMethod( "getFieldHandler" ); Method method = clazz.getDeclaredMethod( "getFieldHandler" );
Assert.assertNotNull( method ); 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());
}
}
}
} }

View File

@ -1,12 +1,23 @@
//$Id$ //$Id$
package org.hibernate.jpa.test.instrument; package org.hibernate.jpa.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 Emmanuel Bernard
* @author Dustin Schultz
*/ */
public class Simple { public class Simple {
private String name; 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() { public String getName() {
return name; return name;
@ -15,4 +26,12 @@ public class Simple {
public void setName(String name) { public void setName(String name) {
this.name = name; this.name = name;
} }
public Collection<SimpleRelation> getRelations() {
return relations;
}
public void setRelations(Collection<SimpleRelation> relations) {
this.relations = relations;
}
} }