Uploaded image for project: 'OpenEJB'
  1. OpenEJB
  2. OPENEJB-1880

atomicity violation bugs because of misusing concurrent collections

    Details

    • Type: Bug
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: 4.0.0
    • Fix Version/s: None
    • Component/s: None
    • Labels:

      Description

      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 OpenEJB
      4.0.0, which may result in potential atomicity violation bugs or harm
      the performance.

      The code below is a snapshot of the code in file
      container/openejb-core/src/main/java/org/apache/openejb/core/managed/ManagedContainer.java
      from line 615 to 635

      L615 Instance instance = checkedOutInstances.get(primaryKey);
      L616 if (instance == null) {
      L617 try

      { L618 instance = cache.checkOut(primaryKey); L619 }


      ...

      L630 // remember instance until it is returned to the cache
      L631 checkedOutInstances.put(primaryKey, instance);
      L632 }

      L635 synchronized (instance) {

      In the code above, suppose a thread T1 executes line 615 and finds out
      the concurrent hashmap "checkedOutInstances" does not contain the key
      "primaryKey". Before it gets to execute line 631, another thread T2
      puts a pair <primaryKey, v> in the concurrent hashmap
      "checkedOutInstances". 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. Because "instance" object is
      used in the subsequent code, the code may use an "instance" that is
      not in the map, which is incorrect (I prepare a patch to fix the
      problem).

      I found some similar misusages in other files:

      In QueryProxy.java at line 203 and 366. In
      SocketConnectionFactory.java at line 166. In Client.java at line
      423. In Tracker.java at line 148. In StatefulContainer.java at line
      691, can we remove the "synchronized" at line 674 if we use
      "putIfAbsent" method at line 691? In StatsInterceptor.java at line
      197, if we use "putIfAbsent" method, can we remove the synchronization
      at line 193? In DynamicMBeanHandler.java at line 149 if using
      "putIfAbsent", the synchronization may also be removed. In
      DelegatePermissionCollection.java at line 81 and 97, we may also
      remove the synchronization if we use "putIfAbsent" (but we should use
      CopyOnWriteArrayList for list "permissions" since it will be accessed
      concurrently).

      There is another kind of misusage. Here is a piece of code in file
      examples/dynamic-datasource-routing/src/main/java/org/superbiz/dynamicdatasourcerouting/DeterminedRouter.java
      from line 90 to 94:

      L90 if (!dataSources.containsKey(datasourceName))

      { L91 throw new IllegalArgumentException("data source called " + datasourceName + " can't be found."); L92 }

      L93 DataSource ds = dataSources.get(datasourceName);
      L94 currentDataSource.set(ds);

      In the code above, suppose a thread T1 executes line 90 and finds out
      the concurrent hashmap "dataSources" contains the key
      "datasourceName". Before it gets to execute line 93, another thread T2
      removes the key "datasourceName" from the concurrent hashmap
      "dataSources". Now thread T1 resumes execution and it will
      get a null value, which is incorrect.

      Similar bugs can be found in DynamicDataSourceTest.java at line 283,
      QueryProxy.java at line 353 and 200, DelegatePermissionCollection.java
      at line 76.

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              jacklondongood Yu Lin
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated: