Author: kkolinko Date: Sat Jul 12 14:20:24 2014 New Revision: 1609924 URL: http://svn.apache.org/r1609924 Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56712 Fix session time comparisons in PersistentManagerBase. When asking for session times use internal**() methods to avoid an IllegalStateException.
Speed up TestPersistentManager test for BZ 56698 by removing sleep(1000) and fix occasional failures. It is backport of r1609920 from tomcat/trunk. Modified: tomcat/tc7.0.x/trunk/ (props changed) tomcat/tc7.0.x/trunk/java/org/apache/catalina/session/PersistentManagerBase.java tomcat/tc7.0.x/trunk/test/org/apache/catalina/session/TestPersistentManager.java tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Propchange: tomcat/tc7.0.x/trunk/ ------------------------------------------------------------------------------ Merged /tomcat/trunk:r1609920 Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/session/PersistentManagerBase.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/session/PersistentManagerBase.java?rev=1609924&r1=1609923&r2=1609924&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/catalina/session/PersistentManagerBase.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/session/PersistentManagerBase.java Sat Jul 12 14:20:24 2014 @@ -926,11 +926,11 @@ public abstract class PersistentManagerB continue; int timeIdle; if (StandardSession.LAST_ACCESS_AT_START) { - timeIdle = (int) ((timeNow - session.getLastAccessedTime()) / 1000L); + timeIdle = (int) ((timeNow - session.getLastAccessedTimeInternal()) / 1000L); } else { - timeIdle = (int) ((timeNow - session.getThisAccessedTime()) / 1000L); + timeIdle = (int) ((timeNow - session.getThisAccessedTimeInternal()) / 1000L); } - if (timeIdle > maxIdleSwap && timeIdle > minIdleSwap) { + if (timeIdle >= maxIdleSwap && timeIdle >= minIdleSwap) { if (session.accessCount != null && session.accessCount.get() > 0) { // Session is currently being accessed - skip it @@ -981,11 +981,11 @@ public abstract class PersistentManagerB synchronized (session) { int timeIdle; if (StandardSession.LAST_ACCESS_AT_START) { - timeIdle = (int) ((timeNow - session.getLastAccessedTime()) / 1000L); + timeIdle = (int) ((timeNow - session.getLastAccessedTimeInternal()) / 1000L); } else { - timeIdle = (int) ((timeNow - session.getThisAccessedTime()) / 1000L); + timeIdle = (int) ((timeNow - session.getThisAccessedTimeInternal()) / 1000L); } - if (timeIdle > minIdleSwap) { + if (timeIdle >= minIdleSwap) { if (session.accessCount != null && session.accessCount.get() > 0) { // Session is currently being accessed - skip it @@ -1027,7 +1027,7 @@ public abstract class PersistentManagerB synchronized (session) { if (!session.isValid()) continue; - long lastAccessedTime = session.getLastAccessedTime(); + long lastAccessedTime = session.getLastAccessedTimeInternal(); Long persistedLastAccessedTime = (Long) session.getNote(PERSISTED_LAST_ACCESSED_TIME); if (persistedLastAccessedTime != null && @@ -1035,11 +1035,11 @@ public abstract class PersistentManagerB continue; int timeIdle; if (StandardSession.LAST_ACCESS_AT_START) { - timeIdle = (int) ((timeNow - session.getLastAccessedTime()) / 1000L); + timeIdle = (int) ((timeNow - session.getLastAccessedTimeInternal()) / 1000L); } else { - timeIdle = (int) ((timeNow - session.getThisAccessedTime()) / 1000L); + timeIdle = (int) ((timeNow - session.getThisAccessedTimeInternal()) / 1000L); } - if (timeIdle > maxIdleBackup) { + if (timeIdle >= maxIdleBackup) { if (log.isDebugEnabled()) log.debug(sm.getString ("persistentManager.backupMaxIdle", Modified: tomcat/tc7.0.x/trunk/test/org/apache/catalina/session/TestPersistentManager.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/session/TestPersistentManager.java?rev=1609924&r1=1609923&r2=1609924&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/test/org/apache/catalina/session/TestPersistentManager.java (original) +++ tomcat/tc7.0.x/trunk/test/org/apache/catalina/session/TestPersistentManager.java Sat Jul 12 14:20:24 2014 @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; @@ -46,6 +47,10 @@ public class TestPersistentManager exten private String oldActivityCheck; + /** + * As documented in config/manager.html, the "ACTIVITY_CHECK" property must + * be set to "true" for PersistentManager to function correctly. + */ @Before public void setActivityCheck() { oldActivityCheck = System.setProperty(ACTIVITY_CHECK, "true"); @@ -60,8 +65,37 @@ public class TestPersistentManager exten } } + /** + * Wait enough for the system clock to update its value. On some systems + * (e.g. old Windows) the clock granularity is tens of milliseconds. + */ + private void waitForClockUpdate() throws InterruptedException { + long startTime = System.currentTimeMillis(); + int waitTime = 1; + do { + Thread.sleep(waitTime); + waitTime *= 10; + } while (System.currentTimeMillis() == startTime); + } + + /** + * Wait while session access counter has a positive value. + */ + private void waitWhileSessionIsActive(StandardSession session) + throws InterruptedException { + long maxWaitTime = System.currentTimeMillis() + 60000; + AtomicInteger accessCount = session.accessCount; + while (accessCount.get() > 0) { + // Wait until o.a.c.connector.Request.recycle() completes, + // as it updates lastAccessedTime. + Assert.assertTrue(System.currentTimeMillis() < maxWaitTime); + Thread.sleep(200); + } + } + @Test - public void backsUpOnce() throws IOException, LifecycleException { + public void backsUpOnce_56698() throws IOException, LifecycleException, + InterruptedException { // Setup Tomcat instance Tomcat tomcat = getTomcatInstance(); @@ -82,19 +116,36 @@ public class TestPersistentManager exten tomcat.start(); String sessionId = getUrl("http://localhost:" + getPort() + "/dummy") .toString(); - sleepABit(); + + // Note: PersistenceManager.findSession() silently updates + // session.lastAccessedTime, so call it only once before other work. + Session session = manager.findSession(sessionId); + + // Wait until request processing ends, as Request.recycle() updates + // session.lastAccessedTime via session.endAccess(). + waitWhileSessionIsActive((StandardSession) session); + + long lastAccessedTime = session.getLastAccessedTimeInternal(); + + // Session should be idle at least for 0 second (maxIdleBackup) + // to be eligible for persistence, thus no need to wait. + + // Waiting a bit, to catch changes in last accessed time of a session + waitForClockUpdate(); manager.processPersistenceChecks(); Assert.assertEquals(Arrays.asList(sessionId), store.getSavedIds()); + Assert.assertEquals(lastAccessedTime, session.getLastAccessedTimeInternal()); // session was not accessed, so no save will be performed + waitForClockUpdate(); manager.processPersistenceChecks(); Assert.assertEquals(Arrays.asList(sessionId), store.getSavedIds()); + Assert.assertEquals(lastAccessedTime, session.getLastAccessedTimeInternal()); // access session - manager.findSession(sessionId).access(); - manager.findSession(sessionId).endAccess(); - sleepABit(); + session.access(); + session.endAccess(); // session was accessed, so it will be saved once again manager.processPersistenceChecks(); @@ -107,14 +158,6 @@ public class TestPersistentManager exten store.getSavedIds()); } - private void sleepABit() { - try { - Thread.sleep(1000); - } catch (InterruptedException e) { - // ignore Interrupt - } - } - private static class DummyServlet extends HttpServlet { private static final long serialVersionUID = -3696433049266123995L; Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1609924&r1=1609923&r2=1609924&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Sat Jul 12 14:20:24 2014 @@ -159,6 +159,10 @@ <bug>56698</bug>: When persisting idle sessions, only persist newly idle sessions. Patch provided by Felix Schumacher. (markt) </fix> + <fix> + <bug>56712</bug>: Fix session idle time calculations in + <code>PersistenceManager</code>. (kkolinko) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org