Easily reproducible memory leak in Grails

classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|

Easily reproducible memory leak in Grails

candrews
I have found a very easy to reproduce memory leak in Grails - just create a new app and run-app it, then touch a gsp and refresh the URL that serves that GSP. Full instructions are at all http://jira.grails.org/browse/GRAILS-11296

This issue is always reproducible when using "run-app" and can be reproduced in war mode when grails.gsp.enable.reload = true.

It appears that org.codehaus.groovy.runtime.metaclass.MetaMethodIndex$Entry instances are being created but never GC'ed. The application crashed with an out of permgen error.. so I think there's both a heap leak and a permgen leak.

Can someone please help me track this down? It seems like a pretty serious bug in Grails (perhaps Groovy too?).

Thanks!
Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

johnrengelman
This is expected when enabling GSP reloading in a Grails application. The docs state:
One thing to bear in mind with this technique is that every time you modify a GSP, it uses up permgen space. So at some point you will eventually hit "out of permgen space" errors unless you restart the server. So this technique is not recommended for frequent or large changes to the views.
GSPs are compiled into Classes, so each time you modify the source, a new class is compiled and loaded by the ClassLoader. This is what eats up the PermGen space (and some heap space for the associated Meta objects for the class). 
The memory leak occurs because the ClassLoader contains a reference to the Class which contains references to the MetaMethod objects that live in the heap (though I believe these will be GC'd if the heap nears its max, though that typically just leads to GC thrashing).

-- John

On Tuesday, April 8, 2014 at 1:01 PM, candrews wrote:

I have found a very easy to reproduce memory leak in Grails - just create a
new app and run-app it, then touch a gsp and refresh the URL that serves
that GSP. Full instructions are at all

This issue is always reproducible when using "run-app" and can be reproduced
in war mode when grails.gsp.enable.reload = true.

It appears that org.codehaus.groovy.runtime.metaclass.MetaMethodIndex$Entry
instances are being created but never GC'ed. The application crashed with an
out of permgen error.. so I think there's both a heap leak and a permgen
leak.

Can someone please help me track this down? It seems like a pretty serious
bug in Grails (perhaps Groovy too?).

Thanks!



--
Sent from the Grails - dev mailing list archive at Nabble.com.

---------------------------------------------------------------------
To unsubscribe from this list, please visit:


Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

candrews
johnrengelman wrote
This is expected when enabling GSP reloading in a Grails application. The docs state:
> One thing to bear in mind with this technique is that every time you modify a GSP, it uses up permgen space. So at some point you will eventually hit "out of permgen space" errors unless you restart the server. So this technique is not recommended for frequent or large changes to the views.

GSPs are compiled into Classes, so each time you modify the source, a new class is compiled and loaded by the ClassLoader. This is what eats up the PermGen space (and some heap space for the associated Meta objects for the class).
The memory leak occurs because the ClassLoader contains a reference to the Class which contains references to the MetaMethod objects that live in the heap (though I believe these will be GC'd if the heap nears its max, though that typically just leads to GC thrashing).

-- John
I see the disclaimer in the doc, but I think that's kind of a cop-out :-) This is (in my opinion) a significant problem, it really should be fixed - and I'm very interested in fixing it. Fixing this issue would help developers working in Grails (as I bet many people frequently edit GSPs when developing) and it would help deployments of Grails that update GSPs at runtime (as my use case requires).

Given that GSPs compile to classes, when a class is no longer referenced, it should GCed. Other similar technologies (such as JSP) work the same way, and don't leak, so not leaking seems possible.

What ClassLoader is holding references to these classes? Why is it holding hard references? Instead of using hard references, could we use weak or soft references instead?

Alternatively, could we use a new ClassLoader each time a GSPs is loaded? That way, the ClassLoader and the GSP class will both not be referenced after the GSP class is replaced (because the GSP changed resulting in a new GSP class).
Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

johnrengelman
I don't really think it's a cop-out. Class unloading is very difficult in the JVM:


This has also been broached a number of times with Grails itself:

GSP reloading is a bit of a niche capability. Mainly its for development, and restarting the server shouldn't really be a problem, and you can configure your JAVA_OPTS to have enough PermGen to get you through any development cycle. When enabling GSP reloading in a production environment, the number of GSP edits should be very minimal thus the leak will be inconsequential. If you find yourself needing a large number of GSP reloads, then there are other tools for doing it besides GSPs:


And just to clarify, we literally just encountered this same issue in our production environment last week.

-- John

On Tuesday, April 8, 2014 at 4:21 PM, candrews wrote:

johnrengelman wrote
This is expected when enabling GSP reloading in a Grails application. The
docs state:
One thing to bear in mind with this technique is that every time you
modify a GSP, it uses up permgen space. So at some point you will
eventually hit "out of permgen space" errors unless you restart the
server. So this technique is not recommended for frequent or large
changes to the views.

GSPs are compiled into Classes, so each time you modify the source, a new
class is compiled and loaded by the ClassLoader. This is what eats up the
PermGen space (and some heap space for the associated Meta objects for the
class).
The memory leak occurs because the ClassLoader contains a reference to the
Class which contains references to the MetaMethod objects that live in the
heap (though I believe these will be GC'd if the heap nears its max,
though that typically just leads to GC thrashing).

-- John

I see the disclaimer in the doc, but I think that's kind of a cop-out :-)
This is (in my opinion) a significant problem, it really should be fixed -
and I'm very interested in fixing it. Fixing this issue would help
developers working in Grails (as I bet many people frequently edit GSPs when
developing) and it would help deployments of Grails that update GSPs at
runtime (as my use case requires).

Given that GSPs compile to classes, when a class is no longer referenced, it
should GCed. Other similar technologies (such as JSP) work the same way, and
don't leak, so not leaking seems possible.

What ClassLoader is holding references to these classes? Why is it holding
hard references? Instead of using hard references, could we use weak or soft
references instead?

Alternatively, could we use a new ClassLoader each time a GSPs is loaded?
That way, the ClassLoader and the GSP class will both not be referenced
after the GSP class is replaced (because the GSP changed resulting in a new
GSP class).



--
Sent from the Grails - dev mailing list archive at Nabble.com.

---------------------------------------------------------------------
To unsubscribe from this list, please visit:


Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

candrews
I certainly cannot disagree that class loading is difficult in the JVM... but I think we should take a page from JFK's playbook and fix these issues "not because they are easy, but because they are hard" (and because fixing this would make Grails much more useful!).

I've been spending some time looking into this, and the "pageCache" implementation in GroovyPagesTemplateEngine seems to work fine. It's not growing unbounded (in the scenario from my bug report, it stays at size 2).

However, I'm pretty sure I found a leak elsewhere. In org.codehaus.groovy.reflection.ClassInfo there's this field:
private static final ClassInfoSet globalClassSet = new ClassInfoSet(softBundle);
It appears to be holding strong references to classes, and it's never cleared. Each GroovyPage class generated for a GSP gets a ClassInfo instance added to ClassInfo.globalClassSet. ClassInfo holds a soft reference to the GroovyPage class, so the GroovyPage class gets GC'ed, but the ClassInfo instance for that GroovyPage class never does. The adding is done at this line:
https://github.com/groovy/groovy-core/blob/GROOVY_2_0_8/src/main/org/codehaus/groovy/reflection/ClassInfo.java#L103
In my testing, ClassInfo.globalClassSet gets to many thousands, eventually causing the VM to run out of memory.

Does that look like it may be the source of the problem to you?
Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

longwa
Java 8 eliminates the concept of PermGem altogether so I'm curious to see how that will affect all the various class reloading, hot deployment, etc. leaks that we see. Obviously classes are still loaded and have to take up memory somewhere, but hopefully the JVM will get smarter about cleaning up.


On Tue, Apr 8, 2014 at 6:10 PM, candrews <[hidden email]> wrote:
I certainly cannot disagree that class loading is difficult in the JVM... but
I think we should take a page from JFK's playbook and fix these issues "not
because they are easy, but because they are hard" (and because fixing this
would make Grails much more useful!).

I've been spending some time looking into this, and the "pageCache"
implementation in GroovyPagesTemplateEngine seems to work fine. It's not
growing unbounded (in the scenario from my bug report, it stays at size 2).

However, I'm pretty sure I found a leak elsewhere. In
org.codehaus.groovy.reflection.ClassInfo there's this field:
private static final ClassInfoSet globalClassSet = new
ClassInfoSet(softBundle);
It appears to be holding strong references to classes, and it's never
cleared. Each GroovyPage class generated for a GSP gets a ClassInfo instance
added to ClassInfo.globalClassSet. ClassInfo holds a soft reference to the
GroovyPage class, so the GroovyPage class gets GC'ed, but the ClassInfo
instance for that GroovyPage class never does. The adding is done at this
line:
https://github.com/groovy/groovy-core/blob/GROOVY_2_0_8/src/main/org/codehaus/groovy/reflection/ClassInfo.java#L103
In my testing, ClassInfo.globalClassSet gets to many thousands, eventually
causing the VM to run out of memory.

Does that look like it may be the source of the problem to you?



--
View this message in context: http://grails.1312388.n4.nabble.com/Easily-reproducible-memory-leak-in-Grails-tp4655816p4655824.html
Sent from the Grails - dev mailing list archive at Nabble.com.

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email



Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

David Estes
In reply to this post by candrews
How does this affect dynamic gsp rendering... i.e. From a database ? This seems like a major issue and would only get worse when gsp is made as a standalone capable grails plugin no?

Sent from my iPad

> On Apr 8, 2014, at 6:10 PM, candrews <[hidden email]> wrote:
>
> I certainly cannot disagree that class loading is difficult in the JVM... but
> I think we should take a page from JFK's playbook and fix these issues "not
> because they are easy, but because they are hard" (and because fixing this
> would make Grails much more useful!).
>
> I've been spending some time looking into this, and the "pageCache"
> implementation in GroovyPagesTemplateEngine seems to work fine. It's not
> growing unbounded (in the scenario from my bug report, it stays at size 2).
>
> However, I'm pretty sure I found a leak elsewhere. In
> org.codehaus.groovy.reflection.ClassInfo there's this field:
> private static final ClassInfoSet globalClassSet = new
> ClassInfoSet(softBundle);
> It appears to be holding strong references to classes, and it's never
> cleared. Each GroovyPage class generated for a GSP gets a ClassInfo instance
> added to ClassInfo.globalClassSet. ClassInfo holds a soft reference to the
> GroovyPage class, so the GroovyPage class gets GC'ed, but the ClassInfo
> instance for that GroovyPage class never does. The adding is done at this
> line:
> https://github.com/groovy/groovy-core/blob/GROOVY_2_0_8/src/main/org/codehaus/groovy/reflection/ClassInfo.java#L103
> In my testing, ClassInfo.globalClassSet gets to many thousands, eventually
> causing the VM to run out of memory.
>
> Does that look like it may be the source of the problem to you?
>
>
>
> --
> View this message in context: http://grails.1312388.n4.nabble.com/Easily-reproducible-memory-leak-in-Grails-tp4655816p4655824.html
> Sent from the Grails - dev mailing list archive at Nabble.com.
>
> ---------------------------------------------------------------------
> To unsubscribe from this list, please visit:
>
>    http://xircles.codehaus.org/manage_email
>
>

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email


Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

candrews
longwa wrote
Java 8 eliminates the concept of PermGem altogether so I'm curious to see how that will affect all the various class reloading, hot deployment, etc. leaks that we see. Obviously classes are still loaded and have to take up memory somewhere, but hopefully the JVM will get smarter about cleaning up.
I don't think Java 8's different memory organization would eliminate this issue. It may delay it from happening, but it would still happen. Notably, the Java 8 organization of memory is similar to how IBM Java J9 organizes memory, and I definitely see this problem on that VM.

David Estes wrote
How does this affect dynamic gsp rendering... i.e. From a database ? This seems like a major issue and would only get worse when gsp is made as a standalone capable grails plugin no?
To be totally honest, I'm not 100% positive, as I don't fully understand what's happening yet. But, based on what I know so far with the way ClassInfo works, it *seems* like this should result in a memory leak when doing dynamic GSP rendering with a database. I say this because reading the GSP source text from a database should be the same as reading from a file system which is what the issue I created describes how to reproduce.
Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

Graeme Rocher-2
I've asked a member of the Groovy team to comment on 'globalClassSet'
and its usage

On Wed, Apr 9, 2014 at 4:07 AM, candrews <[hidden email]> wrote:

> longwa wrote
>> Java 8 eliminates the concept of PermGem altogether so I'm curious to see
>> how that will affect all the various class reloading, hot deployment, etc.
>> leaks that we see. Obviously classes are still loaded and have to take up
>> memory somewhere, but hopefully the JVM will get smarter about cleaning
>> up.
>
> I don't think Java 8's different memory organization would eliminate this
> issue. It may delay it from happening, but it would still happen. Notably,
> the Java 8 organization of memory is similar to how IBM Java J9 organizes
> memory, and I definitely see this problem on that VM.
>
>
> David Estes wrote
>> How does this affect dynamic gsp rendering... i.e. From a database ? This
>> seems like a major issue and would only get worse when gsp is made as a
>> standalone capable grails plugin no?
>
> To be totally honest, I'm not 100% positive, as I don't fully understand
> what's happening yet. But, based on what I know so far with the way
> ClassInfo works, it *seems* like this should result in a memory leak when
> doing dynamic GSP rendering with a database. I say this because reading the
> GSP source text from a database should be the same as reading from a file
> system which is what the issue I created describes how to reproduce.
>
>
>
> --
> View this message in context: http://grails.1312388.n4.nabble.com/Easily-reproducible-memory-leak-in-Grails-tp4655816p4655830.html
> Sent from the Grails - dev mailing list archive at Nabble.com.
>
> ---------------------------------------------------------------------
> To unsubscribe from this list, please visit:
>
>     http://xircles.codehaus.org/manage_email
>
>



--
Graeme Rocher
Grails Project Lead
SpringSource

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email


Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

Graeme Rocher-2
Note GroovyPagesTemplateEngine uses a single shared class loader for
reloading changes to GSPs so even if Groovy is holding onto
references, the fact that a single hard references class loader is
used means the classes will always stick around.

Cheers

On Wed, Apr 9, 2014 at 2:32 PM, Graeme Rocher <[hidden email]> wrote:

> I've asked a member of the Groovy team to comment on 'globalClassSet'
> and its usage
>
> On Wed, Apr 9, 2014 at 4:07 AM, candrews <[hidden email]> wrote:
>> longwa wrote
>>> Java 8 eliminates the concept of PermGem altogether so I'm curious to see
>>> how that will affect all the various class reloading, hot deployment, etc.
>>> leaks that we see. Obviously classes are still loaded and have to take up
>>> memory somewhere, but hopefully the JVM will get smarter about cleaning
>>> up.
>>
>> I don't think Java 8's different memory organization would eliminate this
>> issue. It may delay it from happening, but it would still happen. Notably,
>> the Java 8 organization of memory is similar to how IBM Java J9 organizes
>> memory, and I definitely see this problem on that VM.
>>
>>
>> David Estes wrote
>>> How does this affect dynamic gsp rendering... i.e. From a database ? This
>>> seems like a major issue and would only get worse when gsp is made as a
>>> standalone capable grails plugin no?
>>
>> To be totally honest, I'm not 100% positive, as I don't fully understand
>> what's happening yet. But, based on what I know so far with the way
>> ClassInfo works, it *seems* like this should result in a memory leak when
>> doing dynamic GSP rendering with a database. I say this because reading the
>> GSP source text from a database should be the same as reading from a file
>> system which is what the issue I created describes how to reproduce.
>>
>>
>>
>> --
>> View this message in context: http://grails.1312388.n4.nabble.com/Easily-reproducible-memory-leak-in-Grails-tp4655816p4655830.html
>> Sent from the Grails - dev mailing list archive at Nabble.com.
>>
>> ---------------------------------------------------------------------
>> To unsubscribe from this list, please visit:
>>
>>     http://xircles.codehaus.org/manage_email
>>
>>
>
>
>
> --
> Graeme Rocher
> Grails Project Lead
> SpringSource



--
Graeme Rocher
Grails Project Lead
SpringSource

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email


Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

candrews
Graeme Rocher-2 wrote
Note GroovyPagesTemplateEngine uses a single shared class loader for
reloading changes to GSPs so even if Groovy is holding onto
references, the fact that a single hard references class loader is
used means the classes will always stick around.
I'm trying to figure out how that works. By setting a breakpoint in groovy.lang.GroovyClassLoader.doParseClass, I can see that, as you said, there is 1 instance of org.codehaus.groovy.grails.compiler.web.pages.GroovyPageClassLoader used for all GSPs. By inspecting groovy.lang.GroovyClassLoader.classCache, I can see that the classCache, however, never grows. Using Grails 2.2.5 and the scenario from the ticket, it stays at size 51 always.

The classCache uses the name as the key, and since the name always stays the same, the value gets overwritten and therefore the class reference is lost. I can't seem to find the holding of hard references to every GSP every compiled by the class loader that you mentioned (if the GSP name changes all the time, I can definitely see what you said being a problem, but that's not the case in this scenario).
Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

candrews
candrews wrote
Graeme Rocher-2 wrote
Note GroovyPagesTemplateEngine uses a single shared class loader for
reloading changes to GSPs so even if Groovy is holding onto
references, the fact that a single hard references class loader is
used means the classes will always stick around.
I'm trying to figure out how that works. By setting a breakpoint in groovy.lang.GroovyClassLoader.doParseClass, I can see that, as you said, there is 1 instance of org.codehaus.groovy.grails.compiler.web.pages.GroovyPageClassLoader used for all GSPs. By inspecting groovy.lang.GroovyClassLoader.classCache, I can see that the classCache, however, never grows. Using Grails 2.2.5 and the scenario from the ticket, it stays at size 51 always.

The classCache uses the name as the key, and since the name always stays the same, the value gets overwritten and therefore the class reference is lost. I can't seem to find the holding of hard references to every GSP every compiled by the class loader that you mentioned (if the GSP name changes all the time, I can definitely see what you said being a problem, but that's not the case in this scenario).
I went down the rabbit hole in org.codehaus.groovy.grails.compiler.web.pages.GroovyPageClassLoader. The JVM updates the private field "classes" so the classloader that loads a class always holds a strong reference to the class. Inspecting the field, I saw that "classes" is always empty. Reading groovy.lang.GroovyClassLoader.doParseClass, you can see how Groovy create a groovy.lang.GroovyClassLoader.InnerLoader instance for each class it loads - that way, each class loader loads exactly 1 class, so there's no big classloader with references to a whole bunch of classes. So, with that said, I'm becoming increasingly certain that this isn't a classloader problem, but something else in the way Groovy and/or Grails works.

From analyzing the heap dump, you can see there are a LOT of instances of ExpandoMetaClass. What's happening there is:
* The GSP is loaded in GroovyPagesTemplateEngine.createTemplate, which calls
* GroovyPagesTemplateEngine.compileGroovyPage which calls
* GroovyPagesMetaUtils.registerMethodMissingForGSP which calls
* GrailsMetaClassUtils.getExpandoMetaClass which calls
* MetaClassRegistry.getClassInfo which calls
* ClassInfo.getClassInfo which has this line:
return (ClassInfo) globalClassSet.getOrPut(cls,null);

And that's the problem... entries are added to globalClassSet but never removed.

ClassInfo.globalClassSet is declared as:
private static final ClassInfoSet globalClassSet = new ClassInfoSet(softBundle);
It seems to be some kind of soft hash map, where if the referent gets GC'ed (because of the soft reference) the item should be removed (this is implemented in org.codehaus.groovy.util.ManagedConcurrentMap). But I haven't been able to figure out why that's not working. I set breakpoints in:
ManagedConcurrentMap.Entry.finalizeRef
ManagedConcurrentMap.EntryWithValue.finalizeRef
ClassInfo.finalizeRef
and they're never hit.
Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

candrews
Similar to how ClassInfo.globalClassSet grows boundlessly, ClassInfo.modifiedExpandos also grows boundlessly. Items are added to ClassInfo.modifiedExpandos in ClassInfo.setStrongMetaClass which is called for every GroovyPage instance.

The method ClassInfo.clearModifiedExpandos() is the only thing that could remove items from ClassInfo.modifiedExpandos, but it's never called.

So there's at least 2 leaking collections:
* ClassInfo.globalClassSet
* ClassInfo.modifiedExpandos

It seems like the item in ClassInfo.modifiedExpandos should be removed at some point.
Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

Graeme Rocher-2
It might be that the MetaClass of the old GSP class is not being
cleared and a hard reference retained in ClassInfo.modifiedExpandos

Calling GroovySystem.metaClassRegistry.removeMetaClass(oldGspClass)
might solve the problem

On Thu, Apr 10, 2014 at 12:33 AM, candrews <[hidden email]> wrote:

> Similar to how ClassInfo.globalClassSet grows boundlessly,
> ClassInfo.modifiedExpandos also grows boundlessly. Items are added to
> ClassInfo.modifiedExpandos in ClassInfo.setStrongMetaClass which is called
> for every GroovyPage instance.
>
> The method ClassInfo.clearModifiedExpandos() is the only thing that could
> remove items from ClassInfo.modifiedExpandos, but it's never called.
>
> So there's at least 2 leaking collections:
> * ClassInfo.globalClassSet
> * ClassInfo.modifiedExpandos
>
> It seems like the item in ClassInfo.modifiedExpandos should be removed at
> some point.
>
>
>
> --
> View this message in context: http://grails.1312388.n4.nabble.com/Easily-reproducible-memory-leak-in-Grails-tp4655816p4655868.html
> Sent from the Grails - dev mailing list archive at Nabble.com.
>
> ---------------------------------------------------------------------
> To unsubscribe from this list, please visit:
>
>     http://xircles.codehaus.org/manage_email
>
>



--
Graeme Rocher
Grails Project Lead
SpringSource

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email


Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

candrews
Graeme Rocher-2 wrote
It might be that the MetaClass of the old GSP class is not being
cleared and a hard reference retained in ClassInfo.modifiedExpandos

Calling GroovySystem.metaClassRegistry.removeMetaClass(oldGspClass)
might solve the problem
I came to the same conclusion. The problem I had is figuring out when/where to call GroovySystem.metaClassRegistry.removeMetaClass(oldGspClass); there no clear place (as far as I can tell) to know when a GSP class isn't being used any more.

I took a different approach; instead of having the metaClass have a strong reference to the class, make it a weak reference.
In GroovyPagesMetaUtils, change this:
-------------------------------------------
    public static void registerMethodMissingForGSP(Class gspClass, TagLibraryLookup gspTagLibraryLookup) {
        registerMethodMissingForGSP(GrailsMetaClassUtils.getExpandoMetaClass(gspClass), gspTagLibraryLookup)
    }
-------------------------------------------
to this:
-------------------------------------------
    public static void registerMethodMissingForGSP(Class gspClass, TagLibraryLookup gspTagLibraryLookup) {
        registerMethodMissingForGSP(GrailsMetaClassUtils.getExpandoMetaClass(gspClass), gspTagLibraryLookup)
        // change the metaclass reference from a Strong to a Weak Reference
        // this way, when the GSP class is no longer used, it'll be eligible for GC
        final ClassInfo info = ClassInfo.getClassInfo(gspClass);
        info.lock();
        try {
          MetaClass mc = info.getStrongMetaClass();
          info.setStrongMetaClass(null);
          info.setWeakMetaClass(mc);
        } finally {
          info.unlock();
        }
    }
-------------------------------------------

That seems to fix the problem (I had to patch it in a different way for my testing, because I haven't gone through the effort of building Grails, but the fix is the same).

What are your thoughts?
Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

Graeme Rocher-2
I'm not sure what side effects that may have

Cheers

On Thu, Apr 10, 2014 at 5:54 PM, candrews <[hidden email]> wrote:

> Graeme Rocher-2 wrote
>> It might be that the MetaClass of the old GSP class is not being
>> cleared and a hard reference retained in ClassInfo.modifiedExpandos
>>
>> Calling GroovySystem.metaClassRegistry.removeMetaClass(oldGspClass)
>> might solve the problem
>
> I came to the same conclusion. The problem I had is figuring out when/where
> to call GroovySystem.metaClassRegistry.removeMetaClass(oldGspClass); there
> no clear place (as far as I can tell) to know when a GSP class isn't being
> used any more.
>
> I took a different approach; instead of having the metaClass have a strong
> reference to the class, make it a weak reference.
> In GroovyPagesMetaUtils, change this:
> -------------------------------------------
>     public static void registerMethodMissingForGSP(Class gspClass,
> TagLibraryLookup gspTagLibraryLookup) {
>
> registerMethodMissingForGSP(GrailsMetaClassUtils.getExpandoMetaClass(gspClass),
> gspTagLibraryLookup)
>     }
> -------------------------------------------
> to this:
> -------------------------------------------
>     public static void registerMethodMissingForGSP(Class gspClass,
> TagLibraryLookup gspTagLibraryLookup) {
>
> registerMethodMissingForGSP(GrailsMetaClassUtils.getExpandoMetaClass(gspClass),
> gspTagLibraryLookup)
>         // change the metaclass reference from a Strong to a Weak Reference
>         // this way, when the GSP class is no longer used, it'll be eligible
> for GC
>         final ClassInfo info = ClassInfo.getClassInfo(gspClass);
>         info.lock();
>         try {
>           MetaClass mc = info.getStrongMetaClass();
>           info.setStrongMetaClass(null);
>           info.setWeakMetaClass(mc);
>         } finally {
>           info.unlock();
>         }
>     }
> -------------------------------------------
>
> That seems to fix the problem (I had to patch it in a different way for my
> testing, because I haven't gone through the effort of building Grails, but
> the fix is the same).
>
> What are your thoughts?
>
>
>
> --
> View this message in context: http://grails.1312388.n4.nabble.com/Easily-reproducible-memory-leak-in-Grails-tp4655816p4655906.html
> Sent from the Grails - dev mailing list archive at Nabble.com.
>
> ---------------------------------------------------------------------
> To unsubscribe from this list, please visit:
>
>     http://xircles.codehaus.org/manage_email
>
>



--
Graeme Rocher
Grails Project Lead
SpringSource

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

    http://xircles.codehaus.org/manage_email


Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

candrews
In reply to this post by candrews
Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

candrews
After some testing, I discovered the same thing that Graeme discovered - the MetaClass, since it's soft referenced (the groovy variable name is "weak" but if you look at the code it's "soft"), can get GC'ed. In my testing using the simple reproduction steps, I didn't encounter the metaclass getting GC'ed. But apparently the IBM JVM (at least as I'm using it) collects soft references more aggressively, and make this problem obvious. The result, when this happens, is MissingMethodExceptions and broken GSP behavior.

I'm working on a different way to solve this problem now.
Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

candrews
I submitted a pull request for a new approach: https://github.com/grails/grails-core/pull/482 : When GroovyPageMetaInfo is removed from the pageCache, remove the metaclass from the pageClass in the GroovyPageMetaInfo.

I actually implemented this change yesterday, have been running on my application (with the Oracle and the IBM JVMs) since then... and no problems yet.

I see 2 problems with this approach:
1) If something is using the pageClass while GroovyPagesTemplateEngine compiles a new version of that pageClass and updates it's pageCache, the old pageClass will have it's metaClass removed (which is good) but that pageClass may be in use still (which is bad). I think this is probably rare, but it's still not perfect, which makes me unhappy.
2) If the pageCache isn't used (there are some code paths that don't use the pageCache), then this fix has no effect and the leak remains.

I think the best solution would be to have the metaClass be weakly reachable from the class in all cases (not just for GSP classes, I'm talking big picture here). This would be a Groovy change (not a Grails one), and I haven't really figured out how to implement it. But as a start, I'm thinking that:
1) org.codehaus.groovy.reflection.ClassInfo.globalClassSet (which is a map of Class->ClassInfo) needs to use weak references to the Class (key in the map) and strong references to the ClassInfo (value in the map). Note that this may already be the way it works... I don't understand org.codehaus.groovy.util.ManagedConcurrentMap well enough yet.
2) org.codehaus.groovy.reflection.ClassInfo.modifiedExpandos needs to hold weak references to ClassInfo

With these changes in place, when a Class gets GC'ed, no references to its ClassInfo should exist, allowing the ClassInfo to be GC'ed, which will allow the ExpandoMetaClass (that the ClassInfo held the only reference to) to be GC'ed.
Reply | Threaded
Open this post in threaded view
|

Re: Easily reproducible memory leak in Grails

candrews
I created a test case that reproduces the memory leak using just Groovy (not all of Grails) and asked for help on groovy-dev: http://groovy.329449.n5.nabble.com/Memory-leak-in-ClassInfo-when-using-MetaClasses-td5719218.html