From b8ddd2c690fe3f00bb5e3d9f913a37504aab49a0 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 1 May 2026 15:12:03 +0100 Subject: [PATCH] Avoid race in InMemoryWebSession Closes gh-36742 --- .../web/server/WebSession.java | 28 ++++---- .../session/InMemoryWebSessionStore.java | 67 ++++++++++++------- 2 files changed, 55 insertions(+), 40 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/server/WebSession.java b/spring-web/src/main/java/org/springframework/web/server/WebSession.java index b497b5675ef..449fab1f09a 100644 --- a/spring-web/src/main/java/org/springframework/web/server/WebSession.java +++ b/spring-web/src/main/java/org/springframework/web/server/WebSession.java @@ -101,20 +101,6 @@ public interface WebSession { */ boolean isStarted(); - /** - * Generate a new id for the session and update the underlying session - * storage to reflect the new id. After a successful call {@link #getId()} - * reflects the new session id. - * @return completion notification (success or error) - */ - Mono changeSessionId(); - - /** - * Invalidate the current session and clear session storage. - * @return completion notification (success or error) - */ - Mono invalidate(); - /** * Save the session through the {@code WebSessionStore} as follows: *
    @@ -132,6 +118,20 @@ public interface WebSession { */ Mono save(); + /** + * Generate a new id for the session and update the underlying session + * storage to reflect the new id. After a successful call {@link #getId()} + * reflects the new session id. + * @return completion notification (success or error) + */ + Mono changeSessionId(); + + /** + * Invalidate the current session and clear session storage. + * @return completion notification (success or error) + */ + Mono invalidate(); + /** * Return {@code true} if the session expired after {@link #getMaxIdleTime() * maxIdleTime} elapsed. diff --git a/spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java b/spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java index 963a0f94d58..5a117f9cc49 100644 --- a/spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java +++ b/spring-web/src/main/java/org/springframework/web/server/session/InMemoryWebSessionStore.java @@ -184,6 +184,7 @@ public class InMemoryWebSessionStore implements WebSessionStore { private final AtomicReference state = new AtomicReference<>(State.NEW); + private final Lock lock = new ReentrantLock(); public InMemoryWebSession(Instant creationTime) { this.creationTime = creationTime; @@ -232,29 +233,6 @@ public class InMemoryWebSessionStore implements WebSessionStore { return this.state.get().equals(State.STARTED) || !getAttributes().isEmpty(); } - @Override - public Mono changeSessionId() { - return Mono.defer(() -> { - String currentId = this.id.get(); - InMemoryWebSessionStore.this.sessions.remove(currentId); - String newId = String.valueOf(idGenerator.generateId()); - this.id.set(newId); - InMemoryWebSessionStore.this.sessions.put(this.id.get(), this); - return Mono.empty(); - }) - .subscribeOn(Schedulers.boundedElastic()) - .publishOn(Schedulers.parallel()) - .then(); - } - - @Override - public Mono invalidate() { - this.state.set(State.EXPIRED); - getAttributes().clear(); - InMemoryWebSessionStore.this.sessions.remove(this.id.get()); - return Mono.empty(); - } - @Override @SuppressWarnings("NullAway") public Mono save() { @@ -268,11 +246,19 @@ public class InMemoryWebSessionStore implements WebSessionStore { if (isStarted()) { // Save - InMemoryWebSessionStore.this.sessions.put(this.id.get(), this); + if (InMemoryWebSessionStore.this.sessions.get(getId()) == null) { + this.lock.lock(); + try { + InMemoryWebSessionStore.this.sessions.putIfAbsent(getId(), this); + } + finally { + this.lock.unlock(); + } + } // Unless it was invalidated if (this.state.get().equals(State.EXPIRED)) { - InMemoryWebSessionStore.this.sessions.remove(this.id.get()); + InMemoryWebSessionStore.this.sessions.remove(getId()); return Mono.error(new IllegalStateException("Session was invalidated")); } } @@ -283,12 +269,41 @@ public class InMemoryWebSessionStore implements WebSessionStore { private void checkMaxSessionsLimit() { if (sessions.size() >= maxSessions) { expiredSessionChecker.removeExpiredSessions(clock.instant()); - if (sessions.size() >= maxSessions && !sessions.containsKey(this.id.get())) { + if (sessions.size() >= maxSessions && !sessions.containsKey(getId())) { throw new IllegalStateException("Max sessions limit reached: " + sessions.size()); } } } + @Override + public Mono changeSessionId() { + return Mono.defer(() -> { + this.lock.lock(); + try { + String oldId = getId(); + String newId = String.valueOf(idGenerator.generateId()); + InMemoryWebSessionStore.this.sessions.remove(oldId); + InMemoryWebSessionStore.this.sessions.put(newId, this); + this.id.set(newId); + } + finally { + this.lock.unlock(); + } + return Mono.empty(); + }) + .subscribeOn(Schedulers.boundedElastic()) + .publishOn(Schedulers.parallel()) + .then(); + } + + @Override + public Mono invalidate() { + this.state.set(State.EXPIRED); + getAttributes().clear(); + InMemoryWebSessionStore.this.sessions.remove(getId()); + return Mono.empty(); + } + @Override public boolean isExpired() { return isExpired(clock.instant());