Details
-
Bug
-
Status: Open
-
Major
-
Resolution: Unresolved
-
4.0.0
-
None
-
None
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
...
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.