Skip to content
This repository was archived by the owner on Mar 13, 2025. It is now read-only.

Calling getId() on a proxy causes the proxy to initialize #175

Closed
longwa opened this issue Feb 4, 2020 · 10 comments · Fixed by #624
Closed

Calling getId() on a proxy causes the proxy to initialize #175

longwa opened this issue Feb 4, 2020 · 10 comments · Fixed by #624

Comments

@longwa
Copy link

longwa commented Feb 4, 2020

GORM 7.0.2.RELEASE
Hibernate 5.4.10.Release

Calling getId() or id on a HibernateProxy causes unwrap. For example, a simple integration test like:

    void "calling getId() should not unwrap proxy"() {
        when:
        def proxy = Author.load(authorId)

        then: "load returns a proxy"
        !Hibernate.isInitialized(proxy)

        when:
        proxy.id

        then: "getId should also not unwrap the proxy"
        !Hibernate.isInitialized(proxy)
    }

Will fail in a default Grails 4 application.

This is because the getMetaClass method is called by groovy as part of the method invocation and still cause the unwrap. Previously, GORM had special proxy handling for hibernate's javassist proxies. However, in 5.3, they switched to using ByteBuddy proxies as the default which seem to have broken this behavior.

Forcing hibernate to use javassist again via hibernate.bytecode.provider="javassist" actually restores the correct behavior and the above test case will pass.

I'm not sure what the ramifications for using javassist vs. bytebuddy would be and whether this is a viable workaround.

The better solution might be to implement an extension for Hibernate's byte buddy handler that includes the Groovy friendly logic for ignoring getMetaClass or getStaticMetaClass, similar to what the org.grails.datastore.mapping.proxy.JavassistProxyFactory used to do.

@graemerocher
Copy link
Member

I recommend we configure javassist as default for the moment as it is know to work fine

@longwa
Copy link
Author

longwa commented Feb 4, 2020

We are going to manually switch to javassist and see if it causes any test failures or issues. I'll report back.

Digging a little more, the javassist proxy that was being returned by Hibernate had a metaClass attribute populated already. The byte buddy proxy that's being returned has metaClass = null. I'm not exactly sure how the javassist proxy had a metaClass property already injected but that seems to prevent the initialization.

@longwa
Copy link
Author

longwa commented Feb 14, 2020

Even with the javassist proxy, if you use the property syntax id on a domain object that is a subclass, it still unrolls the proxy. Using getId() does not. This was likely the behavior before Grails 4, but it's worth pointing out as it still breaks the contract.

@longwa
Copy link
Author

longwa commented May 28, 2020

This seems to be manifesting in a much worse way in some cases for us now that we have Grails 4 in production.

In cases where we have a domain which extends another domain, we are seeing weird, random class cast exceptions when the proxy is being unwrapped. I say random, because they are hard to reproduce and will go away usually with a server restart (for awhile). It seems like some kind of internal state changes that causes it to start happening.

java.lang.IllegalArgumentException: java.lang.ClassCastException@5d3d6a4e at 
sun.reflect.GeneratedMethodAccessor1665.invoke(Unknown Source) at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at 
java.lang.reflect.Method.invoke(Method.java:498) at 
org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:101) at 
groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:323) at 
groovy.lang.MetaClassImpl.getProperty(MetaClassImpl.java:1859) at 
groovy.lang.ExpandoMetaClass.getProperty(ExpandoMetaClass.java:1155) at 
groovy.lang.MetaClassImpl.getProperty(MetaClassImpl.java:3797) at 
groovy.lang.ExpandoMetaClass.getProperty(ExpandoMetaClass.java:1167) at 
org.codehaus.groovy.runtime.callsite.PogoMetaClassGetPropertySite.getProperty(PogoMetaClassGetPropertySite.java:50) at 
org.codehaus.groovy.runtime.callsite.AbstractCallSite.callGetProperty(AbstractCallSite.java:298) at 
org.codehaus.groovy.runtime.callsite.AbstractCallSite.callGetPropertySafe(AbstractCallSite.java:416) at 
com.triu.unit.Unit.getSuggestedLocationCode(Unit.groovy:1474) at 
sun.reflect.GeneratedMethodAccessor15535.invoke(Unknown Source) at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at 
java.lang.reflect.Method.invoke(Method.java:498) at 
org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:101) at 
groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:323) at 
groovy.lang.MetaClassImpl.getProperty(MetaClassImpl.java:1859) at 
groovy.lang.ExpandoMetaClass.getProperty(ExpandoMetaClass.java:1155) at 
groovy.lang.MetaClassImpl.getProperty(MetaClassImpl.java:3797) at 
groovy.lang.ExpandoMetaClass.getProperty(ExpandoMetaClass.java:1167) at 
org.codehaus.groovy.runtime.callsite.PogoMetaClassGetPropertySite.getProperty(PogoMetaClassGetPropertySite.java:50) at 
org.codehaus.groovy.runtime.callsite.AbstractCallSite.callGetProperty(AbstractCallSite.java:298) at 
com.triu.operational.edi.AuditGateCommand$_autofill_closure1.doCall(GateEntryController.groovy:431) at

In this case, we have an object:

StorageLocation that extends Location.
Location has an assigned id (not surrogate), which is a String.

The code that triggers this exception has a proxy reference to a StorageLocation instance and is calling .id on it (which shouldn't actually unwrap the proxy, but does due to this ticket). That ends up triggering ClassCastException down in the invoke method for some reason. Usually restarting the server will allow this same thing to work fine for a bit but then it will eventually start failing again.

We have no 2nd level caching on any of these objects. It's very hard to reproduce in any small test environment since it seems to depend on some other factors that we can't nail down.

@longwa
Copy link
Author

longwa commented Jun 9, 2020

To work around this, we keep having to use a method like this to access the Id on anything that could be a proxy:

    static <O extends Serializable> O getEntityId(Object obj) {
        O identifier = proxyHandler.getIdentifier(obj) as O
        identifier ?: obj?.id
    }

But like I mentioned, these are not always reproducible so they pass our test cases and fail in production. We've probably found 1 per week in production since we've gone live for the last month.

@basejump
Copy link
Contributor

@longwa what did you figure out here? This is a big blocker for us as there is a huge performance hit across our code to hydrate/initialize the entities on a simple id check.
Not sure how is this is not affecting more people, or maybe there is a related issue in grails, didn't see a related one in gorm-datastore.

  • also for work around how are you setting hibernate.bytecode.provider="javassist", only way I came up with is hibernate.properties file which was added to grails-app/conf but we are seeing weird instances where its not always picked up across multi-projects, working on isolating that.
  • might want to update the docs to mention that if thats way to change back , we can take that tweak if so

@longwa
Copy link
Author

longwa commented Nov 3, 2020

Pretty sure it works just setting the property via -Dhibernate.bytecode.provider=javassist

@longwa
Copy link
Author

longwa commented Dec 14, 2021

Just to bump this a bit, this is still a problem even in Grails 5 and using Groovy 3.0.9.

To add a bit more urgency, Hibernate is removing support for the Javassist proxy in the next version (according to their warning in the logs) so the workaround will not really be viable.

Plus, in Grails 5, we are seeing now that even the Javassist proxy is unwrapping with getId() again, so the workaround really isn't useful anyway. When using the default ByteBuddy proxy, getMetaClass() is causing the unwrap. Presumably this is b/c Hibernate doesn't really know that the proxy is being used as a GroovyObject.

basejump added a commit to yakworks/gorm-hibernate5 that referenced this issue Sep 24, 2022
… Javassist collions. uses Hibernate helpers.

Mark SimpleHibernateProxyHandler as deprecated.
Add tests to show the proxy problems with groovy
resolves grails#464
resolves grails#175
@basejump
Copy link
Contributor

basejump commented Sep 24, 2022

@longwa @puneetbehl I added a test here that shows the problem.
#624

The recent issue is somewhat related #464.

  • The HibernateProxyHandler was extending a class that still used JavaAssist. Only under a couple circumstances did that cause issues. Fixed HibernateProxyHandler so that it will always use the bytebuddy as it just sticks with using pure hibernate for most of the calls
  • as can be seen in the HibernateProxySpec any call in dynamically compiled code will try and initialize the proxy.
  • When the .getId() or .id call is made in statically compiled code then its fine. the .id gets compiled to getId do they end up being the same thing in that case.
  • any truthy check against the object will initialize it.

I made a POC to override the ByteBuddyInterceptor in gorm-tools and its working. Can be seen here https://github.com/yakworks/gorm-tools/tree/dev/gorm-tools/src/main/groovy/gorm/tools/hibernate/proxy

With passing tests here. https://github.com/yakworks/gorm-tools/blob/dev/gorm-tools/src/test/groovy/gorm/tools/hibernate/HibernateProxySpec.groovy

Going to work on breaking it into it own library so its non invasive and can work with any Groovy/Hibernate use as the problem is not Gorm specific. Will update the branch here with example project once its done.

@basejump
Copy link
Contributor

added the lib here that I think solves the main issues. https://github.com/yakworks/hibernate-groovy.
PR with example is submitted too.

jamesfredley pushed a commit that referenced this issue Feb 19, 2025
* Upgrade to Liquibase 3.10.1 (also liquibase-hibernate5)
     - Fixed tests by extracting changeLog part (Gradle verbose log causes polluted outputCapture)
     - Fixes GroovyChangeLogSpec if default JVM langauge is not en.
     - String comparisons are now done without spaces in tests
     - fixes #95 #100 #159
* Use openjdk8 in Travis
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants