Spring-session: Cleanup in RedisOperationsSessionRepository can cause session to be deleted incorrectly

[rwinch]

The background task that cleans up sessions can incorrectly remove a session due to a race condition.

Assume an existing session with the id "1" exists and will expire at 1420656360000. This means our redis store has the following:

spring:session:expirations:1420656360000 -> [1]
spring:session:session:1 -> <session>

Consider the following sequence:

• Thread 1 requests Session 1 and determines it should be forcibly deleted up at 1420656420000
• Thread 2 requests Session 2 and determines it should be forcibly deleted one minute later at 1420656480000
• Thread 2 removes Session 1 from 1420656360000, so it will no longer be forcibly deleted at that time

spring:session:expirations:1420656360000 -> []
spring:session:session:1 -> <session>

• Thread 2 adds Session 1 to 1420656480000

spring:session:expirations:1420656360000 -> []
spring:session:session:1 -> <session>
spring:session:expirations:1420656480000 -> [1]

• Thread 1 removes Session 1 (which was already removed) from 1420656360000 (the original expiration)

spring:session:expirations:1420656360000 -> []
spring:session:session:1 -> <session>
spring:session:expirations:1420656480000 -> [1]

• Thread 1 adds Session 1 to 1420656420000

spring:session:expirations:1420656360000 -> []
spring:session:session:1 -> <session>
spring:session:expirations:1420656480000 -> [1]
spring:session:expirations:1420656420000 -> [1]

Now the session is mapped to be forcibly deleted in two locations. However, at most it will be cleaned up in one location. This means that the session will be forcibly deleted even if the session is continued to be used.

Instead of deleting the session, we should have the background task access the key which will only forcibly delete the key if it is expired. This means that a session could at earliest be deleted when the value in the datastore indicates.

This still means that a session can be deleted too soon since the incorrect TTL may be set on a key. However, at worst this is the the longest HTTP request length. Short of using distributed locking there isn't a good answer to get exact consistency.

Related #92

[lexburner]

Thread 2 requests Session 2 and determines it should be forcibly deleted one minute later at 1420656480000

The sentence may be wrong，session 2 => session 1 .

make me confused