Grails
  1. Grails
  2. GRAILS-5426

HibernatePluginSupport.#addValidationMethods(...) is reason of memory leak

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.1.2
    • Fix Version/s: 1.1.2
    • Component/s: Persistence
    • Labels:
      None

      Description

      The memory leak problem was detected after we patched the Grails 1.1.1 with http://jira.codehaus.org/browse/GRAILS-4580 changes http://github.com/grails/grails/commit/a7af74af3abb85faa75652433b3bb07e257aed43

      But in fact the reason of memory leak is http://jira.codehaus.org/browse/GRAILS-3163 changes diff (added lines 283-290)
      Just memory leak became more obvious with http://jira.codehaus.org/browse/GRAILS-4580 changes.

      So about the problem:

      With http://jira.codehaus.org/browse/GRAILS-4580 changes GroovyAwareJavassistLazyInitializer#getProxy(...) calls HibernatePluginSupport.initializeDomain(persistentClass);

      And HibernatePluginSupport#initializeDomain(persistentClass) calls lazyInit closure on GrailsDomainClass which calls registerDynamicMethods(...)

      HibernatePluginSupport#registerDynamicMethods(...) where addValidationMethods(...) is called where executed the code introduced with http://jira.codehaus.org/browse/GRAILS-3163 changes

      ...
              MetaProperty originalPropertiesProperty = metaClass.getMetaProperty("properties")
              metaClass.setProperties = {Object o ->
                  originalPropertiesProperty.setProperty delegate, o
                  if(delegate.hasErrors()) {
                      GrailsHibernateUtil.setObjectToReadyOnly delegate,sessionFactory
                  }
              }
      ...
      

      Let look at the code above more carefully:
      The originalPropertiesProperty is the MetaBeanProperty for "properties" with getter and setter.
      The metaClass.setProperties =

      {Object o -> ...} construction 'replaces' setter for MetaBeanProperty "properties" with specified closure.

      But in fact metaClass.setProperties = {Object o -> ...}

      also creates new instance of MetaBeanProperty for "properties" with old getter and new setter according to specified closure.

      From other side specified closure uses originalPropertiesProperty inside and it leads to memory leak because this created instance of MetaBeanProperty refers to previous MetaBeanProperty

      It prevents these instances and all referenced instances to be collected by GC

      In case of loading a lot of object with a lot of hibernate proxies it lead to OutOfMemoryError very soon even with big size of allocated heap memory.

      It seems this memory leak was fixed by http://jira.codehaus.org/browse/GRAILS-4843 changes

      in head/master but not in 1.1.x branch.

      How to solve this problem for Grails 1.1.2 ?
      Are http://jira.codehaus.org/browse/GRAILS-3163 changes related to metaClass.setProperties =

      {Object o -> ...}

      really necessary yet in Grails 1.1.x branch?
      Can we just remove

      ...
              MetaProperty originalPropertiesProperty = metaClass.getMetaProperty("properties")
              metaClass.setProperties = {Object o ->
                  originalPropertiesProperty.setProperty delegate, o
                  if(delegate.hasErrors()) {
                      GrailsHibernateUtil.setObjectToReadyOnly delegate,sessionFactory
                  }
              }
      ...
      

      as it was done by http://github.com/grails/grails/commit/3df4414877e7859aa269db22ca23e76b4a8d1140 ?

      1. stackTrace.txt
        4 kB
        Goroschenya Eugene
      1. memoryLeak.PNG
        94 kB
      2. memoryLeakReason.PNG
        179 kB

        Activity

        Hide
        Goroschenya Eugene added a comment -

        Attached memoryLeak.PNG shows that objects are not collected by CG after task was done completely.
        Attached memoryLeakReason.PNG shows why objects are not collected by CG.
        Attached stackTrace.txt can be useful to understand what happens on

                metaClass.setProperties = {Object o ->
                    originalPropertiesProperty.setProperty delegate, o
                    if(delegate.hasErrors()) {
                        GrailsHibernateUtil.setObjectToReadyOnly delegate,sessionFactory
                    }
                }
        

        execution

        Show
        Goroschenya Eugene added a comment - Attached memoryLeak.PNG shows that objects are not collected by CG after task was done completely. Attached memoryLeakReason.PNG shows why objects are not collected by CG. Attached stackTrace.txt can be useful to understand what happens on metaClass.setProperties = { Object o -> originalPropertiesProperty.setProperty delegate, o if (delegate.hasErrors()) { GrailsHibernateUtil.setObjectToReadyOnly delegate,sessionFactory } } execution
        Hide
        Graeme Rocher added a comment -

        Ok i see its only still present in 1.1.x branch, will fix for the 1.1.2 release in the next couple of days

        Show
        Graeme Rocher added a comment - Ok i see its only still present in 1.1.x branch, will fix for the 1.1.2 release in the next couple of days
        Hide
        Goroschenya Eugene added a comment - - edited

        Good, thanks

        What changes are expected to fix this problem?
        Is it expected to remove only

        ...
                MetaProperty originalPropertiesProperty = metaClass.getMetaProperty("properties")
                metaClass.setProperties = {Object o ->
                    originalPropertiesProperty.setProperty delegate, o
                    if(delegate.hasErrors()) {
                        GrailsHibernateUtil.setObjectToReadyOnly delegate,sessionFactory
                    }
                }
        ...
        

        without additional changes (maybe additional changes are necessary to save http://jira.codehaus.org/browse/GRAILS-3163)

        Could you please provide link to changes after it will be fixed in any case, especially if it will be more complex changes than only removing code above?

        So we can apply only these changes in our Grails 1.1.1 already patched with http://jira.codehaus.org/browse/GRAILS-4580 changes without waiting official Grails 1.1.2 release.

        Show
        Goroschenya Eugene added a comment - - edited Good, thanks What changes are expected to fix this problem? Is it expected to remove only ... MetaProperty originalPropertiesProperty = metaClass.getMetaProperty( "properties" ) metaClass.setProperties = { Object o -> originalPropertiesProperty.setProperty delegate, o if (delegate.hasErrors()) { GrailsHibernateUtil.setObjectToReadyOnly delegate,sessionFactory } } ... without additional changes (maybe additional changes are necessary to save http://jira.codehaus.org/browse/GRAILS-3163 ) Could you please provide link to changes after it will be fixed in any case, especially if it will be more complex changes than only removing code above? So we can apply only these changes in our Grails 1.1.1 already patched with http://jira.codehaus.org/browse/GRAILS-4580 changes without waiting official Grails 1.1.2 release.
        Show
        Graeme Rocher added a comment - fixed by http://github.com/grails/grails/commit/d1b78f7c7d259ad1f4d77bfc0515965a50a6ea7a

          People

          • Assignee:
            Graeme Rocher
            Reporter:
            Goroschenya Eugene
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development