Re: [RESULT] (Was: [VOTE] C-T-R for any translation fixes)
On 03/17/2010 05:07 AM, Konstantin Kolinko wrote:
> Thank you for all your votes and feedback.
> I am tallying the results below.
>
>>> 1. Commit-Then-Review for any documentation, including JavaDoc and code
>>> comments.
>>
>> Already decided. No need to vote.
>>
>> 2. Allow C-T-R for changes to svn properties:
>>
>
> +1: Konstantin, Tim
> Mark: +1 for line endings, +0 for the rest
> 0: Rainer, Filip, Yoav
> -1: Mladen, Bill: because the proposal is too wide ("it can mean
> anything and nothing", and
> because for some of those changes "one needs a good reason")
>
> RESULT: Does not pass
>
>> 3. Allow C-T-R for any whitespace changes:
>> These are generally discouraged, but might be necessary to ease
>> backporting of patches.
>
> +1: Konstantin, Mladen, Rainer, Tim
> Mark: +1. Only if required to ease back-porting.
> -1: Filip: "makes tracing down when and how code changed a pain. It's
> not beneficial."
> 0: Yoav, Bill
>
> RESULT: Passes
>
>> 4. Allow C-T-R for trivial fixes to English messages that are in resource
>> files
>> and those that are inline in the code. This includes typos and rephrasing,
>> but does not include adding/removing message parameters.
>
> +1: Konstantin, Mladen, Rainer, Yoav, Tim, Mark
> 0: Filip, Bill
>
> RESULT: Passes
>
>> 5. Allow C-T-R for any fixes for non-English resource files.
>> The files must use 7-bit characters only. Other symbols must be
>> escaped with \u, as does native2ascii.
>
> +1: Konstantin, Mladen, Rainer, Yoav, Tim, Mark (providing that
> native2ascii has been used), Bill
> 0: Filip
>
> RESULT: Passes
>
>> 6. Require some indication in the commit message for code that usually is
>> covered by RTC, that this commit was done using C-T-R rule.
>
> +1: Mladen, Rainer, Bill
> +0: Mark: Putting CTR at the start works for me.
> 0: Konstantin, Filip, Yoav, Tim
>
> RESULT: Passes
>
>
> I did not count the votes by Henri Gomez (probably +1 to 2.,3.,4.,5.)
> and by Jean-Frederic Clere (looks like -0 to all),
Yep
Cheers
Jean-Frederic
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 48928] New: An alternative solution to preloading classes when running with SecurityManager
https://issues.apache.org/bugzilla/show_bug.cgi?id=48928 Summary: An alternative solution to preloading classes when running with SecurityManager Product: Tomcat 6 Version: 6.0.26 Platform: PC OS/Version: Windows XP Status: NEW Severity: enhancement Priority: P2 Component: Catalina AssignedTo: dev@tomcat.apache.org ReportedBy: knst.koli...@gmail.com Created an attachment (id=25140) --> (https://issues.apache.org/bugzilla/attachment.cgi?id=25140) 2010-03-17_tc6_CatalinaSecurityManager.patch While looking again at those AccessControlException exceptions that are cured by preloading classes in SecurityClassLoad classes of Catalina and Jasper, e.g. bug 48581, bug 48580, bug 48438, bug 48323 I thought of a solution that disables package access check in sun.misc.Launcher$AppClassLoader.loadClass() I am attaching the patch. The patch is for tc6.0.x. It implements a new class, o.a.c.security.CatalinaSecurityManager, modifies build.xml to pack it into bootstrap.jar, and modifies *.bat/*.sh scripts and SecurityClassLoad classes to make use of it. It allows to get rid of all preloading in SecurityClassLoad classes of catalina and jasper, except one call in Jasper that is still necessary. While package access protection in loadClass() is removed, I know that the package access protection is certainly still present in the following cases: - ClassLoader.defineClass() (e.g. bug 48218 still happens in 6.0) - When trying to use reflection, as implemented in java.lang.Class.checkMemberAccess() As of now, I do not see any harm in what this patch does. Comments are welcome. I tested it running Sun JRE 6u18 on Windows with Tomcat examples and other sample applications (changes to catalina.sh are not tested). All runs well, without AccessControlException (except the expected ones from ChatServlet - described in bug 48218). -- 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
DO NOT REPLY [Bug 48928] An alternative solution to preloading classes when running with SecurityManager
https://issues.apache.org/bugzilla/show_bug.cgi?id=48928 Konstantin Kolinko changed: What|Removed |Added Attachment #25140|0 |1 is obsolete|| --- Comment #1 from Konstantin Kolinko 2010-03-17 10:28:06 UTC --- Created an attachment (id=25141) --> (https://issues.apache.org/bugzilla/attachment.cgi?id=25141) 2010-03-17_tc6_CatalinaSecurityManager_v2.patch - Added check of the method name in CatalinaSecurityManager. - Added property read permission to catalina.policy. The org.apache.coyote.Constants class might fail to initialize without it. -- 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
svn commit: r924228 - /tomcat/tc6.0.x/trunk/STATUS.txt
Author: mturk Date: Wed Mar 17 11:32:52 2010 New Revision: 924228 URL: http://svn.apache.org/viewvc?rev=924228&view=rev Log: Proposal for using daemon 1.0.2 and its binaries Modified: tomcat/tc6.0.x/trunk/STATUS.txt Modified: tomcat/tc6.0.x/trunk/STATUS.txt URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=924228&r1=924227&r2=924228&view=diff == --- tomcat/tc6.0.x/trunk/STATUS.txt (original) +++ tomcat/tc6.0.x/trunk/STATUS.txt Wed Mar 17 11:32:52 2010 @@ -228,3 +228,10 @@ PATCHES PROPOSED TO BACKPORT: http://people.apache.org/~kkolinko/patches/2010-03-15_tc6_bug48903_v2.patch +1: kkolinko -1: + +* Backport using Commons Daemon 1.0.2 from trunk + Vote presumes removing daemon executables from our SVN. + http://people.apache.org/~mturk/tomcat/patches/tomcat-6.0.x-daemon-1.0.2.patch + +1: mturk + -1: + - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r924230 - /tomcat/tc5.5.x/trunk/STATUS.txt
Author: mturk Date: Wed Mar 17 11:34:12 2010 New Revision: 924230 URL: http://svn.apache.org/viewvc?rev=924230&view=rev Log: Proposal for using daemon 1.0.2 and its binaries Modified: tomcat/tc5.5.x/trunk/STATUS.txt Modified: tomcat/tc5.5.x/trunk/STATUS.txt URL: http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/STATUS.txt?rev=924230&r1=924229&r2=924230&view=diff == --- tomcat/tc5.5.x/trunk/STATUS.txt (original) +++ tomcat/tc5.5.x/trunk/STATUS.txt Wed Mar 17 11:34:12 2010 @@ -127,3 +127,10 @@ PATCHES PROPOSED TO BACKPORT: http://people.apache.org/~kkolinko/patches/2010-03-11_tc55_commons-daemon.patch +1: kkolinko -1: + +* Backport using Commons Daemon 1.0.2 from trunk + Vote presumes removing daemon executables from our SVN. + http://people.apache.org/~mturk/tomcat/patches/tomcat-6.0.x-daemon-1.0.2.patch + +1: mturk + -1: + - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r924231 - /tomcat/tc5.5.x/trunk/STATUS.txt
Author: mturk Date: Wed Mar 17 11:35:10 2010 New Revision: 924231 URL: http://svn.apache.org/viewvc?rev=924231&view=rev Log: Oops. Copy/paste from 6.0 proposal Modified: tomcat/tc5.5.x/trunk/STATUS.txt Modified: tomcat/tc5.5.x/trunk/STATUS.txt URL: http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/STATUS.txt?rev=924231&r1=924230&r2=924231&view=diff == --- tomcat/tc5.5.x/trunk/STATUS.txt (original) +++ tomcat/tc5.5.x/trunk/STATUS.txt Wed Mar 17 11:35:10 2010 @@ -130,7 +130,7 @@ PATCHES PROPOSED TO BACKPORT: * Backport using Commons Daemon 1.0.2 from trunk Vote presumes removing daemon executables from our SVN. - http://people.apache.org/~mturk/tomcat/patches/tomcat-6.0.x-daemon-1.0.2.patch + http://people.apache.org/~mturk/tomcat/patches/tomcat-5.0.x-daemon-1.0.2.patch +1: mturk -1: - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: svn commit: r924228 - /tomcat/tc6.0.x/trunk/STATUS.txt
2010/3/17 : > Author: mturk > Date: Wed Mar 17 11:32:52 2010 > New Revision: 924228 > > URL: http://svn.apache.org/viewvc?rev=924228&view=rev > Log: > Proposal for using daemon 1.0.2 and its binaries > > Modified: >tomcat/tc6.0.x/trunk/STATUS.txt Note, that my proposal for this issue is already there, since 2010-03-11. Best regards, Konstantin Kolinko - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r924240 - /tomcat/tc5.5.x/trunk/STATUS.txt
Author: mturk Date: Wed Mar 17 11:48:32 2010 New Revision: 924240 URL: http://svn.apache.org/viewvc?rev=924240&view=rev Log: -1 Konstantin daemon 1.0.2 patch cause it contains extra sections removed unrelated to the patch itself. Did I said I love the STATUS.txt file ;) Modified: tomcat/tc5.5.x/trunk/STATUS.txt Modified: tomcat/tc5.5.x/trunk/STATUS.txt URL: http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/STATUS.txt?rev=924240&r1=924239&r2=924240&view=diff == --- tomcat/tc5.5.x/trunk/STATUS.txt (original) +++ tomcat/tc5.5.x/trunk/STATUS.txt Wed Mar 17 11:48:32 2010 @@ -126,7 +126,10 @@ PATCHES PROPOSED TO BACKPORT: 2) http://people.apache.org/~kkolinko/patches/2010-03-11_tc55_commons-daemon.patch +1: kkolinko - -1: + -1: + mturk - Your patch contains a lot more then it claims. Proposed an + alternative that does update only new commons-daemon dist binaries + layout. * Backport using Commons Daemon 1.0.2 from trunk Vote presumes removing daemon executables from our SVN. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r924248 - /tomcat/tc6.0.x/trunk/STATUS.txt
Author: mturk Date: Wed Mar 17 12:06:28 2010 New Revision: 924248 URL: http://svn.apache.org/viewvc?rev=924248&view=rev Log: Revoke my proposal and use Konstantin's. Did I said I love the STATUS.txt file ;) Modified: tomcat/tc6.0.x/trunk/STATUS.txt Modified: tomcat/tc6.0.x/trunk/STATUS.txt URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=924248&r1=924247&r2=924248&view=diff == --- tomcat/tc6.0.x/trunk/STATUS.txt (original) +++ tomcat/tc6.0.x/trunk/STATUS.txt Wed Mar 17 12:06:28 2010 @@ -190,7 +190,7 @@ PATCHES PROPOSED TO BACKPORT: svn delete res/procrun 2) http://people.apache.org/~kkolinko/patches/2010-03-11_tc6_commons-daemon.patch - +1: kkolinko + +1: kkolinko, mturk -1: * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48795 @@ -229,9 +229,3 @@ PATCHES PROPOSED TO BACKPORT: +1: kkolinko -1: -* Backport using Commons Daemon 1.0.2 from trunk - Vote presumes removing daemon executables from our SVN. - http://people.apache.org/~mturk/tomcat/patches/tomcat-6.0.x-daemon-1.0.2.patch - +1: mturk - -1: - - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r924250 - /tomcat/tc5.5.x/trunk/STATUS.txt
Author: mturk Date: Wed Mar 17 12:10:49 2010 New Revision: 924250 URL: http://svn.apache.org/viewvc?rev=924250&view=rev Log: Use the correct patch file in STATUS.txt. Modified: tomcat/tc5.5.x/trunk/STATUS.txt Modified: tomcat/tc5.5.x/trunk/STATUS.txt URL: http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/STATUS.txt?rev=924250&r1=924249&r2=924250&view=diff == --- tomcat/tc5.5.x/trunk/STATUS.txt (original) +++ tomcat/tc5.5.x/trunk/STATUS.txt Wed Mar 17 12:10:49 2010 @@ -133,7 +133,7 @@ PATCHES PROPOSED TO BACKPORT: * Backport using Commons Daemon 1.0.2 from trunk Vote presumes removing daemon executables from our SVN. - http://people.apache.org/~mturk/tomcat/patches/tomcat-5.0.x-daemon-1.0.2.patch + http://people.apache.org/~mturk/tomcat/patches/tomcat-5.5.x-daemon-1.0.2.patch +1: mturk -1: - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: svn commit: r924228 - /tomcat/tc6.0.x/trunk/STATUS.txt
On 03/17/2010 12:39 PM, Konstantin Kolinko wrote: Log: Proposal for using daemon 1.0.2 and its binaries Modified: tomcat/tc6.0.x/trunk/STATUS.txt Note, that my proposal for this issue is already there, since 2010-03-11. LOL. Didn't saw the 6.0 proposal. It could save me couple of hours. Hate that STATUS.txt BS. Regards -- ^TM - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r924269 - /tomcat/tc5.5.x/trunk/STATUS.txt
Author: kkolinko
Date: Wed Mar 17 13:13:02 2010
New Revision: 924269
URL: http://svn.apache.org/viewvc?rev=924269&view=rev
Log:
update
Modified:
tomcat/tc5.5.x/trunk/STATUS.txt
Modified: tomcat/tc5.5.x/trunk/STATUS.txt
URL:
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/STATUS.txt?rev=924269&r1=924268&r2=924269&view=diff
==
--- tomcat/tc5.5.x/trunk/STATUS.txt (original)
+++ tomcat/tc5.5.x/trunk/STATUS.txt Wed Mar 17 13:13:02 2010
@@ -124,16 +124,20 @@ PATCHES PROPOSED TO BACKPORT:
1)
svn delete connectors/procrun
2)
-
http://people.apache.org/~kkolinko/patches/2010-03-11_tc55_commons-daemon.patch
+
http://people.apache.org/~kkolinko/patches/2010-03-17_tc55_commons-daemon.patch
+1: kkolinko
-1:
mturk - Your patch contains a lot more then it claims. Proposed an
alternative that does update only new commons-daemon dist
binaries
layout.
+ kkolinko - I updated the patch to do not delete "deploy.old" target.
+ Regarding "deploy.old": Note, that the commons-daemon files that
were
+ copied in that target are already copied by "deploy-static", and
that
+ I see no reason why one of the files was copied to
${tomcat.dist},
+ as that target updates ${tomcat.build} only.
* Backport using Commons Daemon 1.0.2 from trunk
Vote presumes removing daemon executables from our SVN.
http://people.apache.org/~mturk/tomcat/patches/tomcat-5.5.x-daemon-1.0.2.patch
+1: mturk
- -1:
-
+ -1: kkolinko: ${tomcat.dsit}
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
svn commit: r924289 - /tomcat/tc5.5.x/trunk/STATUS.txt
Author: mturk
Date: Wed Mar 17 13:39:35 2010
New Revision: 924289
URL: http://svn.apache.org/viewvc?rev=924289&view=rev
Log:
Vote on Konstantin patch and revoke my proposal, cause they are now the same
Modified:
tomcat/tc5.5.x/trunk/STATUS.txt
Modified: tomcat/tc5.5.x/trunk/STATUS.txt
URL:
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/STATUS.txt?rev=924289&r1=924288&r2=924289&view=diff
==
--- tomcat/tc5.5.x/trunk/STATUS.txt (original)
+++ tomcat/tc5.5.x/trunk/STATUS.txt Wed Mar 17 13:39:35 2010
@@ -125,19 +125,11 @@ PATCHES PROPOSED TO BACKPORT:
svn delete connectors/procrun
2)
http://people.apache.org/~kkolinko/patches/2010-03-17_tc55_commons-daemon.patch
- +1: kkolinko
+ +1: kkolinko, mturk
-1:
- mturk - Your patch contains a lot more then it claims. Proposed an
- alternative that does update only new commons-daemon dist
binaries
- layout.
kkolinko - I updated the patch to do not delete "deploy.old" target.
Regarding "deploy.old": Note, that the commons-daemon files that
were
copied in that target are already copied by "deploy-static", and
that
I see no reason why one of the files was copied to
${tomcat.dist},
as that target updates ${tomcat.build} only.
-* Backport using Commons Daemon 1.0.2 from trunk
- Vote presumes removing daemon executables from our SVN.
-
http://people.apache.org/~mturk/tomcat/patches/tomcat-5.5.x-daemon-1.0.2.patch
- +1: mturk
- -1: kkolinko: ${tomcat.dsit}
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: svn commit: r924269 - /tomcat/tc5.5.x/trunk/STATUS.txt
On 03/17/2010 02:13 PM, kkoli...@apache.org wrote:
Author: kkolinko
+ kkolinko - I updated the patch to do not delete "deploy.old" target.
+ Regarding "deploy.old": Note, that the commons-daemon files that
were
+ copied in that target are already copied by "deploy-static", and
that
+ I see no reason why one of the files was copied to
${tomcat.dist},
+ as that target updates ${tomcat.build} only.
Agreed.
Think another patch should be made to cleanup the build.xml
There are indeed huge amount of redundancy and leftovers from
who knows when.
Regards
--
^TM
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
DO NOT REPLY [Bug 48928] An alternative solution to preloading classes when running with SecurityManager
https://issues.apache.org/bugzilla/show_bug.cgi?id=48928 --- Comment #2 from Sebb 2010-03-17 17:05:47 UTC --- This will also affect any instructions for running Tomcat as a service with security enabled. Not sure where this is documented - if anywhere. There is a command-file for installing Tomcat as a service (bin/service.bat) but this does not currently support enabling the security manager. -- 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
[Tomcat Wiki] Update of "MemoryLeakProtection" by Sylva inLaurent
Dear Wiki user,
You have subscribed to a wiki page or wiki category on "Tomcat Wiki" for change
notification.
The "MemoryLeakProtection" page has been changed by SylvainLaurent.
http://wiki.apache.org/tomcat/MemoryLeakProtection?action=diff&rev1=3&rev2=4
--
The leak is caused because we have a custom class for the {{{ThreadLocal}}}
instance, and also a custom class for the value bound to the Thread. Actually
the important thing is that both classes were loaded by the webapp classloader.
- Hopefully tomcat 6.0.24 can detect the leak when the application is stopped:
each Thread in the JVM is examined, and the internal structures of the Thread
and {{{ThreadLocal}}} classes are introspected to see if either the
{{{ThreadLocal}}} instance of the value bound to it were loaded by the
{{{WebAppClassLoader}}} of the application being stopped.
+ Hopefully tomcat 6.0.24 can detect the leak when the application is stopped:
each Thread in the JVM is examined, and the internal structures of the Thread
and {{{ThreadLocal}}} classes are introspected to see if either the
{{{ThreadLocal}}} instance or the value bound to it were loaded by the
{{{WebAppClassLoader}}} of the application being stopped.
In this particular case, the leak is detected, a message is logged and
internal structures of the JDK ({{{ThreadLocalMap}}}) are modified to remove
the reference to the {{{ThreadLocal}}} instance.
@@ -78, +78 @@
=== Webapp class instance as ThreadLocal value ===
+ Suppose that we have the following class in the common classpath (for
instance in a jar in tomcat/lib) :
+ {{{
+ public class ThreadScopedHolder {
+ private final static ThreadLocal threadLocal = new
ThreadLocal();
+
+ public static void saveInHolder(Object o) {
+ threadLocal.set(o);
+ }
+
+ public static Object getFromHolder() {
+ return threadLocal.get();
+ }
+ }
+ }}}
+
+ And those 2 classes in the webapp :
+ {{{
+ public class MyCounter {
+ private int count = 0;
+
+ public void increment() {
+ count++;
+ }
+
+ public int getCount() {
+ return count;
+ }
+ }
+ public class LeakingServlet extends HttpServlet {
+
+ protected void doGet(HttpServletRequest request,
+ HttpServletResponse response) throws ServletException,
IOException {
+
+ MyCounter counter =
(MyCounter)ThreadScopedHolder.getFromHolder();
+ if (counter == null) {
+ counter = new MyCounter();
+ ThreadScopedHolder.saveInHolder(counter);
+ }
+
+ response.getWriter().println(
+ "The current thread served this servlet " +
counter.getCount()
+ + " times");
+ counter.increment();
+ }
+ }
+ }}}
+
+ If the servlet is invoked at least once, the webapp classloader would not be
GCed when the app is stopped: since the classloader of {{{ThreadScopedHolder}}}
is the common classloader, it remains forever which is as expected. But its
{{{ThreadLocal}}} instance has a value bound to it (for the non-terminated
thread(s) that served the sevlet), which is an instance of a class loaded by
the webapp classloader...
+
+ Here again, tomcat 6.0.24 will detect and fix the leak by removing this
{{{ThreadLocal}}} reference from each Thread.
+ {{{
+ Mar 17, 2010 10:23:13 PM org.apache.catalina.loader.WebappClassLoader
clearThreadLocalMap
+ SEVERE: A web application created a ThreadLocal with key of type
[java.lang.ThreadLocal] (value [java.lang.threadlo...@44676e3f]) and a value of
type [test.leak.threadlocal.value.MyCounter] (value
[test.leak.threadlocal.value.mycoun...@62770d2e]) but failed to remove it when
the web application was stopped. To prevent a memory leak, the ThreadLocal has
been forcibly removed.
+ }}}
+
+
=== Webapp class instance indirectly held through a ThreadLocal value ===
+ Suppose we have the same {{{ThreadScopedHolder}}} class (in the common
classloader) and {{{MyCounter}}} class in the webapp, but with the following
servlet :
+ {{{
+ public class LeakingServlet extends HttpServlet {
+
+ protected void doGet(HttpServletRequest request,
+ HttpServletResponse response) throws ServletException,
IOException {
+
+ List counterList = (List)
ThreadScopedHolder
+ .getFromHolder();
+ MyCounter counter;
+ if (counterList == null) {
+ counter = new MyCounter();
+ ThreadScopedHolder.saveInHolder(Arrays.asList(counter));
+ } else {
+ counter = counterList.get(0);
+ }
+
+ response.getWriter().println(
+ "The current thread served t
[Tomcat Wiki] Update of "MemoryLeakProtection" by Sylva inLaurent
Dear Wiki user,
You have subscribed to a wiki page or wiki category on "Tomcat Wiki" for change
notification.
The "MemoryLeakProtection" page has been changed by SylvainLaurent.
http://wiki.apache.org/tomcat/MemoryLeakProtection?action=diff&rev1=4&rev2=5
--
== ThreadLocal pseudo-leak ==
+ Suppose we have the same {{{MyCounter}}} class as above (in the webapp) and
the following servlet :
+ {{{
+ public class LeakingServlet extends HttpServlet {
+ private ThreadLocal myThreadLocal = new
ThreadLocal();
+
+ protected void doGet(HttpServletRequest request,
+ HttpServletResponse response) throws ServletException,
IOException {
+
+ MyCounter counter = myThreadLocal.get();
+ if (counter == null) {
+ counter = new MyCounter();
+ myThreadLocal.set(counter);
+ }
+
+ response.getWriter().println(
+ "The current thread served this servlet " +
counter.getCount()
+ + " times");
+ counter.increment();
+ }
+
+ @Override
+ public void destroy() {
+ super.destroy();
+ // normally not needed, just to make my point
+ myThreadLocal = null;
+ }
+ }
+ }}}
+
+ Notice that the {{{ThreadLocal}}} instance is referenced through an instance
variable, not a static one.
+
+ Sun's implementation of {{{ThreadLocal}}} (and {{{WeakHashMap}}}) too is such
that {{{ThreadLocalMap}}} entries whose key is GCed are not immediately removed.
+ (The key is a weak reference to the {{{ThreadLocal}}} instance, see
{{{java.lang.ThreadLocal.ThreadLocalMap.Entry in JDK 5/6}}}. And there's no
daemon thread waiting on a {{{ReferenceQueue}}}). Instead, it's only during
subsequent uses of {{{ThreadLocal}}} features that each Thread removes the
abandoned {{{ThreadLocalMap.Entry}}} entries (see
{{{ThreadLocalMap.expungeStaleEntries()}}}.
+
+ If many threads were used to serve our leaking webapp, but after we stop it
only a couple of threads are enough to serve other webapps, one could have some
threads that are no longer used, waiting for some work. Since those threads are
blocked, they have no interaction with their {{{ThreadLocalMap}}} (i.e. there's
no {{{ThreadLocal}}} value bound to them or removed), so that there's no
opportunity to {{{expungeStaleEntries()}}}.
+
+ Tomcat 6.0.24 "speeds up" the removal of stale entries (and thus fixes the
pseudo-leak), by calling {{{expungeStaleEntries()}}} for each thread that has
some stale entries.
+
== Threads ContextClassLoader ==
=== Threads spawned by JRE classes ===
=== Threads spawned by classes loaded by the common classloader ===
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
