Re: [jboss-dev] [jboss-cvs] JBossAS SVN: r94875 - in projects/jboss-cl/trunk: classloader/src/main/java/org/jboss/classloader/spi/filter and 7 other directories. - jboss-development

Wednesday, 14 October 2009

You're probably right, although I don't trust volatile. 
It's my c and pre Java5 background. ;-)

The issue flagged by FindBugs was that some access to
this ivar was synchronized and some wasn't.
The gets and sets were synchronized.

The real issue was code like the following
in an unsynchronized method:

if (loadedClass != null)
   doStuff();

blah();

if (loadClass == null)
   doSomethingElse();

Which won't work with volatile. The two if statements could
potentially see different values (although in practice
they won't in this use case).

However, I also fixed that code to copy the ivar into a local variable
at the start of the method so it has a consistent view
throughout, which should mean volatile is ok. At least with
the lastest java memory model. ;-)

On Wed, 2009-10-14 at 11:07 -0500, David M. Lloyd wrote:
...
 On 10/14/2009 11:01 AM, jboss-cvs-commits(a)lists.jboss.org wrote:
 > Author: adrian(a)jboss.org
 > Date: 2009-10-14 12:01:45 -0400 (Wed, 14 Oct 2009)
 > New Revision: 94875

 > Modified:
projects/jboss-cl/trunk/classloader/src/main/java/org/jboss/classloader/spi/base/ClassLoadingTask.java
 > ===================================================================
 > ---
projects/jboss-cl/trunk/classloader/src/main/java/org/jboss/classloader/spi/base/ClassLoadingTask.java	2009-10-14
15:23:40 UTC (rev 94874)
 > +++
projects/jboss-cl/trunk/classloader/src/main/java/org/jboss/classloader/spi/base/ClassLoadingTask.java	2009-10-14
16:01:45 UTC (rev 94875)
 > @@ -27,6 +27,7 @@
 >   import java.security.CodeSource;
 >   import java.security.PrivilegedAction;
 >   import java.security.ProtectionDomain;
 > +import java.util.concurrent.atomic.AtomicReference;
 [..]
 > +      return loadedClass.get();
 [..]
 > +      this.loadedClass.set(loadedClass);
 [..]
 > +      Class<?>  loadedClass = getLoadedClass();
 [..]
 > +      Class<?>  loadedClass = getLoadedClass();
 [..]
 > +         this.loadedClass.set(theClass);
 [..]
 > +         return loadedClass.get();
 [..]
 > +         Class<?>  loadedClass = getLoadedClass();

 If you only do get & set, might as well just make it volatile instead of 
 using AtomicReference, and save a bit of memory.

 - DML
 _______________________________________________
 jboss-cvs-commits mailing list
 jboss-cvs-commits(a)lists.jboss.org
 https://lists.jboss.org/mailman/listinfo/jboss-cvs-commits -- 
xxxxxxxxxxxxxxxxxxxxxxxxxxxx
Adrian Brock
Chief Scientist
JBoss by Red Hat
xxxxxxxxxxxxxxxxxxxxxxxxxxxx

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

Re: [jboss-dev] [jboss-cvs] JBossAS SVN: r94875 - in projects/jboss-cl/trunk: classloader/src/main/java/org/jboss/classloader/spi/filter and 7 other directories.