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

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 Labels: patch 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.