2010/3/12 <kkoli...@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-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
