commitSession() called twice

[fe2s]

Hi,

I'm developing custom SessionRepository.

I noticed that SessionRepositoryRequestWrapper.commitSession() called twice

• onResponseCommitted
• in doFilterInternal, finally block

This causes two calls of sessionRepository.save(session) and leads to persisting session to external storage twice.

What's the reason to commitSession() twice?

[rwinch]

it is necessary to persist the session as soon as the response is committed to ensure the session is available to the clients as soon as the client has access to the session id. Consider the following scenario:

• Request A: A user is authenticated and placed into session
• Request A: The response is committed (but not totally completed) with a particular session id
• Request B: The client gets the (partial) response from Request A and then makes Request B with the session id
• Request B: The server looks for the session but it is not found because while the response was committed and the client could see it the Filter invocation did not complete

It is necessary to write after the session is committed because further modifications to the session might be made. For example:

• Request A: A request for a session is made
• Request A: the session is updated with user=first
• Request A: The response is committed (so the session is written)
• Request A: The session is updated with user=second
• What is going to ensure the session is written without having the finally blocK?

It is generally recommended for a SessionRepository implementation to keep track of changes to ensure that only deltas are saved. You can see that this is how the Redis implementation provided with Spring Session behaves.

Does this make sense now?

[fe2s]

Got it. Thanks

[cemo]

It might be a good idea to add this into documentation. Thanks for detailed information.

[rwinch]

@cemo Thanks for the suggestion. We will include your suggestion in #199

[zeronone]

When Spring's session-scoped beans are used, it is actually committed only in the second time (in the finally block). This can be very confusing to the users.

Also from the performance viewpoint, it is bad. But Spring session scoped beans are always set using Session#setAttribute, so it is always saved back to the repository. link

Is there any configuration or settings that would store all the changes in one round.

[vpavic]

When Spring's session-scoped beans are used, it is actually committed only in the second time (in the finally block).

Not sure what you meant there @zeronone. From what I'm seeing in a request involving session scoped bean SessionRepositoryFilter.SessionRepositoryRequestWrapper#commitSession gets invoked both from SessionRepositoryFilter.SessionRepositoryResponseWrapper#onResponseCommitted and from finally block in SessionRepositoryFilter#doFilterInternal, as expected.

Is there any configuration or settings that would store all the changes in one round.

For reasons explained in this comment, it wouldn't be safe to allow that.

If you're dealing with a concrete problem, it's preferred to open a new issue and provide as much info as possible (ideally, coupled with a sample app) rather then to comment on a closed issue.

[zeronone]

@vpavic Here is the sequence of operations happening in our application.

1. Incoming request
2. The session is accessed so lastAccessTime is updated.
3. Spring session scoped is created ServletRequestAttributes#getAttribute which also updates the sessionAttributesToUpdate property
4. The response is sent to the response OutputStream, so SessionRepositoryFilter.SessionRepositoryRequestWrapper#commitSession is called. Since lastAccessTime has changed since, so the associated redis opertions will happen (hmset, pexpire etc).
5. AbstractRequestAttributes#requestCompleted() -> ServletRequestAttributes#updateAccessedSessionAttributes is called. Which updates the session again with all the session scoped beans (no matter if mutated or not).
6. SessionRepositoryFilter#doFilterInternal will call commitSession in the finally block again, and since the session has been updated again by ServletRequestAttributes#updateAccessedSessionAttributes, another set of redis operations will occur.

The problem is that the session scoped beans are inserted back into the session (Session.setAttribute) after the response has been flushed to OutputStream, causing an extra round of Redis operations.

[ravenadesk]

@rwinch
I am not sure if it is a bug, but I am facing the inconsistency between the two saves when using redis with replica.

In our project, we are using redis as the storage, there is one master and two replica.
When I was testing /health endpoint with a little load, sometimes the second save cannot get the right session status (taking it as not stored in redis) throwing IllegalStateException

	@Override
	public void save(RedisSession session) {
		if (!session.isNew) {
			String key = getSessionKey(session.hasChangedSessionId() ? session.originalSessionId : session.getId());
			Boolean sessionExists = this.sessionRedisOperations.hasKey(key);
			if (sessionExists == null || !sessionExists) {
				throw new IllegalStateException("Session was invalidated");
			}
		}
		session.save();
	}

I have tried adding a conditional break point at the line before if condition, and let it pause a little while when key does not exist. And it turns out no problem(key exists, no exception). I have also tried that remove all my replica of redis, and there is no problem at all.
So I am wondering is this twice save patten conflict against replica synchronization of redis.