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

Reply via email to