https://issues.apache.org/bugzilla/show_bug.cgi?id=52850

--- Comment #7 from Rohit Kelapure <kelap...@gmail.com> 2012-03-09 22:20:48 UTC 
---
(In reply to comment #5)

Thanks for accepting the patch

"I don't see any changes to address this. Did I miss them?"
Ensure that key and value types ALWAYS show up for all types of threadlocal
leaks (direct & indirect)
Method expungeStaleEntriesMethod =
tlmClass.getDeclaredMethod("expungeStaleEntries");
expungeStaleEntriesMethod.setAccessible(true);


I had to change WebappClassLoader.loadedByThisOrChild(Object)) to get the
Threadlocal leak test cases to pass.

I will be grateful if you add a check in the tests that confirms that the leak
warnings have showed up in the log. I did not add junit asserts for the
presence of log warnings. 

--Thanks


> (In reply to comment #0)
> > - Most of the detection and fixing code has been tested ONLY on Sun JVMs.
> Correct.
> 
> > - Tests in tomcat7.source\test\org\apache\catalina\loader do not seem to run
> > successfully and are incomplete for all the protection that Tomcat provides 
> > for
> > classloader memory leaks.
> The tests that are present do work. Yes, code coverage of the tests is a long
> way from 100%.
> 
> > - For some categories of threadlocal memory leaks the key and value are not
> > displayed correctly in the warning messages; particularly ones dealing with
> > indirect references to threadlocals
> I don't see any changes to address this. Did I miss them?
> 
> > -  org.apache.catalina.loader.WebappClassLoader.loadedByThisOrChild(Object)
> > incorrectly traverses the object classloader hierarchy instead of the 
> > current
> > (this) classloader hierarchy
> No. The test is not "loaded by this or parent", it is "loaded by this or 
> child"
> and is correct.
> 
> I haven't looked at the tests yet. I'll commit the fixes (less the change to
> WebappClassLoader.loadedByThisOrChild(Object)) with the tests once I have
> reviewed the new tests.

(In reply to comment #5)
> (In reply to comment #0)
> > - Most of the detection and fixing code has been tested ONLY on Sun JVMs.
> Correct.
> 
> > - Tests in tomcat7.source\test\org\apache\catalina\loader do not seem to run
> > successfully and are incomplete for all the protection that Tomcat provides 
> > for
> > classloader memory leaks.
> The tests that are present do work. Yes, code coverage of the tests is a long
> way from 100%.
> 
> > - For some categories of threadlocal memory leaks the key and value are not
> > displayed correctly in the warning messages; particularly ones dealing with
> > indirect references to threadlocals
> I don't see any changes to address this. Did I miss them?
> 
> > -  org.apache.catalina.loader.WebappClassLoader.loadedByThisOrChild(Object)
> > incorrectly traverses the object classloader hierarchy instead of the 
> > current
> > (this) classloader hierarchy
> No. The test is not "loaded by this or parent", it is "loaded by this or 
> child"
> and is correct.
> 
> I haven't looked at the tests yet. I'll commit the fixes (less the change to
> WebappClassLoader.loadedByThisOrChild(Object)) with the tests once I have
> reviewed the new tests.

-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to