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]
