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

[Yu Lin (JIRA)]

     [ https://issues.jboss.org/browse/JBAOP-813?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Yu Lin updated JBAOP-813:
-------------------------

    Attachment: jboss-aop-2.2.2.patch


The patch that fixes the problem.
                
> 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.

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