https://issues.apache.org/bugzilla/show_bug.cgi?id=53022
Leonardo Uribe <lu4...@apache.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |REOPENED
Resolution|INVALID |
--- Comment #4 from Leonardo Uribe <lu4...@apache.org> 2012-04-02 20:41:44 UTC
---
Ok, I'll explain it in deep. Look this code in
org.apache.catalina.session.ManagerBase
protected String generateSessionId() {
String result = null;
do {
if (result != null) {
// Not thread-safe but if one of multiple increments is lost
// that is not a big deal since the fact that there was any
// duplicate is a much bigger issue.
duplicates++;
}
result = sessionIdGenerator.generateSessionId();
} while (sessions.containsKey(result));
return result;
}
But the in tomcat 7.0.23 was
protected synchronized String generateSessionId() {
......
The code was changed in 7.0.26 from use just one SecureRandom to:
private Queue<SecureRandom> randoms = new
ConcurrentLinkedQueue<SecureRandom>();
There is one good reason for use a synchronized method in that part. Before
Java 7, there is no mention anywere on java spec that SecureRandom is thread
safe. To ensure thread safety, SecureRandom and the algorithm should ensure
thread safety (look than in 7.0.26, the algorihm can be configured, are you
100% sure the algorithm selected will be thread safe?). Being strict with java
spec, you should use a synchronized block in that part.
But the worst part is the use of a queue of SecureRandom instances and the way
the algorithm check for duplicates. Since the containsKey() operation and the
further put() operation occur in different places (see ManagerBase.add()),
there is no warrant a duplicate sessionid cannot be included in the time
between containsKey() and put() and since you have multiple SecureRandom
instances this risk is even more evident.
Even if the probability that those events could happen is very, very, very
small, it is clear that ManagerBase.add() operation should do the check for
duplicates too and if that so, try to generate and alternate identifier.
Note how tricky is all this. Reproduce this failure is something almost
impossible, one in a million. There is no unit test that could catch it, but
the bug exists, it is theorically feasible.
After investigating my particular problem, I found that my problem was more
related with JMeter. In my load test I used the default Java for HTTP protocol
implementation (because is a little bit faster) but since there is no control
over connection reuse, Tomcat throw SocketException after some time without log
it in the default logger. Switching to HttpClient4 solves the problem.
It is curious how many users has been caught over the time into the same
problem (JMeter-Tomcat). After all, the solution is not trivial.
Please be more kind in the future. By experience I know that sometimes gather
evidence about these kind of bugs is very difficult, so don't throw away such
reports, without consider them first.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
