[jboss-jira] [JBoss JIRA] (JBAOP-813) atomicity violations because of misusing concurrent collections

[Yu Lin (JIRA)]

Yu Lin created JBAOP-813:
----------------------------

             Summary: atomicity violations because of misusing concurrent collections
                 Key: JBAOP-813
                 URL: https://issues.jboss.org/browse/JBAOP-813
             Project: JBoss AOP
          Issue Type: Bug
      Security Level: Public (Everyone can see)
    Affects Versions: 2.2.2.GA
            Reporter: Yu Lin
         Attachments: jboss-aop-2.2.2.patch

My name is Yu Lin. I'm a Ph.D. student in the CS department at
UIUC. I'm currently doing research on mining Java concurrent library
misusages. I found some misusages of ConcurrentHashMap in JBossAOP
2.2.2, which may result in potential atomicity violation bugs or harm
the performance.

The code below is a snapshot of the code in file
aop/src/main/java/org/jboss/aop/Advisor.java from line 1217 to 1225

L1217     Set<Joinpoint> joinpoints = perInstanceJoinpointAspectDefinitions.get(def);
L1218     if (joinpoints == null)
L1219     {
L1220          joinpoints = new ConcurrentSet<Joinpoint>();
L1221          initPerInstanceJoinpointAspectDefinitionsMap();
L1222          perInstanceJoinpointAspectDefinitions.put(def, joinpoints);
L1223          def.registerAdvisor(this);
L1224     }
L1225     joinpoints.add(joinpoint);

In the code above, an atomicity violation may occur between lines
<1219 and 1222>. Suppose a thread T1 executes line 1217 and finds out
the concurrent hashmap does not contain the key "def". Before it gets
to execute line 1222, another thread T2 puts a pair <def, v> in the
concurrent hashmap "perInstanceJoinpointAspectDefinitions". Now thread
T1 resumes execution and it will overwrite the value written by thread
T2. Thus, the code no longer preserves the "put-if-absent"
semantics. We can use "putIfAbsent" method at line 1222 rather than
"put" to preserve the "put-if-absent" semantics. Also, the same
problem exists at lines 1235 and 1279 of this file. (I attach a patch
that can fix the problem)

I also found such problem in other files:

In aop/src/main/java/org/jboss/aop/GeneratedClassAdvisor.java, lines
910, 918 and 922: after using "putIfAbsent" at these lines, I think we
can remove the "synchronized" key word on method
"addPerClassJoinpointAspect".

In aop/src/main/java/org/jboss/aop/annotation/AnnotationRepository.java, 
lines 146, 159, 229, 242, 285.

In aop/src/main/java/org/jboss/aop/instrument/WeavingRegistry.java, line 46.

In aop/src/main/java/org/jboss/aop/metadata/MethodMetaData.java, line
81: after using "putIfAbsent" at this line, we may remove the
synchronization on method "addMethodMetaData".

In
asintegration-mc/src/main/java/org/jboss/aop/asintegration/jboss5/ScopedVFSClassLoaderDomain.java,
lines 101 and 118.

In
asintegration-jmx/src/main/java/org/jboss/aop/domain/ScopedRepositoryClassLoaderDomain.java,
lines 73 and 91, here I'm not sure after using "putIfAbsent", whether
can we remove the synchronization on map "myPerVMAspects" at line 66.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira