[ 
https://issues.apache.org/jira/browse/JXPATH-152?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13220937#comment-13220937
 ] 

Naozumi Taromaru commented on JXPATH-152:
-----------------------------------------

Collections.synchronizedMap is thread safe method.

However, since these HashMap are referred to frequently,
it becomes a bottleneck by the method of synchronizedMap's lock
in a multi-core CPU (or multi CPU) environment.


I recommend correcting by one of the following methods.
These methods can perform Map#get concurrently.

 * Since byInterface resembles byClass, byInterface omits.

Before correction: (Before #1293412 modification.)
----
private static HashMap byClass = new HashMap();
...
byClass.put(...);
...
beanInfo = (JXPathBeanInfo) byClass.get(...);
----


correction method 1: (Easy)
----
private static Map byClass = ConcurrentHashMap();
...
byClass.put(...);
...
beanInfo = (JXPathBeanInfo) byClass.get(...);
----
 * ConcurrentHashMap : java.util.concurrent.ConcurrentHashMap (JDK5 or later)

The feature of the method 1:
 * Unlike JXPath-1.3, required JDK5 or later.
 * Unlike JXPath-1.3, if key or value is null, NullPointerException occur.
(If you maintain the compatibility when key or value is null value, please see 
method 2.)


correction method 2: (Specification is full compatible)
----
private static HashMap byClass = new HashMap();
private static final ReentrantReadWriteLock byClassReadWriteLock = new 
ReentrantReadWriteLock();
private static final Lock byClassReadLock = byClassReadWriteLock.readLock();
private static final Lock byClassWriteLock = byClassReadWriteLock.writeLock();
...
byClassWriteLock.lock();
try {
    byClass.put(...);
} finally {
    byClassWriteLock.unlock();
}
...
byClassReadLock.lock();
try {
    beanInfo = (JXPathBeanInfo) byClass.get(...);
} finally {
    byClassReadLock.unlock();
}
----
 * ReentrantReadWriteLock : java.util.concurrent.locks.ReentrantReadWriteLock 
(JDK5 or later)
 * Lock : java.util.concurrent.locks.Lock (JDK5 or later)
please see
http://docs.oracle.com/javase/1.5.0/docs/api/java/util/concurrent/locks/ReentrantReadWriteLock.html
"RWDictionary" example.

The feature of the method 2:
 * Unlike JXPath-1.3, required JDK5 or later.
 * Like JXPath-1.3, null can be used for key or value.


correction method 3: (JDK1.3 compatible. but contrary to usage.)
----
private static HashMap byClass = new HashMap();
...
synchronized (byClass) {
    byClass.put(...);
}
...
beanInfo = (JXPathBeanInfo) byClass.get(...);
if (beanInfo == null) { // not mapping, null mapping, or when byClass is being 
expanded.
    synchronized (byClass) {
        beanInfo = (JXPathBeanInfo) byClass.get(...);
    }
}
----
 * Although it is contrary to usage,
   this method can be used if it is java.util.HashMap,
   using method are "put" "get" only.
   
(At least in the case of Sun(Oracle) JDK.)

The feature of the method 3:
 * Like JXPath-1.3, JDK1.3 compatible.
 * Like JXPath-1.3, null can be used for key or value.
 * It is contrary to the following description of HashMap's API document.
"If multiple threads access this map concurrently,
and at least one of the threads modifies the map structurally,
it must be synchronized externally."
(http://docs.oracle.com/javase/1.5.0/docs/api/java/util/HashMap.html)

                
> Concurrent access on hashmap of JXPathIntrospector
> --------------------------------------------------
>
>                 Key: JXPATH-152
>                 URL: https://issues.apache.org/jira/browse/JXPATH-152
>             Project: Commons JXPath
>          Issue Type: Bug
>    Affects Versions: 1.3
>         Environment: Java5, Windows/AIX
>            Reporter: pleutre
>            Assignee: Matt Benson
>            Priority: Minor
>             Fix For: 1.4
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>
> JXPathIntrospector.registerDynamicClass method can be called in static part 
> of classes. 
> If two classes A & B try to registerDynamicClass in the same time a 
> concurrent access exception can append on hashmap of JXPathIntrospector.
> Replace hashmap by concurrent hashmap or synchronized access to these 
> hashmaps.

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

        

Reply via email to