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.
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