Author: kkolinko
Date: Sat Jul 12 13:54:30 2014
New Revision: 1609920
URL: http://svn.apache.org/r1609920
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.
Modified:
tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
tomcat/trunk/test/org/apache/catalina/session/TestPersistentManager.java
tomcat/trunk/webapps/docs/changelog.xml
Modified:
tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java?rev=1609920&r1=1609919&r2=1609920&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
(original)
+++ tomcat/trunk/java/org/apache/catalina/session/PersistentManagerBase.java
Sat Jul 12 13:54:30 2014
@@ -903,8 +903,8 @@ public abstract class PersistentManagerB
synchronized (session) {
if (!session.isValid())
continue;
- int timeIdle = (int) (session.getIdleTime() / 1000L);
- if (timeIdle > maxIdleSwap && timeIdle > minIdleSwap) {
+ int timeIdle = (int) (session.getIdleTimeInternal() /
1000L);
+ if (timeIdle >= maxIdleSwap && timeIdle >= minIdleSwap) {
if (session.accessCount != null &&
session.accessCount.get() > 0) {
// Session is currently being accessed - skip it
@@ -952,8 +952,8 @@ public abstract class PersistentManagerB
for (int i = 0; i < sessions.length && toswap > 0; i++) {
StandardSession session = (StandardSession) sessions[i];
synchronized (session) {
- int timeIdle = (int) (session.getIdleTime() / 1000L);
- if (timeIdle > minIdleSwap) {
+ int timeIdle = (int) (session.getIdleTimeInternal() / 1000L);
+ if (timeIdle >= minIdleSwap) {
if (session.accessCount != null &&
session.accessCount.get() > 0) {
// Session is currently being accessed - skip it
@@ -994,14 +994,14 @@ 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 &&
lastAccessedTime ==
persistedLastAccessedTime.longValue())
continue;
- int timeIdle = (int) (session.getIdleTime() / 1000L);
- if (timeIdle > maxIdleBackup) {
+ int timeIdle = (int) (session.getIdleTimeInternal() /
1000L);
+ if (timeIdle >= maxIdleBackup) {
if (log.isDebugEnabled())
log.debug(sm.getString
("persistentManager.backupMaxIdle",
Modified:
tomcat/trunk/test/org/apache/catalina/session/TestPersistentManager.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/session/TestPersistentManager.java?rev=1609920&r1=1609919&r2=1609920&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/session/TestPersistentManager.java
(original)
+++ tomcat/trunk/test/org/apache/catalina/session/TestPersistentManager.java
Sat Jul 12 13:54:30 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/trunk/webapps/docs/changelog.xml
URL:
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1609920&r1=1609919&r2=1609920&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Sat Jul 12 13:54:30 2014
@@ -71,6 +71,10 @@
defined as <code>HIGH:!aNULL:!eNULL:!EXPORT:!DES:!RC4:!MD5</code> so
that no weak ciphers are enabled by default. (remm)
</fix>
+ <fix>
+ <bug>56712</bug>: Fix session idle time calculations in
+ <code>PersistenceManager</code>. (kkolinko)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
