1. Serialization/deserialization happens in the sessionStore, the utility class only handles the DB operations. 2. SessionTableAttributes was completely removed 3. ExpiredSessionReaper was completely removed 4. The serialization/deserialization happens with Tomcat's own methods. 5. The DB structure was simplified. Only a key (sessionID) and the serialized session data are stored. (Tomcat's reaper needs no more, it checks the expiration date in the deserialized session object) 6 A DB synchronization added to the keys() method (It refresh the keys in each hour from the DB).

Here is the full story behind this version:

I tried to explore the issue discussed here: https://github.com/aws/aws-dynamodb-session-tomcat/issues/18 I took the trouble and examined how the session handling works in Tomcat and also analysed the whole package line by line. It turned out that this is not a simple bug but a very serious logic error in the algorithm.

Let's see the issues step by step:

The serialization of the sessions happens in the utility class but the deserialization happens in the DynamoDBSessionStore class. Well it's not a bug but not too nice and also a bad omen...
The inner structure of the DB is exposed via the public SessionTableAttributes class. Why do we need this if we have a utility class for accessing the DB? Bad omen 2.
The package has an ExpiredSessionReaper for deleting the expired sessions. But Tomcat has it's own process for this task what runs in every minute. Why do we need an additional reaper? Still not a real bug, but this was the third bad omen... a very evil thing must be near...
Tomcat's own reaper works in a general way. It has no direct access to the actual storage, so it uses the public methods of the storage implementation, like the load and remove methods. It loads the sessions one by one, checks they expiration date and remove them from the storage if needed. Let's check the load method in the DynamoDBSessionStore! We can find the following line is in it:

manager.add(session);

The session is added to the manager inside the load method. Let's see what happens here:

Let's assume a user has made some changes in his session but the session is not stored yet, because it's only stored after a 30 seconds idle interval. Then Tomcat's own sessionReaper starts working. It goes through all the sessions in the store, so it also loads this user's session too using the load method mentioned above. Of course the session is not expired so it won't delete it, but during the load method the freshly loaded - but out-of-date session will be registered to the manager. Because it has the same sessionId as the already existing - and updated - session (of course) it will simply override the actual session in the manager! And that's exactly what we experienced and what was described in the mentioned issue above. The session has switched back to an earlier state!

But removing this line from the load method won't solve the problem. The session creation itself also register the session to the manager:

Session session = getManager().createSession(id);

Basically when you provide an id and a manager object to a session, it will be instantly registered to the manager. There is only one exception: If we use the StandardSession's own deserialization method. Because it HAS an own and very well implemented serialization and deserialization method in it!

So, why do we want to serialize and deserialize the session objects by our own way, when we could use the original, well tested and managed methods? The JSBCSessionStore and FileSessionStore implemetations also use these methods and they work perfectly.

I've found one more glitch: The DynamoDBSessionStore manages a set of session keys in the memory. It is a faster solution than loading back the session keys from the DB each time but other instances could also add or remove sessions from the DB, so some keys could become obsolete over time. We should synchronize this list with the DB time to time to avoid unnecessary checks.

So I made the following changes:

Serialization/deserialization happens in the sessionStore, the utility class only handles the DB operations.
SessionTableAttributes was completely removed
ExpiredSessionReaper was completely removed
The serialization/deserialization happens with Tomcat's own methods.
The DB structure was simplified. Only a key (sessionID) and the serialized session data are stored. (Tomcat's reaper needs no more, it checks the expiration date in the deserialized session object) 6 A DB synchronization added to the keys() method (It refresh the keys in each hour from the DB).

I also changed the major version number, because the new manager is NOT compatible with the previous versions. The sessions are stored in a different format now in the DB.

Hey @Selindek, after looking into this I agree with your original assessment. I had hoped to be able to preserve backwards compatibility with previous versions by only setting the manager after all fields on the session have been set (setManager is just a normal setter with no additional side effects of registering the session). Unfortunately when going down that avenue I discovered that we are not setting the accessed time correctly. We only set the creation time of the session which initializes the access time to the same value, we store the last accessed time in the database but do nothing with it. This too is a pretty serious bug as sessions can be reaped before they are truly expired. The access time doesn't have an exposed setter, calls to access() just set the last accessed time to the current time. It looks like the only option to set the last accessed time would be to subclass StandardSession to gain direct access to those fields or use the serialization/deserialization methods you pointed out in your pull request. I prefer your approach as it's less susceptible to breaking changes in StandardSession across different versions of Tomcat but would like to discuss the options with @fulghum before proceeding.

amazon-archives / aws-dynamodb-session-tomcat