Bug 50138

Summary: Lack of synchronization in org.apache.catalina.security.SecurityUtil
Product: Tomcat 6 Reporter: Dmitry Mikhaylov <mikhailov.dmitry>
Component: CatalinaAssignee: Tomcat Developers Mailing List <dev>
Severity: normal    
Priority: P2    
Version: 6.0.29   
Target Milestone: default   
Hardware: PC   
OS: Linux   

Description Dmitry Mikhaylov 2010-10-21 09:22:56 UTC
Symptom: all processor threads spin madly in:
"tomcat-processor-20" daemon prio=10 tid=0x09210800 nid=0x51fb runnable [0x61b76000]
   java.lang.Thread.State: RUNNABLE
	at java.util.HashMap.getEntry(HashMap.java:347)
	at java.util.HashMap.containsKey(HashMap.java:335)
	at org.apache.catalina.security.SecurityUtil.doAsPrivilege(SecurityUtil.java:227)
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:230)
	at org.apache.catalina.core.ApplicationFilterChain.access$000(ApplicationFilterChain.java:56)
	at org.apache.catalina.core.ApplicationFilterChain$1.run(ApplicationFilterChain.java:189)
	at java.security.AccessController.doPrivileged(Native Method)
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:185)

Cause: org.apache.catalina.security.SecurityUtil.objectCache is a HashMap, but access to it is not synchronized. The javadoc for HashMap says:
Note that this implementation is not synchronized. If multiple threads access a hash map concurrently, and at least one of the threads modifies the map structurally, it must be synchronized externally.

Proposed solution: change objectCache to ConcurrentHashMap;
Comment 1 Sebb 2010-10-21 09:51:59 UTC
That may not be the only bug. 

There are two instances of the following code:

            methodsCache = objectCache.get(targetObject);

If the object is removed between the two statements, then an NPE will follow.

Surely the code should just check whether it got a non-null object?

Also, the private static fields should be final.
Comment 2 Mark Thomas 2010-10-21 12:02:00 UTC
Thanks for the report.

Fixed in trunk for 7.0.5 onwards and proposed for 6.0.x
Comment 3 Mark Thomas 2010-10-25 11:46:19 UTC
Fixed in 6.0.x and will be included in 6.0.30 onwards.
Comment 4 Dmitry Mikhaylov 2010-10-27 02:44:00 UTC
Thanks for prompt fix, waiting for 6.0.30.