You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Mat McGowan <ma...@nynodata.no> on 2004/12/07 20:28:17 UTC
[jelly] comment - memory leak from using ThreadLocal
This is quite long so here's an overview:
- couldn't get jelly CVS HEAD to work with maven 1.0.1
- from code inspection, seems there may still be a memory leak even
with the fix, possibly because the tests don't exercise all tag types.
After signing in to JIRA, I wanted to a comment to the JIRA issue JELLY-148,
but was denied access, so I'll post it here.
Recent comments requested that people try out the CVS HEAD containing the
memory leak fix. Alas, I couldn't get the latest state to run: With latest
jelly from CVS patched into maven 1.0.1, running maven multiproject fails
with:
[echo] Generating the Source Xref...
maven-jxr-plugin:report:
[echo]
BUILD FAILED
File...... E:\Documents and
Settings\mat\.maven\cache\maven-multiproject-plugin-
1.3.1\plugin.jelly
Element... maven:reactor
Line...... 103
Column.... 9
Unable to obtain goal [site] -- E:\Documents and
Settings\mat\.maven\cache\maven
-jxr-plugin-1.4.2\plugin.jelly:94:55: <ant:copy> Warning: Could not find
file R:\stylesheet.css to copy.
This problem doesn't occur with the original jelly jar included in the maven
distribution. (The evaluation of the filename r:\stylesheet.css is incorrect
- it should be several directories down.) I also tried with the CVS HEAD of
jexl incase that was needed as well, but it made no difference. I also tried
replacing the tag-libraries with the latest versions, but no change.)
Anyway, I was also trying to diagnise the memory leak before I'd discovered
a fix was already available. From what I saw, I also wonder if there is
still exists a memory leak, although couldn't look at it fully because of
the regression problem above. When profiling the older jelly code (pre
WeakRefScript as found in maven 1.0.1), from the code, I was expecting all
scripts that are being held from GC by ThreadLocal to be TagScript
instances, but I also found some instances of other script types
(StaticScript, TemplateScript). The only explanation I have is that
TagScript.setTag is being called, which under current CVS HEAD _doesn't_
wrap the tag's script in a weak reference holder, so this may potentially
still cause leaks?
If I could get jelly running with maven, I would have changed
TagScript.setTag to
protected void setTag(Tag tag) {
Script body = tag.getBody();
if (!(body instanceof WeakReferenceWrapperScript))
{
tag.setBody(new WeakReferenceWrapperScript(body));
}
tagHolder.set(tag);
}
to see if that helped.
Appologies for the long post.
Regards,
Mat McGowan
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
RE: [jelly] comment - memory leak from using ThreadLocal
Posted by Brett Porter <br...@apache.org>.
I've fixed the jelly/maven integration on Maven's CVS HEAD.
- Brett
Quoting Hans Gilde <hg...@earthlink.net>:
> TagScript sets the body Script into the Tag just before the Tag is executed.
> This is after setTag has been called, and it's the only time that a Tag is
> access to its body Script. So, as far as I can tell, a Tag is never given
> access to its body Script directly. The leak was caused by a circular
> reference that caused the WeakHashMap in ThreadLocal to keep Tag instances
> forever. The circular reference is eliminated as long as a Tag doesn't have
> direct access to a Script. But, I will check out your concern; others have
> expressed questions about whether the problem is really fixed. Will also
> look into the Maven issue.
>
> Please feel free to send long posts full of good technical info any time.
>
> Hans
>
> -----Original Message-----
> From: Mat McGowan [mailto:mat@nynodata.no]
> Sent: Tuesday, December 07, 2004 2:28 PM
> To: commons-dev@jakarta.apache.org
> Subject: [jelly] comment - memory leak from using ThreadLocal
>
>
> This is quite long so here's an overview:
> - couldn't get jelly CVS HEAD to work with maven 1.0.1
> - from code inspection, seems there may still be a memory leak even
> with the fix, possibly because the tests don't exercise all tag types.
>
>
> After signing in to JIRA, I wanted to a comment to the JIRA issue JELLY-148,
> but was denied access, so I'll post it here.
>
> Recent comments requested that people try out the CVS HEAD containing the
> memory leak fix. Alas, I couldn't get the latest state to run: With latest
> jelly from CVS patched into maven 1.0.1, running maven multiproject fails
> with:
>
> [echo] Generating the Source Xref...
> maven-jxr-plugin:report:
> [echo]
>
> BUILD FAILED
> File...... E:\Documents and
> Settings\mat\.maven\cache\maven-multiproject-plugin-
> 1.3.1\plugin.jelly
> Element... maven:reactor
> Line...... 103
> Column.... 9
> Unable to obtain goal [site] -- E:\Documents and
> Settings\mat\.maven\cache\maven
> -jxr-plugin-1.4.2\plugin.jelly:94:55: <ant:copy> Warning: Could not find
> file R:\stylesheet.css to copy.
>
> This problem doesn't occur with the original jelly jar included in the maven
> distribution. (The evaluation of the filename r:\stylesheet.css is incorrect
> - it should be several directories down.) I also tried with the CVS HEAD of
> jexl incase that was needed as well, but it made no difference. I also tried
> replacing the tag-libraries with the latest versions, but no change.)
>
> Anyway, I was also trying to diagnise the memory leak before I'd discovered
> a fix was already available. From what I saw, I also wonder if there is
> still exists a memory leak, although couldn't look at it fully because of
> the regression problem above. When profiling the older jelly code (pre
> WeakRefScript as found in maven 1.0.1), from the code, I was expecting all
> scripts that are being held from GC by ThreadLocal to be TagScript
> instances, but I also found some instances of other script types
> (StaticScript, TemplateScript). The only explanation I have is that
> TagScript.setTag is being called, which under current CVS HEAD _doesn't_
> wrap the tag's script in a weak reference holder, so this may potentially
> still cause leaks?
>
> If I could get jelly running with maven, I would have changed
> TagScript.setTag to
> protected void setTag(Tag tag) {
> Script body = tag.getBody();
> if (!(body instanceof WeakReferenceWrapperScript))
> {
> tag.setBody(new WeakReferenceWrapperScript(body));
> }
> tagHolder.set(tag);
> }
>
> to see if that helped.
>
> Appologies for the long post.
> Regards,
> Mat McGowan
>
>
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
RE: [jelly] comment - memory leak from using ThreadLocal
Posted by Hans Gilde <hg...@earthlink.net>.
TagScript sets the body Script into the Tag just before the Tag is executed.
This is after setTag has been called, and it's the only time that a Tag is
access to its body Script. So, as far as I can tell, a Tag is never given
access to its body Script directly. The leak was caused by a circular
reference that caused the WeakHashMap in ThreadLocal to keep Tag instances
forever. The circular reference is eliminated as long as a Tag doesn't have
direct access to a Script. But, I will check out your concern; others have
expressed questions about whether the problem is really fixed. Will also
look into the Maven issue.
Please feel free to send long posts full of good technical info any time.
Hans
-----Original Message-----
From: Mat McGowan [mailto:mat@nynodata.no]
Sent: Tuesday, December 07, 2004 2:28 PM
To: commons-dev@jakarta.apache.org
Subject: [jelly] comment - memory leak from using ThreadLocal
This is quite long so here's an overview:
- couldn't get jelly CVS HEAD to work with maven 1.0.1
- from code inspection, seems there may still be a memory leak even
with the fix, possibly because the tests don't exercise all tag types.
After signing in to JIRA, I wanted to a comment to the JIRA issue JELLY-148,
but was denied access, so I'll post it here.
Recent comments requested that people try out the CVS HEAD containing the
memory leak fix. Alas, I couldn't get the latest state to run: With latest
jelly from CVS patched into maven 1.0.1, running maven multiproject fails
with:
[echo] Generating the Source Xref...
maven-jxr-plugin:report:
[echo]
BUILD FAILED
File...... E:\Documents and
Settings\mat\.maven\cache\maven-multiproject-plugin-
1.3.1\plugin.jelly
Element... maven:reactor
Line...... 103
Column.... 9
Unable to obtain goal [site] -- E:\Documents and
Settings\mat\.maven\cache\maven
-jxr-plugin-1.4.2\plugin.jelly:94:55: <ant:copy> Warning: Could not find
file R:\stylesheet.css to copy.
This problem doesn't occur with the original jelly jar included in the maven
distribution. (The evaluation of the filename r:\stylesheet.css is incorrect
- it should be several directories down.) I also tried with the CVS HEAD of
jexl incase that was needed as well, but it made no difference. I also tried
replacing the tag-libraries with the latest versions, but no change.)
Anyway, I was also trying to diagnise the memory leak before I'd discovered
a fix was already available. From what I saw, I also wonder if there is
still exists a memory leak, although couldn't look at it fully because of
the regression problem above. When profiling the older jelly code (pre
WeakRefScript as found in maven 1.0.1), from the code, I was expecting all
scripts that are being held from GC by ThreadLocal to be TagScript
instances, but I also found some instances of other script types
(StaticScript, TemplateScript). The only explanation I have is that
TagScript.setTag is being called, which under current CVS HEAD _doesn't_
wrap the tag's script in a weak reference holder, so this may potentially
still cause leaks?
If I could get jelly running with maven, I would have changed
TagScript.setTag to
protected void setTag(Tag tag) {
Script body = tag.getBody();
if (!(body instanceof WeakReferenceWrapperScript))
{
tag.setBody(new WeakReferenceWrapperScript(body));
}
tagHolder.set(tag);
}
to see if that helped.
Appologies for the long post.
Regards,
Mat McGowan
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org