[hibernate-dev] WildFly tests with ByteBuddy enhancement are failing
Gail Badner
gbadner at redhat.com
Thu Mar 28 17:39:58 EDT 2019
PR: https://github.com/hibernate/hibernate-orm/pull/2825
On Thu, Mar 28, 2019 at 1:49 PM Gail Badner <gbadner at 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.smet at 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 <gbadner at 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 <gbadner at 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.smet at 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 <smarlow at 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 <smarlow at redhat.com
>>>>> > >> <mailto:smarlow at 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 at lists.jboss.org
>>>>> > >> <mailto:hibernate-dev at lists.jboss.org>
>>>>> > >> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>>> > >> >
>>>>> > >>
>>>>> > >>
>>>>> > >>
>>>>> > >> --
>>>>> > >> Regards,
>>>>> > >> Tomek
>>>>> > _______________________________________________
>>>>> > hibernate-dev mailing list
>>>>> > hibernate-dev at lists.jboss.org
>>>>> > https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>>> _______________________________________________
>>>>> hibernate-dev mailing list
>>>>> hibernate-dev at lists.jboss.org
>>>>> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>>>>>
>>>>
More information about the hibernate-dev
mailing list