https://issues.apache.org/bugzilla/show_bug.cgi?id=51195
--- Comment #8 from Joern Huxhorn <jhuxh...@googlemail.com> 2011-05-24 14:35:17 UTC --- (In reply to comment #7) > Joern, > (In reply to comment #6) > > The cleanup performed in the code is merely to prevent a *false* memory leak > > warning. A real leak does, in fact, not exist. This is the reason why I put > > it > > directly before the execution of System.gc() in the "Find Leaks" function. > > One could argue that a memory leak does, in fact, exist: just because the > references will be /eventually/ collected doesn't mean that they won't loiter > for quite a long time... unless the GC is getting seriously low on memory, > those objects will stick in memory, and so, likely, will the > WebappClassLoader, > and all of the java.lang.Class objects it manages. > > I agree with Mark that this feature is best placed into WebappClassLoader and > run during it's disposal sequence. Alright, I agree that it's better to get rid of a WebappClassLoader as soon as possible instead of waiting for a hard gc caused by low memory. > > Since "Find Leaks" isn't supposed to be executed in a production > > environment, I > > don't think that any further optimization (suggestion in 2.) makes sense. > > If this will be used in WebappClassLoader, such optimizations should obviously > be made. It's actually pretty hard to implement those optimizations. The keys used in those maps are two static internal classes of ObjectStreamClass. FieldReflectorKey is private and WeakClassKey is package-private. Performing a selective cleanup would involve serious complexity an reflection magic, including lots of assumptions about internal behavior in the hope that it won't change... I'd leave it like it is right now to prevent errors caused by this. My current implementation will simply do nothing if localDescs & reflectors are not available or not instances of class ConcurrentHashMap. I don't think that the performance impact is a big thing but I haven't done any benchmarks, either. You should remember that this is exactly what would happen naturally if the memory of the JVM is running low. We are just getting rid of SoftReferences. This would be a problem if performed continuously but doing so once during undeploy shouldn't interfere with other webapps that much. > > It > > would probably make sense if this code would indeed be added to the normal > > undeploy procedure but I really don't think that this is necessary or > > advisable. > > This could be implemented as a utility method accessible to both pieces of > code, and called directly during a "find leaks" operation... I would have to > look at the "find leaks" feature to see if that would even be necessary... my > sense is that the WCL is disposed before the leaks are detected, so I don't > think any changes to the manager app would need to be made. I don't have enough know-how about the way Tomcat functionality is structured. So I just assume you are right. The manager app would likely stay untouched since "Find leaks" simply checks if the WebappClassLoader has been unloaded properly, if I remember correctly. -- 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
