I've fixed the jelly/maven integration on Maven's CVS HEAD. - Brett
Quoting Hans Gilde <[EMAIL PROTECTED]>: > 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:[EMAIL PROTECTED] > Sent: Tuesday, December 07, 2004 2:28 PM > To: [EMAIL PROTECTED] > 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: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
