Skip to content

Commit 01c49c1

Browse files
committed
Ignore failed rename operation for deleted session
In scenario with concurrent requests attempting to change session id, the "ERR no such key" error will occur for a thread that comes in second. This commit addresses the problem by ignoring the aforementioned error. Resolves: #1327
1 parent 7c8fe93 commit 01c49c1

File tree

2 files changed

+39
-10
lines changed

2 files changed

+39
-10
lines changed

spring-session-data-redis/src/integration-test/java/org/springframework/session/data/redis/RedisOperationsSessionRepositoryITests.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2014-2018 the original author or authors.
2+
* Copyright 2014-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -600,6 +600,25 @@ public void changeSessionIdWhenSessionIsDeleted() {
600600
assertThat(this.repository.findById(sessionId)).isNull();
601601
}
602602

603+
@Test // gh-1270
604+
public void changeSessionIdSaveConcurrently() {
605+
RedisSession toSave = this.repository.createSession();
606+
String originalId = toSave.getId();
607+
this.repository.save(toSave);
608+
609+
RedisSession copy1 = this.repository.findById(originalId);
610+
RedisSession copy2 = this.repository.findById(originalId);
611+
612+
copy1.changeSessionId();
613+
this.repository.save(copy1);
614+
copy2.changeSessionId();
615+
this.repository.save(copy2);
616+
617+
assertThat(this.repository.findById(originalId)).isNull();
618+
assertThat(this.repository.findById(copy1.getId())).isNotNull();
619+
assertThat(this.repository.findById(copy2.getId())).isNull();
620+
}
621+
603622
private String getSecurityName() {
604623
return this.context.getAuthentication().getName();
605624
}

spring-session-data-redis/src/main/java/org/springframework/session/data/redis/RedisOperationsSessionRepository.java

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2014-2018 the original author or authors.
2+
* Copyright 2014-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -864,24 +864,34 @@ private void saveChangeSessionId(String sessionId) {
864864
if (!isNew()) {
865865
String originalSessionIdKey = getSessionKey(this.originalSessionId);
866866
String sessionIdKey = getSessionKey(sessionId);
867-
RedisOperationsSessionRepository.this.sessionRedisOperations.rename(
868-
originalSessionIdKey, sessionIdKey);
867+
try {
868+
RedisOperationsSessionRepository.this.sessionRedisOperations
869+
.rename(originalSessionIdKey, sessionIdKey);
870+
}
871+
catch (NonTransientDataAccessException ex) {
872+
handleErrNoSuchKeyError(ex);
873+
}
869874
String originalExpiredKey = getExpiredKey(this.originalSessionId);
870875
String expiredKey = getExpiredKey(sessionId);
871876
try {
872-
RedisOperationsSessionRepository.this.sessionRedisOperations.rename(
873-
originalExpiredKey, expiredKey);
877+
RedisOperationsSessionRepository.this.sessionRedisOperations
878+
.rename(originalExpiredKey, expiredKey);
874879
}
875880
catch (NonTransientDataAccessException ex) {
876-
if (!"ERR no such key".equals(NestedExceptionUtils
877-
.getMostSpecificCause(ex).getMessage())) {
878-
throw ex;
879-
}
881+
handleErrNoSuchKeyError(ex);
880882
}
881883
}
882884
this.originalSessionId = sessionId;
883885
}
884886
}
887+
888+
private void handleErrNoSuchKeyError(NonTransientDataAccessException ex) {
889+
if (!"ERR no such key"
890+
.equals(NestedExceptionUtils.getMostSpecificCause(ex).getMessage())) {
891+
throw ex;
892+
}
893+
}
894+
885895
}
886896

887897
/**

0 commit comments

Comments
 (0)