i havent reviewed the code, but the findbugs rule here may be misleading. it is possible that someone is using the "classes" variable as the lock object to synchornize code that has nothing to do with what the "classes" variable actually is. just because it happens to be a concurrent collection doesnt make it any less of a candidate to synchronize on. so with that in mind review your change and make sure it still makes sense.
-igor On Sat, Oct 23, 2010 at 9:57 AM, <[email protected]> wrote: > Author: mgrigorov > Date: Sat Oct 23 16:57:04 2010 > New Revision: 1026650 > > URL: http://svn.apache.org/viewvc?rev=1026650&view=rev > Log: > Findbugs warnings: > .../wicket/src/main/java/org/apache/wicket/pageStore/AsynchronousDataStore.java:285 > Synchronization performed on java.util.concurrent.ConcurrentHashMap > > Remove not needed synchronization: > 1) the synchronized object is concurrent > 2) the #put() actually is out of the synchronized block > > Modified: > > wicket/trunk/wicket/src/main/java/org/apache/wicket/application/DefaultClassResolver.java > > Modified: > wicket/trunk/wicket/src/main/java/org/apache/wicket/application/DefaultClassResolver.java > URL: > http://svn.apache.org/viewvc/wicket/trunk/wicket/src/main/java/org/apache/wicket/application/DefaultClassResolver.java?rev=1026650&r1=1026649&r2=1026650&view=diff > ============================================================================== > --- > wicket/trunk/wicket/src/main/java/org/apache/wicket/application/DefaultClassResolver.java > (original) > +++ > wicket/trunk/wicket/src/main/java/org/apache/wicket/application/DefaultClassResolver.java > Sat Oct 23 16:57:04 2010 > @@ -101,21 +101,19 @@ public final class DefaultClassResolver > } > else > { > - synchronized (classes) > + ClassLoader loader = > Thread.currentThread().getContextClassLoader(); > + if (loader == null) > { > - ClassLoader loader = > Thread.currentThread().getContextClassLoader(); > - if (loader == null) > - { > - loader = > DefaultClassResolver.class.getClassLoader(); > - } > - // see > http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6500212 > - // clazz = > loader.loadClass(classname); > - clazz = Class.forName(classname, > false, loader); > - if (clazz == null) > - { > - throw new > ClassNotFoundException(classname); > - } > + loader = > DefaultClassResolver.class.getClassLoader(); > } > + // see > http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6500212 > + // clazz = loader.loadClass(classname); > + clazz = Class.forName(classname, false, > loader); > + if (clazz == null) > + { > + throw new > ClassNotFoundException(classname); > + } > + > classes.put(classname, new > WeakReference<Class<?>>(clazz)); > } > } > > >
