Avoid race in InMemoryWebSession

Closes gh-36742
This commit is contained in:
rstoyanchev
2026-05-01 15:23:59 +01:00
parent a42a6e0c6a
commit b8ddd2c690
2 changed files with 55 additions and 40 deletions
@@ -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<Void> changeSessionId();
/**
* Invalidate the current session and clear session storage.
* @return completion notification (success or error)
*/
Mono<Void> invalidate();
/**
* Save the session through the {@code WebSessionStore} as follows:
* <ul>
@@ -132,6 +118,20 @@ public interface WebSession {
*/
Mono<Void> 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<Void> changeSessionId();
/**
* Invalidate the current session and clear session storage.
* @return completion notification (success or error)
*/
Mono<Void> invalidate();
/**
* Return {@code true} if the session expired after {@link #getMaxIdleTime()
* maxIdleTime} elapsed.
@@ -184,6 +184,7 @@ public class InMemoryWebSessionStore implements WebSessionStore {
private final AtomicReference<State> 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<Void> changeSessionId() {
return Mono.<Void>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<Void> invalidate() {
this.state.set(State.EXPIRED);
getAttributes().clear();
InMemoryWebSessionStore.this.sessions.remove(this.id.get());
return Mono.empty();
}
@Override
@SuppressWarnings("NullAway")
public Mono<Void> 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<Void> changeSessionId() {
return Mono.<Void>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<Void> invalidate() {
this.state.set(State.EXPIRED);
getAttributes().clear();
InMemoryWebSessionStore.this.sessions.remove(getId());
return Mono.empty();
}
@Override
public boolean isExpired() {
return isExpired(clock.instant());