PR: https://github.com/hibernate/hibernate-orm/pull/2825
On Thu, Mar 28, 2019 at 1:49 PM Gail Badner <gbad...@redhat.com> wrote: > Hi Guillaume, > > I've confirmed that my fix gets the WildFly tests to pass. I've created > https://hibernate.atlassian.net/browse/HHH-13343. > > I'm not sure I understand what you are suggesting by "default behavior". > > My proposed fix implements a ClassFileLocator checks if the bytecode > Hibernate is attempting to enhance is the provided bytecode (by default). > > Are you suggesting that ByteBuddy should provide a ClassFileLocator > implementation that does this? > > I'm getting a master PR set up. The tricky bit is to come up with a > Hibernate unit test that can reproduce this. Scott suggested trying to > enhance byte code that has already been enhanced, since the enhanced > bytecode would not be available from the ClassLoader. I'm giving this a > try. > > I will (hopefully) have a PR for master branch shortly. > > Regards, > Gail > > On Thu, Mar 28, 2019 at 10:58 AM Guillaume Smet <guillaume.s...@gmail.com> > wrote: > >> Hi Gail, >> >> Thanks for looking into this. >> >> Your commit is interesting as we had people in Quarkus complaining about >> the fact that it was not possible to chain the ORM enhancer after other >> enhancers as it didn't consider the provided bytes as the "source" to >> consider. >> >> I wonder if what you did (i.e. considering the provided bytes) should be >> the default behavior. >> >> On Thu, Mar 28, 2019 at 7:13 AM Gail Badner <gbad...@redhat.com> wrote: >> >>> Hi Guillaume, >>> >>> Unfortunately, it is not so easy. >>> >>> typeDescription is of type >>> TypePool$Default$WithLazyResolution$LazyTypeDescription, and it doesn't get >>> completely resolved until later. >>> >>> I've pushed a branch [1] that seems to work for MultiplePuTestCase. Here >>> is the commit [2]. >>> >>> I haven't had a chance to try the other tests in Scott's PR. I'll give >>> them a try tomorrow. >>> >>> I did not create a Hibernate issue for this yet because I'm not sure if >>> Hibernate should be working around this issue. >>> >>> It's getting late for me tonight. I'll continue tomorrow. >>> >>> Comments are welcome. >>> >>> Regards, >>> Gail >>> >>> [1] https://github.com/gbadner/hibernate-core/tree/WFLY-11891-5.3 >>> [2] >>> https://github.com/gbadner/hibernate-core/commit/ef1687807def94a4465d6cf2372f9235dc559bd1 >>> >>> On Tue, Mar 26, 2019 at 10:11 AM Gail Badner <gbad...@redhat.com> wrote: >>> >>>> Guillaume, thanks for the suggestion. I'll give it a try... >>>> >>>> On Tue, Mar 26, 2019 at 9:59 AM Guillaume Smet < >>>> guillaume.s...@gmail.com> wrote: >>>> >>>>> I would try changing the start of EnhancerImpl#enhance() to: >>>>> ======= >>>>> public byte[] enhance(String className, byte[] originalBytes) >>>>> throws >>>>> EnhancementException { >>>>> //Classpool#describe does not accept '/' in the description >>>>> name as >>>>> it expects a class name. See HHH-12545 >>>>> final String safeClassName = className.replace( '/', '.' ); >>>>> try { >>>>> final Resolution typeResolution = typePool.describe( >>>>> safeClassName ); >>>>> if ( !typeResolution.isResolved() ) { >>>>> return null; >>>>> } >>>>> >>>>> final TypeDescription typeDescription = >>>>> typeResolution.resolve(); >>>>> ======= >>>>> >>>>> i.e. testing the resolution of the type. >>>>> >>>>> That might fix it. >>>>> >>>>> -- >>>>> Guillaume >>>>> >>>>> On Tue, Mar 26, 2019 at 5:39 PM Scott Marlow <smar...@redhat.com> >>>>> wrote: >>>>> >>>>> > Thinking more about this, I don't think that ByteBuddy should be >>>>> able to >>>>> > do a classloader.getResource() on the class that is being defined >>>>> > (SLSBPersistenceContexts$$$view5.class). It might be correct for the >>>>> > getResource call to return null, until after the class is completely >>>>> > defined. >>>>> > >>>>> > Would it make sense for the above condition (cl.getResource() >>>>> returning >>>>> > null) be detected differently in the callstack [1] and just fall >>>>> through >>>>> > + return to the caller? >>>>> > >>>>> > Scott >>>>> > >>>>> > [1] >>>>> https://gist.github.com/scottmarlow/0e74cd16d7229812261b7c14e452b3cd >>>>> > >>>>> > On 3/26/19 9:53 AM, Scott Marlow wrote: >>>>> > > Hi Tomek, >>>>> > > >>>>> > > I think the pending question now is why ByteBuddy is getting a null >>>>> > > result from the >>>>> > > >>>>> > >>>>> classLoader.getResourceAsStream("org/jboss/as/test/integration/jpa/basic/SLSBPersistenceContexts$$$view5.class") >>>>> > >>>>> > > call. >>>>> > > >>>>> > > We have also seen failures for >>>>> > > org.jboss.as.ejb3.SerializationProxyHackImplementation as well, >>>>> which is >>>>> > > also generated by the EJB container (see exception call stack in >>>>> > > https://issues.jboss.org/browse/WFLY-11891). >>>>> > > >>>>> > > I wonder if this could be an ordering bug where classes generated >>>>> via >>>>> > > JBoss ClassFileWriter are added to the classloader list of classes, >>>>> > > before the actual bytecode is added. >>>>> > > >>>>> > > Scott >>>>> > > >>>>> > > On 3/26/19 9:17 AM, Tomasz Adamski wrote: >>>>> > >> Hi Scott, >>>>> > >> >>>>> > >> Added to my TODO. WIll try to look at it this week. >>>>> > >> >>>>> > >> Regards, >>>>> > >> Tomek >>>>> > >> >>>>> > >> On Mon, Mar 25, 2019 at 5:14 PM Scott Marlow <smar...@redhat.com >>>>> > >> <mailto:smar...@redhat.com>> wrote: >>>>> > >> >>>>> > >> Adding Tomek + Cheng, as they have been working on the >>>>> WildFly EJB >>>>> > >> layer >>>>> > >> recently, which seems to use >>>>> > >> https://github.com/jbossas/jboss-classfilewriter for >>>>> generating >>>>> > >> the EJB >>>>> > >> stub classes like >>>>> > >> >>>>> > >> >>>>> > >>>>> org/jboss/as/test/integration/jpa/basic/SLSBPersistenceContexts$$$view5.class. >>>>> > >>>>> > >> >>>>> > >> >>>>> > >> Perhaps Tomek or Cheng, can answer whether WildFly (EJB >>>>> layer) or >>>>> > >> ByteBuddy should change to handle dynamically generated >>>>> classes >>>>> > >> differently. >>>>> > >> >>>>> > >> In other words, should ByteBuddy respond differently to >>>>> > >> >>>>> > >> >>>>> > >>>>> classLoader.getResourceAsStream("org/jboss/as/test/integration/jpa/basic/SLSBPersistenceContexts$$$view5.class") >>>>> > >>>>> > >> >>>>> > >> >>>>> > >> returning null or should the jboss-classfilewriter project >>>>> somehow >>>>> > >> avoid >>>>> > >> this bug. >>>>> > >> >>>>> > >> Scott >>>>> > >> >>>>> > >> On 3/22/19 2:54 AM, Gail Badner wrote: >>>>> > >> > Scott added bytecode enhancement to some WildFly tests for >>>>> > >> WFLY-11891 [1], >>>>> > >> > which are failing. >>>>> > >> > >>>>> > >> > Here is Scott's PR with the updated tests: [2] >>>>> > >> > >>>>> > >> > When I stepped into >>>>> > >> > >>>>> > >> >>>>> > >> >>>>> > >>>>> org.jboss.as.test.integration.jpa.basic.multiplepersistenceunittest.MultiplePuTestCase, >>>>> > >>>>> > >> >>>>> > >> > I can see that they are failing in ByteBuddy code. >>>>> > >> > >>>>> > >> > I see that: >>>>> > >> > >>>>> > >> > - enhancement of >>>>> > >> > >>>>> > >> org.jboss.as.test.integration.jpa.basic.SLSBPersistenceContexts >>>>> gets >>>>> > >> > skipped several times in a row; >>>>> > >> > - enhancement of some other classes get skipped; >>>>> > >> > - before trying to enhance >>>>> > >> > >>>>> > >> >>>>> > >>>>> org.jboss.as.test.integration.jpa.basic.SLSBPersistenceContexts$$$view5, >>>>> > >> an >>>>> > >> > exception is thrown. >>>>> > >> > >>>>> > >> > Unfortunately, I'm having trouble getting a good >>>>> stacktrace to >>>>> > >> show what >>>>> > >> > happens in ByteBuddy code. >>>>> > >> > >>>>> > >> > Here is what I'm seeing: >>>>> > >> > >>>>> > >> > >>>>> > >> >>>>> > >> >>>>> > >>>>> net.bytebuddy.pool.TypePool$Default.doDescribe("org.jboss.as.test.integration.jpa.basic.SLSBPersistenceContexts$$$view5") >>>>> > >>>>> > >> >>>>> > >> > /* class name differs from run to run */ >>>>> > >> > >>>>> > >> > >>>>> > >> > calls >>>>> > >> net.bytebuddy.dynamic.ClassFileLocator$ForClassLoader.locate( " >>>>> > >> > >>>>> > >> >>>>> > >> >>>>> > >>>>> org.jboss.as.test.integration.jpa.basic.SLSBPersistenceContexts$$$view5") >>>>> > >> > >>>>> > >> > calls >>>>> > >> net.bytebuddy.dynamic.ClassFileLocator.ForClassLoader.locate( >>>>> > >> > classLoader, >>>>> > >> >>>>> > >> >>>>> > >>>>> "org.jboss.as.test.integration.jpa.basic.SLSBPersistenceContexts$$$view5" >>>>> > >> > ) >>>>> > >> > >>>>> > >> > calls >>>>> > >> > >>>>> > >> >>>>> > >> >>>>> > >>>>> classLoader.getResourceAsStream("org/jboss/as/test/integration/jpa/basic/SLSBPersistenceContexts$$$view5.class"), >>>>> > >>>>> > >> >>>>> > >> > which returns null; >>>>> > >> > >>>>> > >> > (I don't actually see a class file with this name) >>>>> > >> > >>>>> > >> > >>>>> > >> > returns new TypePool.Resolution.Illegal( >>>>> > >> > >>>>> > >> > >>>>> > >> >>>>> > >> >>>>> > >>>>> "org/jboss/as/test/integration/jpa/basic/SLSBPersistenceContexts$$$view5.class" >>>>> > >>>>> > >> >>>>> > >> > >>>>> > >> > ) >>>>> > >> > >>>>> > >> > returns TypePool.Resolution.Illegal >>>>> > >> > >>>>> > >> > >>>>> > >> > Ultimately, TypePool.Resolution.Illegal#resolve( ) throws >>>>> > >> > IllegalStateException, because the type description cannot >>>>> be >>>>> > >> resolved. >>>>> > >> > >>>>> > >> > I'm not sure if the problem is in WildFly, Hibernate, or >>>>> > >> ByteBuddy. >>>>> > >> > >>>>> > >> > To build WildFly: >>>>> > >> > ./build.sh clean install -DskipTests=true >>>>> > >> > >>>>> > >> > To run the test: >>>>> > >> > cd testsuite/integration/basic >>>>> > >> > mvn install >>>>> > >> > >>>>> > >> >>>>> > >> >>>>> > >>>>> -Dtest=org/jboss/as/test/integration/jpa/basic/multiplepersistenceunittest/MultiplePuTestCase >>>>> > >>>>> > >> >>>>> > >> > >>>>> > >> > Help would be very much appreciated. >>>>> > >> > >>>>> > >> > Thanks, >>>>> > >> > Gail >>>>> > >> > >>>>> > >> > [1] https://issues.jboss.org/browse/WFLY-11891 >>>>> > >> > [2] https://github.com/wildfly/wildfly/pull/12180 >>>>> > >> > _______________________________________________ >>>>> > >> > hibernate-dev mailing list >>>>> > >> > hibernate-dev@lists.jboss.org >>>>> > >> <mailto:hibernate-dev@lists.jboss.org> >>>>> > >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>>> > >> > >>>>> > >> >>>>> > >> >>>>> > >> >>>>> > >> -- >>>>> > >> Regards, >>>>> > >> Tomek >>>>> > _______________________________________________ >>>>> > hibernate-dev mailing list >>>>> > hibernate-dev@lists.jboss.org >>>>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>>> _______________________________________________ >>>>> hibernate-dev mailing list >>>>> hibernate-dev@lists.jboss.org >>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev >>>>> >>>> _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev