Spring/spring-session-data-mongodb
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

RedisSessionExpirationPolicy.cleanExpiredSessions does not delete performs get

Browse Source 
The Problem:

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.

Fixing the Issue:

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 mean
s 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.

Fixes gh-93
This commit is contained in:
Rob Winch
2015-01-07 14:39:10 -06:00
parent 9b168423b8
commit 5847801a32
4 changed files with 24 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
spring-session/src/test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryTests.java
View File

@@ -47,8 +47,6 @@ public class RedisOperationsSessionRepositoryTests {
BoundSetOperations<String, String> boundSetOperations;
@Captor
ArgumentCaptor<Map<String,Object>> delta;
@Captor
ArgumentCaptor<Set<String>> keys;
private RedisOperationsSessionRepository redisRepository;
@@ -237,19 +235,17 @@ public class RedisOperationsSessionRepositoryTests {
String expiredId = "expired-id";
when(redisOperations.boundHashOps(getKey(expiredId))).thenReturn(boundHashOperations);
when(redisOperations.boundSetOps(anyString())).thenReturn(boundSetOperations);
Set<String> expiredIds = new HashSet<String>(Arrays.asList("expired-key1","expired-key2"));
when(boundSetOperations.members()).thenReturn(expiredIds);
redisRepository.cleanupExpiredSessions();
verify(redisOperations).delete(keys.capture());
for(String id : expiredIds) {
String expiredKey = RedisOperationsSessionRepository.BOUNDED_HASH_KEY_PREFIX + id;
assertThat(keys.getValue()).contains(expiredKey);
// https://github.com/spring-projects/spring-session/issues/93
verify(redisOperations).hasKey(expiredKey);
}
// the key that maps the expiration time to the expired ids
assertThat(keys.getValue().size()).isEqualTo(expiredIds.size() + 1);
}
@SuppressWarnings("rawtypes")