You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2007/12/10 23:24:41 UTC
svn commit: r603074 -
/tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
Author: markt
Date: Mon Dec 10 14:24:40 2007
New Revision: 603074
URL: http://svn.apache.org/viewvc?rev=603074&view=rev
Log:
Fix bug 44041. A small sync is required to prevent attempts to load the same class twice.
Modified:
tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
Modified: tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java?rev=603074&r1=603073&r2=603074&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java (original)
+++ tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java Mon Dec 10 14:24:40 2007
@@ -167,6 +167,11 @@
*/
boolean antiJARLocking = false;
+ /**
+ * Lock to prevent attempts to load duplicate classes from external
+ * repositories.
+ */
+ private Object lock = new Object();
// ----------------------------------------------------------- Constructors
@@ -883,7 +888,9 @@
}
if ((clazz == null) && hasExternalRepositories) {
try {
- clazz = super.findClass(name);
+ synchronized (lock) {
+ clazz = super.findClass(name);
+ }
} catch(AccessControlException ace) {
throw new ClassNotFoundException(name, ace);
} catch (RuntimeException e) {
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r603074 - /tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
Posted by Peter Rossbach <pr...@objektpark.de>.
+1 for that...
Peter
Am 11.12.2007 um 01:53 schrieb Filip Hanik - Dev Lists:
> if String objects are kept in a constants pool, then one would
>
> synchronized (name) {
> clazz = super.findClass(name);
> }
>
> to allow concurrent loading of different classes,
>
> Filip
>
> markt@apache.org wrote:
>> Author: markt
>> Date: Mon Dec 10 14:24:40 2007
>> New Revision: 603074
>>
>> URL: http://svn.apache.org/viewvc?rev=603074&view=rev
>> Log:
>> Fix bug 44041. A small sync is required to prevent attempts to
>> load the same class twice.
>>
>> Modified:
>> tomcat/trunk/java/org/apache/catalina/loader/
>> WebappClassLoader.java
>>
>> Modified: tomcat/trunk/java/org/apache/catalina/loader/
>> WebappClassLoader.java
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/
>> catalina/loader/WebappClassLoader.java?
>> rev=603074&r1=603073&r2=603074&view=diff
>> =====================================================================
>> =========
>> --- tomcat/trunk/java/org/apache/catalina/loader/
>> WebappClassLoader.java (original)
>> +++ tomcat/trunk/java/org/apache/catalina/loader/
>> WebappClassLoader.java Mon Dec 10 14:24:40 2007
>> @@ -167,6 +167,11 @@
>> */
>> boolean antiJARLocking = false; + /**
>> + * Lock to prevent attempts to load duplicate classes from
>> external
>> + * repositories.
>> + */
>> + private Object lock = new Object();
>> //
>> -----------------------------------------------------------
>> Constructors
>> @@ -883,7 +888,9 @@
>> }
>> if ((clazz == null) && hasExternalRepositories) {
>> try {
>> - clazz = super.findClass(name);
>> + synchronized (lock) {
>> + clazz = super.findClass(name);
>> + }
>> } catch(AccessControlException ace) {
>> throw new ClassNotFoundException(name, ace);
>> } catch (RuntimeException e) {
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>>
>>
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
Re: svn commit: r603074 - /tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
Posted by Mark Thomas <ma...@apache.org>.
Filip Hanik - Dev Lists wrote:
> that's a good point, one would expect most names to be pooled when it
> comes to implicit class loading, although as you say, not a guarantee.
> in findClassInternal, we do synchronized(this), so maybe we should not
> need to introduce another locking object, when we already have the
> instance itself.
You are right. I was trying to be clever and failing miserably ;). I'll
update the patch.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r603074 - /tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
Tim Funk wrote:
> But is name a string constant or is it built by using concatenation
> (via reading XML via digester or other)? If its the latter, then name
> is not a string constant.
that's a good point, one would expect most names to be pooled when it
comes to implicit class loading, although as you say, not a guarantee.
in findClassInternal, we do synchronized(this), so maybe we should not
need to introduce another locking object, when we already have the
instance itself.
Filip
>
> -Tim
>
> Filip Hanik - Dev Lists wrote:
>> if String objects are kept in a constants pool, then one would
>>
>> synchronized (name) {
>> clazz = super.findClass(name);
>> }
>>
>> to allow concurrent loading of different classes,
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r603074 - /tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
Posted by Tim Funk <fu...@joedog.org>.
But is name a string constant or is it built by using concatenation (via
reading XML via digester or other)? If its the latter, then name is not
a string constant.
-Tim
Filip Hanik - Dev Lists wrote:
> if String objects are kept in a constants pool, then one would
>
> synchronized (name) {
> clazz = super.findClass(name);
> }
>
> to allow concurrent loading of different classes,
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r603074 - /tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
Posted by Filip Hanik - Dev Lists <de...@hanik.com>.
if String objects are kept in a constants pool, then one would
synchronized (name) {
clazz = super.findClass(name);
}
to allow concurrent loading of different classes,
Filip
markt@apache.org wrote:
> Author: markt
> Date: Mon Dec 10 14:24:40 2007
> New Revision: 603074
>
> URL: http://svn.apache.org/viewvc?rev=603074&view=rev
> Log:
> Fix bug 44041. A small sync is required to prevent attempts to load the same class twice.
>
> Modified:
> tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
>
> Modified: tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java?rev=603074&r1=603073&r2=603074&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java (original)
> +++ tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoader.java Mon Dec 10 14:24:40 2007
> @@ -167,6 +167,11 @@
> */
> boolean antiJARLocking = false;
>
> + /**
> + * Lock to prevent attempts to load duplicate classes from external
> + * repositories.
> + */
> + private Object lock = new Object();
>
> // ----------------------------------------------------------- Constructors
>
> @@ -883,7 +888,9 @@
> }
> if ((clazz == null) && hasExternalRepositories) {
> try {
> - clazz = super.findClass(name);
> + synchronized (lock) {
> + clazz = super.findClass(name);
> + }
> } catch(AccessControlException ace) {
> throw new ClassNotFoundException(name, ace);
> } catch (RuntimeException e) {
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org