Bug 50138 - Lack of synchronization in org.apache.catalina.security.SecurityUtil
Summary: Lack of synchronization in org.apache.catalina.security.SecurityUtil
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 6.0.29
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
Depends on:
Reported: 2010-10-21 09:22 UTC by Dmitry Mikhaylov
Modified: 2014-02-17 13:53 UTC (History)
0 users


Note You need to log in before you can comment on or make changes to this bug.
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.