Bug 46085 - Session are incorrectly expired due to thread unsafe code
Summary: Session are incorrectly expired due to thread unsafe code
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 6
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 6.0.18
Hardware: PC Linux
: P2 major (vote)
Target Milestone: default
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-10-24 12:53 UTC by Philip Shore
Modified: 2009-07-07 11:12 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Shore 2008-10-24 12:53:09 UTC
We have identified a bug where sessions are expired even though they are still valid.  

The source of the problem is concurrent threads reading and writing thisAccessTime in "org.apache.catalina.session.StandardSession" in a way that is not thread safe. 

The good news is that we have a simple solution, for Tomcat 6.  The fix is to mark the class level variables - thisAccessedTime and lastAccessedTime - as volatile.



Detailed explanation of the bug:

We found that StandardSession's isValid() function was making a call to expire() on the following block of code:

        if (maxInactiveInterval >= 0) { 
            long timeNow = System.currentTimeMillis();
            int timeIdle = (int) ((timeNow - thisAccessedTime) / 1000L);
            if (timeIdle >= maxInactiveInterval) {
                expire(true);
            }
        }

Debugging showed that thisAccessedTime was in the past (1983!) and therefore timeIdle is large enough for expiry to happen.

In another thread running in parallel, the access() function runs this line of code:

    this.thisAccessedTime = System.currentTimeMillis();

That single line of code might be atomic, i.e. is composed of multiple instructions, which means it is possible for another thread reading thisAccessedTime to read a value that is effectively corrupt.  Marking thisAccessedTime as volatile ensures that writes are completely finished before reading is allowed. 

Note that the volatile solution only works for Java 1.5 or higher as the meaning of volatile changed then. That means it cannot be used for older Tomcat's if they have a similar problem. Another solution would have been to synchronize reads and writes to that variable.



Reproducing the bug:

This is quite hard within Tomcat as is very dependant on timings, the JVM's optimisations, and any changes to code may affect optimisations. We are only able to reproduce the bug on multiprocessor systems running an IBM JVM.

Specifically, we are running:
  SLES 10.3 Linux on an Intel platform.
  Two or more Intel CPU's
  Tomcat 6.0.18
  JVM package: java-1_5_0-ibm-1.5.0_sr8a-1.1

Initially, we thought that System.currentTimeMillis() was returning an incorrect value and created a test program which demo'd the problem to IBM.  We were wrong but they did give us the volatile solution.  See here: http://www.ibm.com/developerworks/forums/thread.jspa?threadID=230478
Comment 1 Mark Thomas 2008-10-27 11:33:49 UTC
This has been fixed in trunk and proposed for 6.0.x
Comment 2 Mark Thomas 2008-11-05 07:41:18 UTC
This has been fixed in 6.0.x and will be included 6.0.19 onwards.
Comment 3 Daniel 2009-07-06 15:04:28 UTC
(In reply to comment #2)
> This has been fixed in 6.0.x and will be included 6.0.19 onwards.

We encountered random session timeouts with Tomcat 6.0.16 and IBM JVM 1.5.  I tested Tomcat 6.0.20 and still got random session timeouts.  I downloaded the Tomcat source code, declared StandardSession.isValid and StandardSession.access  synchronized, rebuild and used the updated catalina.jar in my Tomcat installation.  I ran a lot of tests and so far haven't seen random session timeouts.  Should this bug be reopened?
Comment 4 Mark Thomas 2009-07-06 15:15:42 UTC
Looking at the code, I can't see why syncs shouldn't be necessary. Does this still happen with a Sun JVM?

Additionally, if you are worried about correct session expiration you should use ACTIVITY_CHECK or STRICT_SERVLET_COMPLIANCE (see http://tomcat.apache.org/tomcat-6.0-doc/config/systemprops.html)
Comment 5 Daniel 2009-07-07 10:26:26 UTC
Hi Mark.  Thanks for the link!  I set ACTIVITY_CHECK to true and still got random session timeouts with the IBM JVM.  Setting STRICT_SERVLET_COMPLIANCE to true didn't change this either.  I have not seen these random session timeouts with the Sun JVM.  My company uses the IBM JVM and the only way I got it to work reliably was adding the synchronized keyword to StandardSession.isValid and StandardSession.access.  Here is the JVM version info:

java version "1.5.0"
Java(TM) 2 Runtime Environment, Standard Edition (build pwi32devifx-20071025 (SR6b))
IBM J9 VM (build 2.3, J2RE 1.5.0 IBM J9 2.3 Windows XP x86-32 j9vmwi3223-20071007 (JIT enabled)
J9VM - 20071004_14218_lHdSMR
JIT  - 20070820_1846ifx1_r8
GC   - 200708_10)
JCL  - 20071025
Comment 6 Mark Thomas 2009-07-07 11:12:44 UTC
Then that looks like a bug in the IBM implementation of atomics. Maybe try testing with a newer IBM JDK? As an aside, this looks like it belongs on the users list rather than bugzilla.