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!



--
View this message in context: http://grails.1312388.n4.nabble.com/Easily-reproducible-memory-leak-in-Grails-tp4655816.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

[John Engelman]

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 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!
--
View this message in context: http://grails.1312388.n4.nabble.com/Easily-reproducible-memory-leak-in-Grails-tp4655816.html
Sent from the Grails - dev mailing list archive at Nabble.com (http://Nabble.com).
---------------------------------------------------------------------
http://xircles.codehaus.org/manage_email

[candrews]

Post by John Engelman

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).



--
View this message in context: http://grails.1312388.n4.nabble.com/Easily-reproducible-memory-leak-in-Grails-tp4655816p4655822.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

[John Engelman]

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

http://stackoverflow.com/questions/148681/unloading-classes-in-java

This has also been broached a number of times with Grails itself:
http://jira.grails.org/browse/GRAILS-6571
http://jira.grails.org/browse/GRAILS-5560
http://jira.grails.org/browse/GRAILS-7169
http://grails.1312388.n4.nabble.com/GroovyPagesTemplateEngine-GSP-related-re-design-needed-td3215501.html

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:

http://freemarker.org/
http://velocity.apache.org/
http://handlebarsjs.com/

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

-- John

Post by candrews

Post by John Engelman

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).
--
View this message in context: http://grails.1312388.n4.nabble.com/Easily-reproducible-memory-leak-in-Grails-tp4655816p4655822.html
Sent from the Grails - dev mailing list archive at Nabble.com (http://Nabble.com).
---------------------------------------------------------------------
http://xircles.codehaus.org/manage_email

[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?



--
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

[Aaron Long]

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.

Post by 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
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
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?
--
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.
---------------------------------------------------------------------
http://xircles.codehaus.org/manage_email

[David Estes]

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

Post by 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
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
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.
---------------------------------------------------------------------
http://xircles.codehaus.org/manage_email

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

http://xircles.codehaus.org/manage_email

[candrews]

Post by Aaron Long
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.

Post by Aaron Long
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]

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

Post by candrews

Post by Aaron Long
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.

Post by Aaron Long
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.
---------------------------------------------------------------------
http://xircles.codehaus.org/manage_email

[Graeme Rocher]

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

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

Post by candrews

Post by Aaron Long
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.

Post by Aaron Long
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.
---------------------------------------------------------------------
http://xircles.codehaus.org/manage_email

--
Graeme Rocher
Grails Project Lead
SpringSource

[candrews]

Post by Graeme Rocher
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).



--
View this message in context: http://grails.1312388.n4.nabble.com/Easily-reproducible-memory-leak-in-Grails-tp4655816p4655851.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

[candrews]

Post by candrews

Post by Graeme Rocher
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.

Post by candrews
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.



--
View this message in context: http://grails.1312388.n4.nabble.com/Easily-reproducible-memory-leak-in-Grails-tp4655816p4655860.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

[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.



--
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]

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

Post by 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.
* 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.
---------------------------------------------------------------------
http://xircles.codehaus.org/manage_email

[candrews]

Post by Graeme Rocher
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]

I'm not sure what side effects that may have

Cheers

Post by candrews

Post by Graeme Rocher
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.
-------------------------------------------
public static void registerMethodMissingForGSP(Class gspClass,
TagLibraryLookup gspTagLibraryLookup) {
registerMethodMissingForGSP(GrailsMetaClassUtils.getExpandoMetaClass(gspClass),
gspTagLibraryLookup)
}
-------------------------------------------
-------------------------------------------
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.
---------------------------------------------------------------------
http://xircles.codehaus.org/manage_email

[candrews]

I submitted a pull request: https://github.com/grails/grails-core/pull/481



--
View this message in context: http://grails.1312388.n4.nabble.com/Easily-reproducible-memory-leak-in-Grails-tp4655816p4655908.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

[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.



--
View this message in context: http://grails.1312388.n4.nabble.com/Easily-reproducible-memory-leak-in-Grails-tp4655816p4655961.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

[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.



--
View this message in context: http://grails.1312388.n4.nabble.com/Easily-reproducible-memory-leak-in-Grails-tp4655816p4655963.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

[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



--
View this message in context: http://grails.1312388.n4.nabble.com/Easily-reproducible-memory-leak-in-Grails-tp4655816p4655985.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