You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by kk...@apache.org on 2010/03/12 16:41:55 UTC
svn commit: r922300 - in /tomcat: tc5.5.x/trunk/STATUS.txt
tc6.0.x/trunk/STATUS.txt
Author: kkolinko
Date: Fri Mar 12 15:41:55 2010
New Revision: 922300
URL: http://svn.apache.org/viewvc?rev=922300&view=rev
Log:
proposal
Modified:
tomcat/tc5.5.x/trunk/STATUS.txt
tomcat/tc6.0.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=922300&r1=922299&r2=922300&view=diff
==============================================================================
--- tomcat/tc5.5.x/trunk/STATUS.txt (original)
+++ tomcat/tc5.5.x/trunk/STATUS.txt Fri Mar 12 15:41:55 2010
@@ -56,6 +56,13 @@ PATCHES PROPOSED TO BACKPORT:
+1: markt
-1:
+ The same as above, but
+ - moves resolveClass() call outside the sync block
+ - does not access entry.loadedClass unless we are in a sync block
+ http://people.apache.org/~kkolinko/patches/2010-03-12_tc55_bug44041.patch
+ +1: kkolinko
+ -1:
+
* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=47878
Return 404's rather than a permanent 500 if a JSP is deleted
Make sure first response port deletion is correct
Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=922300&r1=922299&r2=922300&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Mar 12 15:41:55 2010
@@ -195,3 +195,15 @@ PATCHES PROPOSED TO BACKPORT:
http://svn.apache.org/viewvc?rev=922010&view=rev
+1: markt
-1:
+
+* Followup to BZ 44041/48694 patches
+ - moves resolveClass() call outside the sync block
+ - does not access entry.loadedClass unless we are in a sync block
+ This is proposed as a part of alternative BZ 44041 patch for TC 5.5
+ FIXME: have not yet applied it to trunk
+ http://people.apache.org/~kkolinko/patches/2010-03-12_tc6_bug44041.patch
+ (svn diff -x -w variant of it, ignoring whitespace changes:
+ http://people.apache.org/~kkolinko/patches/2010-03-12_tc6_bug44041_x_w.patch
+ )
+ +1: kkolinko
+ -1:
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r922300 - in /tomcat: tc5.5.x/trunk/STATUS.txt
tc6.0.x/trunk/STATUS.txt
Posted by Konstantin Kolinko <kk...@apache.org>.
2010/3/12 Konstantin Kolinko <kk...@apache.org>:
> 2010/3/12 <kk...@apache.org>:
>> Author: kkolinko
>> Date: Fri Mar 12 15:41:55 2010
>> New Revision: 922300
>>
>> URL: http://svn.apache.org/viewvc?rev=922300&view=rev
>> Log:
>> proposal
>>
>
> My concern, leading to the above proposal, is that
> ClassLoader.resolveClass() should perform "Linking" of the class and
> JLS says that that can incur loading of the dependent classes [1],
> thus I think it could lead to a deadlock.
>
>From thread dump that I attached to
https://issues.apache.org/bugzilla/show_bug.cgi?id=48903
1. It looks that it is ClassLoader.defineClass() call that performs
linking and loading of related classes in Sun JRE 6u18
2. Certain methods e.g. ClassLoader.checkCerts(), require a lock on
the whole ClassLoader instance.
3. I do not understand what holds a lock on the ClassLoader instance
in "http-8080-1" thread. Can it be inside the native method
(ClassLoader.defineClass1()) ??
4. The above 2.+3. may mean that we need sync() on the classloader as
a whole. That is, the sync(name.intern()) trick fails.
More comments to the issue 48903 are welcome.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r922300 - in /tomcat: tc5.5.x/trunk/STATUS.txt
tc6.0.x/trunk/STATUS.txt
Posted by Konstantin Kolinko <kk...@apache.org>.
2010/3/12 <kk...@apache.org>:
> Author: kkolinko
> Date: Fri Mar 12 15:41:55 2010
> New Revision: 922300
>
> URL: http://svn.apache.org/viewvc?rev=922300&view=rev
> Log:
> proposal
>
> Modified:
> tomcat/tc5.5.x/trunk/STATUS.txt
> tomcat/tc6.0.x/trunk/STATUS.txt
>
> Modified: tomcat/tc5.5.x/trunk/STATUS.txt
> + The same as above, but
> + - moves resolveClass() call outside the sync block
> + - does not access entry.loadedClass unless we are in a sync block
> + http://people.apache.org/~kkolinko/patches/2010-03-12_tc55_bug44041.patch
> + +1: kkolinko
> + -1:
> +
>
> Modified: tomcat/tc6.0.x/trunk/STATUS.txt
> +
> +* Followup to BZ 44041/48694 patches
> + - moves resolveClass() call outside the sync block
> + - does not access entry.loadedClass unless we are in a sync block
> + This is proposed as a part of alternative BZ 44041 patch for TC 5.5
> + FIXME: have not yet applied it to trunk
> + http://people.apache.org/~kkolinko/patches/2010-03-12_tc6_bug44041.patch
> + (svn diff -x -w variant of it, ignoring whitespace changes:
> + http://people.apache.org/~kkolinko/patches/2010-03-12_tc6_bug44041_x_w.patch
> + )
> + +1: kkolinko
> + -1:
>
My concern, leading to the above proposal, is that
ClassLoader.resolveClass() should perform "Linking" of the class and
JLS says that that can incur loading of the dependent classes [1],
thus I think it could lead to a deadlock.
That said,
1. I suspect that recent JVMs do linking lazily, so resolveClass()
does not actually load the dependencies. But I want to prevent the
possibility of a deadlock.
2. I think that in the loadClass(name,boolean) call the resolve
argument is usually "false".
That is because ClassLoader.loadClass(name) calls it with "false" value, and
ClassLoader.loadClass(name, resolve) is itself a protected method.
Unless there are calls with resolve=true, there is no difference in
whether we do care about resolveClass() here.
3. I have some worry about whether ClassLoader.resolveClass() method
is thread safe, as I see no explicit statement about it.
JLS says that the class initialization is thread safe [2],
but nothing explicit is said about linking, and nothing is added in
the JavaDoc for resolveClass() method.
My understanding is that at most times resolveClass() is called
implicitly by JVM before initialization of the class takes place, so
JVM takes care of synchronization somewhere, and it will be a JRE
issue if it is not.
Again, we will not hit it unless resolve=true in the loadClass() call.
About the proposed change with "entry.loadedClass" in findClassInternal():
- The loadedClass field would need to be volatile to avoid broken
double-checked locking pattern here, but the change that I am
proposing also fixes it.
Any comments?
[1] http://java.sun.com/docs/books/jls/third_edition/html/execution.html#44487
ch.12.3
[2] http://java.sun.com/docs/books/jls/third_edition/html/execution.html#44557
ch.12.4
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org