Merge pull request #1 from jcoleman/master

merge newest master

Author: peh

README.markdown

@@ -18,10 +18,12 @@ Tomcat Versions
 This project supports both Tomcat 6 and Tomcat 7. Starting at project version 1.1, precompiled JAR downloads are available for either version of Tomcat while project versions before 1.1 are only available for Tomcat 6.
 
 The official release branches in Git are as follows:
-* `tomcat-6`: Continuing work for Tomcat 6 releases. Compatible with Java 6.
-* `master`: Continuing work for Tomcat 7 releases. Compatible with Java 6 or 7.
-* `tomcat-7`: Finalized; has now been merged into `master`. Compatible with Java 6 or 7.
-* `java-7`: All of the work from master for Tomcat 7 but taking advantage of new features in Java 7. Compatible with Java 7 only.
+* `master`: Continuing work for Tomcat 7 releases. Compatible with Java 7.
+* `tomcat-6`: Deprecated; may accept submitted patches, but no new work is being done on this branch. Compatible with Tomcat 6 and Java 6.
+
+Finalized branches include:
+* `tomcat-7`: Has been merged into `master`. Compatible with Java 6 or 7.
+* `java-7`: Has been merged into `master`. All of the work from master for Tomcat 7 but taking advantage of new features in Java 7. Compatible with Java 7 only.
 
 Architecture
 ------------
@@ -96,7 +98,10 @@ This condition will be detected by the session manager and a java.lang.IllegalSt
 
 Normally this should be incredibly unlikely (insert joke about programmers and "this should never happen" statements here) since the connection to save the session into Redis is almost guaranteed to be faster than the latency between a client receiving the response, processing it, and starting a new request.
 
-If you encounter errors, then you can force save the session early (before sending a response to the client) then you can retrieve the current session, and call `currentSession.manager.save(currentSession)` to synchronously eliminate the race condition. Note: this will only work directly if your application has the actual session object directly exposed. Many frameworks (and often even Tomcat) will expose the session in their own wrapper HttpSession implementing class. You may be able to dig through these layers to expose the actual underlying RedisSession instance--if so, then using that instance will allow you to implement the workaround.
+Possible solutions:
+
+- Enable the "save on change" feature by setting `saveOnChange` to `true` in your manager declaration in Tomcat's context.xml. Using this feature will degrade performance slightly as any change to the session will save the session synchronously to Redis, and technically this will still exhibit slight race condition behavior, but it eliminates as much possiblity of errors occurring as possible.
+- If you encounter errors, then you can force save the session early (before sending a response to the client) then you can retrieve the current session, and call `currentSession.manager.save(currentSession)` to synchronously eliminate the race condition. Note: this will only work directly if your application has the actual session object directly exposed. Many frameworks (and often even Tomcat) will expose the session in their own wrapper HttpSession implementing class. You may be able to dig through these layers to expose the actual underlying RedisSession instance--if so, then using that instance will allow you to implement the workaround.
 
 Acknowledgements
 ----------------

build.gradle

@@ -7,7 +7,7 @@ repositories {
 
 dependencies {
   compile group: 'org.apache.tomcat', name: 'tomcat-catalina', version: '7.0.27'
-  compile group: 'redis.clients', name: 'jedis', version: '2.0.0'
+  compile group: 'redis.clients', name: 'jedis', version: '2.5.2'
   // compile group: 'commons-collections', name: 'commons-collections', version: '3.2'
   // testCompile group: 'junit', name: 'junit', version: '4.+'
 }

src/main/java/com/radiadesign/catalina/session/JavaSerializer.java

@@ -19,11 +19,10 @@ public byte[] serializeFrom(HttpSession session) throws IOException {
 
     RedisSession redisSession = (RedisSession) session;
     ByteArrayOutputStream bos = new ByteArrayOutputStream();
-    ObjectOutputStream oos = new ObjectOutputStream(new BufferedOutputStream(bos));
-    oos.writeLong(redisSession.getCreationTime());
-    redisSession.writeObjectData(oos);
-
-    oos.close();
+    try (ObjectOutputStream oos = new ObjectOutputStream(new BufferedOutputStream(bos))) {
+      oos.writeLong(redisSession.getCreationTime());
+      redisSession.writeObjectData(oos);
+    }
 
     return bos.toByteArray();
   }

src/main/java/com/radiadesign/catalina/session/RedisSession.java

@@ -4,9 +4,16 @@
 import org.apache.catalina.Manager;
 import org.apache.catalina.session.StandardSession;
 import java.util.HashMap;
+import java.io.IOException;
+
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
 
 
 public class RedisSession extends StandardSession {
+
+  private final Log log = LogFactory.getLog(RedisSession.class);
+
   protected static Boolean manualDirtyTrackingSupportEnabled = false;
 
   public static void setManualDirtyTrackingSupportEnabled(Boolean enabled) {
@@ -37,7 +44,7 @@ public HashMap<String, Object> getChangedAttributes() {
   }
 
   public void resetDirtyTracking() {
-    changedAttributes = new HashMap<String, Object>();
+    changedAttributes = new HashMap<>();
     dirty = false;
   }
 
@@ -54,16 +61,34 @@ public void setAttribute(String key, Object value) {
               || oldValue == null && value != null
               || !value.getClass().isInstance(oldValue)
               || !value.equals(oldValue) ) ) {
-      changedAttributes.put(key, value);
+      if (this.manager instanceof RedisSessionManager
+          && ((RedisSessionManager)this.manager).getSaveOnChange()) {
+        try {
+          ((RedisSessionManager)this.manager).save(this);
+        } catch (IOException ex) {
+          log.error("Error saving session on setAttribute (triggered by saveOnChange=true): " + ex.getMessage());
+        }
+      } else {
+        changedAttributes.put(key, value);
+      }
     }
 
     super.setAttribute(key, value);
   }
 
   @Override
   public void removeAttribute(String name) {
-    dirty = true;
     super.removeAttribute(name);
+    if (this.manager instanceof RedisSessionManager
+        && ((RedisSessionManager)this.manager).getSaveOnChange()) {
+      try {
+        ((RedisSessionManager)this.manager).save(this);
+      } catch (IOException ex) {
+        log.error("Error saving session on setAttribute (triggered by saveOnChange=true): " + ex.getMessage());
+      }
+    } else {
+      dirty = true;
+    }
   }
 
   @Override

src/main/java/com/radiadesign/catalina/session/RedisSessionManager.java

@@ -39,15 +39,17 @@ public class RedisSessionManager extends ManagerBase implements Lifecycle {
   protected JedisPool connectionPool;
 
   protected RedisSessionHandlerValve handlerValve;
-  protected ThreadLocal<RedisSession> currentSession = new ThreadLocal<RedisSession>();
-  protected ThreadLocal<String> currentSessionId = new ThreadLocal<String>();
-  protected ThreadLocal<Boolean> currentSessionIsPersisted = new ThreadLocal<Boolean>();
+  protected ThreadLocal<RedisSession> currentSession = new ThreadLocal<>();
+  protected ThreadLocal<String> currentSessionId = new ThreadLocal<>();
+  protected ThreadLocal<Boolean> currentSessionIsPersisted = new ThreadLocal<>();
   protected Serializer serializer;
 
   protected static String name = "RedisSessionManager";
 
   protected String serializationStrategyClass = "com.radiadesign.catalina.session.JavaSerializer";
 
+  protected boolean saveOnChange = false;
+
   /**
    * The lifecycle event support for this component.
    */
@@ -97,6 +99,14 @@ public void setSerializationStrategyClass(String strategy) {
     this.serializationStrategyClass = strategy;
   }
 
+  public boolean getSaveOnChange() {
+    return saveOnChange;
+  }
+
+  public void setSaveOnChange(boolean saveOnChange) {
+    this.saveOnChange = saveOnChange;
+  }
+
   @Override
   public int getRejectedSessions() {
     // Essentially do nothing.
@@ -201,13 +211,7 @@ protected synchronized void startInternal() throws LifecycleException {
 
     try {
       initializeSerializer();
-    } catch (ClassNotFoundException e) {
-      log.fatal("Unable to load serializer", e);
-      throw new LifecycleException(e);
-    } catch (InstantiationException e) {
-      log.fatal("Unable to load serializer", e);
-      throw new LifecycleException(e);
-    } catch (IllegalAccessException e) {
+    } catch (ClassNotFoundException | InstantiationException | IllegalAccessException e) {
       log.fatal("Unable to load serializer", e);
       throw new LifecycleException(e);
     }
@@ -246,33 +250,32 @@ protected synchronized void stopInternal() throws LifecycleException {
   }
 
   @Override
-  public Session createSession(String sessionId) {
-    RedisSession session = (RedisSession)createEmptySession();
-
-    // Initialize the properties of the new session and return it
-    session.setNew(true);
-    session.setValid(true);
-    session.setCreationTime(System.currentTimeMillis());
-    session.setMaxInactiveInterval(getMaxInactiveInterval());
-
+  public Session createSession(String requestedSessionId) {
+    RedisSession session = null;
+    String sessionId = null;
     String jvmRoute = getJvmRoute();
 
     Boolean error = true;
     Jedis jedis = null;
-
     try {
       jedis = acquireConnection();
 
       // Ensure generation of a unique session identifier.
-      do {
-        if (null == sessionId) {
-          sessionId = generateSessionId();
-        }
-
+      if (null == requestedSessionId) {
         if (jvmRoute != null) {
-          sessionId += '.' + jvmRoute;
+          sessionId = requestedSessionId + '.' + jvmRoute;
+        }
+        if (jedis.setnx(sessionId.getBytes(), NULL_SESSION) == 0L) {
+          sessionId = null;
         }
-      } while (jedis.setnx(sessionId.getBytes(), NULL_SESSION) == 1L); // 1 = key set; 0 = key already existed
+      } else {
+        do {
+          sessionId = generateSessionId();
+          if (jvmRoute != null) {
+            sessionId += '.' + jvmRoute;
+          }
+        } while (jedis.setnx(sessionId.getBytes(), NULL_SESSION) == 0L); // 1 = key set; 0 = key already existed
+      }
 
       /* Even though the key is set in Redis, we are not going to flag
          the current thread as having had the session persisted since
@@ -282,12 +285,27 @@ This ensures that the save(session) at the end of the request
 
       error = false;
 
-      session.setId(sessionId);
-      session.tellNew();
+      if (null != sessionId) {
+        session = (RedisSession)createEmptySession();
+        session.setNew(true);
+        session.setValid(true);
+        session.setCreationTime(System.currentTimeMillis());
+        session.setMaxInactiveInterval(getMaxInactiveInterval());
+        session.setId(sessionId);
+        session.tellNew();
+      }
 
       currentSession.set(session);
       currentSessionId.set(sessionId);
       currentSessionIsPersisted.set(false);
+
+      if (null != session && this.getSaveOnChange()) {
+        try {
+          save(session);
+        } catch (IOException ex) {
+          log.error("Error saving newly created session (triggered by saveOnChange=true): " + ex.getMessage());
+        }
+      }
     } finally {
       if (jedis != null) {
         returnConnection(jedis, error);
@@ -326,6 +344,9 @@ public Session findSession(String id) throws IOException {
 
       if (session != null) {
         currentSessionIsPersisted.set(true);
+      } else {
+        currentSessionIsPersisted.set(false);
+        id = null;
       }
     }