[hibernate-dev] Javassist enhancement problems with the Hibernate ORM WildFly modules

Sanne Grinovero sanne at hibernate.org
Fri Jan 13 06:20:34 EST 2017 

I finally have a working solution which allows me to move on with the
upgrade of Hibernate Search, yet the solution in the Search testsuite
is essentially sweeping the problem "under the carpet".

I'm taking it as it allows me to move out of stale-mate (and it's not
a Search problem) but opened an ORM issue to eventually fix the real
problem:
 - https://hibernate.atlassian.net/browse/HHH-11394

Scott: your PR 8474 looks great and it would solve the problem. I wish
it was applied long ago!

Steve: understood about the application needing Javassit. Even with PR
8474 the application using ORM would still be "infected" by a possibly
undesired Javassist pushed on its classpath, but at least with this PR
we can choose a version which matches the needs of Hibernate ORM,
and/or eventually remove it when moving to Byte Buddy.
The problem I have with the current JPADependencyProcessor in WildFly
is not just that it pushes Javassist to the client classpath, but it
even pushes the old (wrong) version and gives us no way to prevent
this.

In case you're curious, my workaround for Search is to remove the new
Javassist version; the infamous JPADependencyProcessor will push the
old version and it just happens to work good enough for the tests we
have. I checked by running the ORM testsuite with the same old
Javassit version, and some tests fail so I wouldn't recommend this
solution for any other purpose.

Thanks all!
Sanne


On 12 January 2017 at 14:50, Steve Ebersole <steve at hibernate.org> wrote:
> To be clear... ORM is "expecting the application classpath to include
> Javassist dependency" because it will need it for the *proxies* Hibernate
> returns to be usable once they get them from Hibernate.
>
> It may or may not *need* the application classpath to include Javassist
> dependency if the application is using bytecode generation and *not
> proxies*.  I have not played with this specifically.
>
> On the bytecode generation side I am pretty sure Rafeal said Byte Buddy
> would not be needed on the application's classpath.  No idea about proxies.
> Just in general I think generated proxies are always going to need the
> underlying library available.
>
>
> On Thu, Jan 12, 2017, 8:36 AM Scott Marlow <smarlow at redhat.com> wrote:
>>
>> No, not currently.  We created
>> https://github.com/wildfly/wildfly/pull/8474 to move the exporting of
>> Javassist, to the WildFly static ORM module, so that Hibernate native
>> applications would automatically get the Javassist classes.  That
>> wouldn't of directly addressed your situation but it would of updated
>> the WildFly JPA container (JPADependencyProcessor class) to no longer
>> do the javassist export, so you would of had more control.  I don't
>> think that https://github.com/wildfly/wildfly/pull/8474 is likely to
>> be merged.  Instead, ORM needs to stop expecting the application
>> classpath  to include Javassist dependency and then we can update
>> JPADependencyProcessor to stop exporting Javassist.
>>
>> Perhaps we could discuss this in person soon, to see if we are all on
>> the same page.  My current understanding is that this will be
>> addressed when we switch to Byte Buddy, but the shading (Javassist)
>> idea may also help, as we could stop exporting Javassist from the
>> WildFly JPA container.
>>
>>
>> On Thu, Jan 12, 2017 at 7:36 AM, Sanne Grinovero <sanne at hibernate.org>
>> wrote:
>> > Following Steve's suggestions I thought I had a reasonable and clean
>> > plan, but it's not working in practice.
>> >
>> > Turns out that even after me cleaning up all references to
>> > "org.javassist:main", it's still been forced on the classpath by
>> > hard-coded rules in the JPADependencyProcessor, so conflicting with
>> > the right version:
>> >  -
>> > https://github.com/wildfly/wildfly/blob/6b61a6003f704221f66dcd9f418bcb7af88fb9ab/jpa/subsystem/src/main/java/org/jboss/as/jpa/processor/JPADependencyProcessor.java#L95
>> >
>> > Is there any way I can prevent this injection from happening? Can I
>> > override this JPADependencyProcessor?
>> >
>> > Thanks,
>> > Sanne
>> >
>> > On 11 January 2017 at 19:42, Scott Marlow <smarlow at redhat.com> wrote:
>> >> I'm not really sure but did notice that HikariCP [1] shaded the
>> >> Javassist runtime classes in 2015 and still seems to be shading
>> >> Javassist.
>> >>
>> >> Good point about about the compatibility impact, as build time
>> >> instrumentation with the non-shaded Javassist will produce a Java
>> >> archive that doesn't work with the shaded Javassist.  The application
>> >> will get a CNFE exception unless they add a dependency on the unshaded
>> >> Javassist classes.  I would expect the same problem when we change
>> >> from using Javassist to using Byte Buddy.
>> >>
>> >> [1]
>> >> https://github.com/brettwooldridge/HikariCP/search?utf8=%E2%9C%93&q=javassist
>> >>
>> >> On Wed, Jan 11, 2017 at 11:48 AM, Emmanuel Bernard
>> >> <emmanuel at hibernate.org> wrote:
>> >>> The one small issue is that if the user enhanced the classes at build
>> >>> time (i,e. not via the ClassTransformer), then shading in a newer version of
>> >>> Wildfly means that the same app using the later version will fail unless you
>> >>> rebuild the app.
>> >>> BTW is Javassist Shadable, some of these libs can’t due to string
>> >>> referencing classnames and other fun tricks like that.
>> >>>
>> >>> Emmanuel
>> >>>
>> >>>> On 11 Jan 2017, at 17:35, Scott Marlow <smarlow at redhat.com> wrote:
>> >>>>
>> >>>> On Wed, Jan 11, 2017 at 11:19 AM, Steve Ebersole
>> >>>> <steve at hibernate.org> wrote:
>> >>>>> I think the best option in terms of supporting legacy ORM version
>> >>>>> users
>> >>>>> would be to incorporate a change in those branches to shade/shadow
>> >>>>> Javassist
>> >>>>> into ORM specific packages (in hibernate-core).
>> >>>>
>> >>>> +1
>> >>>>
>> >>>>>
>> >>>>> As I understand it, it is always ORM that does the enhancement,
>> >>>>> right Scott?
>> >>>>
>> >>>> Yes, ORM always does the enhancement.
>> >>>>
>> >>>>>
>> >>>>> On Wed, Jan 11, 2017 at 10:12 AM Scott Marlow <smarlow at redhat.com>
>> >>>>> wrote:
>> >>>>>>
>> >>>>>> Ahh, I was confused then, your talking about the WildFly ORM static
>> >>>>>> module definition [1], which is not controlled by the JPA container
>> >>>>>> or
>> >>>>>> JipiJapa.
>> >>>>>>
>> >>>>>>>>
>> >>>>>>>> We talked about this not long ago and possible solutions, the
>> >>>>>>>> only
>> >>>>>>>> agreed on solution was to eliminate the ORM requirement for
>> >>>>>>>> Javassist
>> >>>>>>>> classes to be on the application classpath, by switching to
>> >>>>>>>> ByteBuddy.
>> >>>>>>>
>> >>>>>>> Like I replied to Gunnar, that's a different problem. Sorry all
>> >>>>>>> for
>> >>>>>>> the confusion!
>> >>>>>>>
>> >>>>>>> In this case it's Hibernate ORM which is being fed two different
>> >>>>>>> versions of Javassist simultaneously; clearly that's our fault.
>> >>>>>>> The
>> >>>>>>> application classpath is not affected.
>> >>>>>>>
>> >>>>>>>> Could you could use one of the previously suggested solutions in
>> >>>>>>>> your
>> >>>>>>>> testing?  For example, your application could use a shaded
>> >>>>>>>> Javassist
>> >>>>>>>> jar, that doesn't interfere with the ORM Javassist.
>> >>>>>>>
>> >>>>>>> I'm not trying to use Javassist. If only the flags to disable it
>> >>>>>>> would
>> >>>>>>> work I'd be happy to disable it ;-)
>> >>>>>>
>> >>>>>> There are no flags for controlling [1], if you want control, just
>> >>>>>> fork
>> >>>>>> WildFly and make occasional changes to the static module
>> >>>>>> definitions
>> >>>>>> that meet your testing changes.  Just keep the changes as minimal
>> >>>>>> as
>> >>>>>> possible, so it is easy to sync up the (test purposes) fork.  The
>> >>>>>> painful part though might be trying to push your fork to maven, so
>> >>>>>> perhaps this is a bad idea, but still wanted to mention it, in case
>> >>>>>> it
>> >>>>>> could help.
>> >>>>>>
>> >>>>>>>>
>> >>>>>>>> Why don't you contribute code changes instead to WildFly?  I did
>> >>>>>>>> create a pull request for
>> >>>>>>>> https://issues.jboss.org/browse/WFLY-5773
>> >>>>>>>> that is still pending, to remove some unneeded dependencies.
>> >>>>>>>> This
>> >>>>>>>> won't get merged without an EAP jira that goes with it, which I
>> >>>>>>>> don't
>> >>>>>>>> plan to create, since there is no EAP need for the change.  Pull
>> >>>>>>>> request is https://github.com/wildfly/wildfly/pull/9305.
>> >>>>>>>
>> >>>>>>> Thanks! Sure I'd be happy to contribute these to WildFly, but
>> >>>>>>> knowing
>> >>>>>>> which dependency is needed - or MIGHT be needed in certain
>> >>>>>>> configurations - requires in depth knowledge of the module one
>> >>>>>>> wants
>> >>>>>>> to cleanup.
>> >>>>>>
>> >>>>>> I'm not sure how you could dynamically update the ORM static module
>> >>>>>> definition [1].
>> >>>>>>
>> >>>>>>> I just have the *impression* that some of these dependencies are
>> >>>>>>> no
>> >>>>>>> longer needed, but going back and forth between projects at
>> >>>>>>> different
>> >>>>>>> releases - and supposed to support various other versions -
>> >>>>>>> doesn't
>> >>>>>>> make it easy.
>> >>>>>>
>> >>>>>> ORM definitely still needs the Javassist dependency, but we should
>> >>>>>> drop ASM, as that is not needed, as well as a few others.
>> >>>>>>
>> >>>>>>>
>> >>>>>>> So I suspect that if the adaptor code itself was bundled directly
>> >>>>>>> with
>> >>>>>>> the consuming JPA implementor, this would come more natural? Just
>> >>>>>>> an
>> >>>>>>> idea.
>> >>>>>>
>> >>>>>> I agree, but others didn't when this came up on the JPA expert
>> >>>>>> group
>> >>>>>> mailing list before.
>> >>>>>>
>> >>>>>>>
>> >>>>>>> See the problem is Hibernate Search needs to test with latest ORM
>> >>>>>>> way
>> >>>>>>> more regularly, so I can't wait for PRs to be included in WildFly,
>> >>>>>>> even less so if they are on hold because of even slower EAP
>> >>>>>>> cycles.
>> >>>>>>
>> >>>>>> I agree that you really need control over the static ORM module
>> >>>>>> definitions.  If you don't want to fork WildFly for testing,
>> >>>>>> perhaps
>> >>>>>> you could modify the static orm module definition before starting
>> >>>>>> the
>> >>>>>> app server, for the testing.  Sounds like a pita.
>> >>>>>>
>> >>>>>>>
>> >>>>>>>>
>> >>>>>>>>> - we should hide Javassist & Byte Buddy from being exposed to
>> >>>>>>>>> the
>> >>>>>>>>> application
>> >>>>>>>>
>> >>>>>>>> Yes, I agree.  You said this before but its ORM that requires
>> >>>>>>>> Javassist classes to be on the application classpath.  ORM does
>> >>>>>>>> not
>> >>>>>>>> require the Byte Buddy classes to be on the application
>> >>>>>>>> classpath.
>> >>>>>>>>
>> >>>>>>>>> - make Byte Buddy an option: I guess improve the JipiJapa
>> >>>>>>>>> itegration
>> >>>>>>>>> to support it and move it into its own private module.
>> >>>>>>>>
>> >>>>>>>> No, ORM doesn't require Byte Buddy to be on the application
>> >>>>>>>> classpath,
>> >>>>>>>> which means that Byte Buddy can be what you want, a separate
>> >>>>>>>> private
>> >>>>>>>> ORM module.  This is a feature of WildFly modules, not JipiJapa.
>> >>>>>>>
>> >>>>>>> What I mean is that JipiJapa is currently triggering enhancement
>> >>>>>>> via
>> >>>>>>> Javassist; it's not giving me an option to use Byte Buddy instead.
>> >>>>>>
>> >>>>>> Its more that the WildFly JPA container, allows the persistence
>> >>>>>> provider to register a javax.persistence.spi.ClassTransformer
>> >>>>>> instance, to be called when entity class definitions are loaded, as
>> >>>>>> per the JPA requirements.  JipiJapa doesn't get involved, as there
>> >>>>>> is
>> >>>>>> a standard JPA contract that ORM uses, so JipiJapa couldn't
>> >>>>>> influence
>> >>>>>> use of Byte Buddy or CGLIB or Javassist...
>> >>>>>>
>> >>>>>>>
>> >>>>>>> So since the sources for that are in yet another project, it looks
>> >>>>>>> like I'd need 6 months to finish my ORM upgrade in Search. Luckily
>> >>>>>>> I'll aim for a different solution ;)
>> >>>>>> :(
>> >>>>>>
>> >>>>>> Scott
>> >>>>>>
>> >>>>>> [1]
>> >>>>>>
>> >>>>>> https://github.com/wildfly/wildfly/blob/master/feature-pack/src/main/resources/modules/system/layers/base/org/hibernate/jipijapa-hibernate5/main/module.xml#L47
>> >>>>>> _______________________________________________
>> >>>>>> 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
>> >>>
>> >>
>> >> _______________________________________________
>> >> 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