Skip to content

[#2713] [3.0] bugfix: Improve thread-safety of Shiro-native sessions by using Atomic* classes #2712

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
steinarb merged 6 commits into from
Jun 1, 2026
Merged

[#2713] [3.0] bugfix: Improve thread-safety of Shiro-native sessions by using Atomic* classes #2712

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a5258fe
bugfix: using atomics for session updates
lprimak May 21, 2026
5b3ad30
simplify getAttributesLazy()
lprimak May 21, 2026
991de8c
enh: code review comments
lprimak May 24, 2026
7aac58c
more code review comments
lprimak May 24, 2026
1a30c96
more code review changes
lprimak May 25, 2026
f847a4c
simplified code, made CachingDAO initialization more concurrency-frie…
lprimak May 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 40 additions & 40 deletions core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,11 @@
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.atomic.AtomicReference;


/**
Expand Down Expand Up @@ -91,18 +94,19 @@ public class SimpleSession implements ValidatingSession, Serializable {
// ==============================================================
private transient Serializable id;
private transient Date startTimestamp;
private transient Date stopTimestamp;
private transient Date lastAccessTime;
private transient long timeout;
private transient boolean expired;
private transient AtomicReference<Date> stopTimestamp;
private transient AtomicReference<Date> lastAccessTime;
private transient AtomicLong timeout;
private transient AtomicBoolean expired = new AtomicBoolean();
private transient String host;
private transient Map<Object, Object> attributes;

public SimpleSession() {
//TODO - remove concrete reference to DefaultSessionManager
this.timeout = DefaultSessionManager.DEFAULT_GLOBAL_SESSION_TIMEOUT;
this.timeout = new AtomicLong(DefaultSessionManager.DEFAULT_GLOBAL_SESSION_TIMEOUT);
this.startTimestamp = new Date();
this.lastAccessTime = this.startTimestamp;
this.stopTimestamp = new AtomicReference<>();
this.lastAccessTime = new AtomicReference<>(this.startTimestamp);
}
Comment thread
lprimak marked this conversation as resolved.

public SimpleSession(String host) {
Expand Down Expand Up @@ -144,19 +148,19 @@ public void setStartTimestamp(Date startTimestamp) {
* active.
*/
public Date getStopTimestamp() {
return stopTimestamp;
return stopTimestamp.get();
}

public void setStopTimestamp(Date stopTimestamp) {
this.stopTimestamp = stopTimestamp;
this.stopTimestamp.set(stopTimestamp);
}

public Date getLastAccessTime() {
return lastAccessTime;
return lastAccessTime.get();
}

public void setLastAccessTime(Date lastAccessTime) {
this.lastAccessTime = lastAccessTime;
this.lastAccessTime.set(lastAccessTime);
}

/**
Expand All @@ -166,19 +170,19 @@ public void setLastAccessTime(Date lastAccessTime) {
* @return true if this session has expired, false otherwise.
*/
public boolean isExpired() {
return expired;
return expired.get();
}

public void setExpired(boolean expired) {
this.expired = expired;
this.expired.set(expired);
}

public long getTimeout() {
return timeout;
return timeout.get();
}

public void setTimeout(long timeout) {
this.timeout = timeout;
this.timeout.set(timeout);
}

public String getHost() {
Expand All @@ -194,17 +198,15 @@ public Map<Object, Object> getAttributes() {
}

public void setAttributes(Map<Object, Object> attributes) {
Comment thread
lprimak marked this conversation as resolved.
this.attributes = attributes;
this.attributes = new ConcurrentHashMap<>(attributes);
}
Comment thread
lprimak marked this conversation as resolved.

public void touch() {
this.lastAccessTime = new Date();
this.lastAccessTime.set(new Date());
}

public void stop() {
if (this.stopTimestamp == null) {
this.stopTimestamp = new Date();
}
stopTimestamp.compareAndSet(null, new Date());
}

protected boolean isStopped() {
Expand All @@ -213,7 +215,7 @@ protected boolean isStopped() {

protected void expire() {
stop();
this.expired = true;
this.expired.set(true);
}

/**
Expand Down Expand Up @@ -301,10 +303,8 @@ public void validate() throws InvalidSessionException {
}

private Map<Object, Object> getAttributesLazy() {
Map<Object, Object> attributes = getAttributes();
if (attributes == null) {
attributes = new HashMap<Object, Object>();
setAttributes(attributes);
attributes = new ConcurrentHashMap<>();
}
return attributes;
Comment thread
lprimak marked this conversation as resolved.
Outdated
}
Expand Down Expand Up @@ -451,17 +451,17 @@ private void writeObject(ObjectOutputStream out) throws IOException {
if (startTimestamp != null) {
out.writeObject(startTimestamp);
}
if (stopTimestamp != null) {
out.writeObject(stopTimestamp);
if (stopTimestamp.get() != null) {
out.writeObject(stopTimestamp.get());
}
if (lastAccessTime != null) {
out.writeObject(lastAccessTime);
if (lastAccessTime.get() != null) {
out.writeObject(lastAccessTime.get());
}
if (timeout != 0L) {
out.writeLong(timeout);
if (timeout.get() != 0L) {
out.writeLong(timeout.get());
}
if (expired) {
out.writeBoolean(expired);
if (expired.get()) {
out.writeBoolean(expired.get());
Comment thread
lprimak marked this conversation as resolved.
Outdated
}
if (host != null) {
out.writeUTF(host);
Expand Down Expand Up @@ -491,16 +491,16 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE
this.startTimestamp = (Date) in.readObject();
}
if (isFieldPresent(bitMask, STOP_TIMESTAMP_BIT_MASK)) {
this.stopTimestamp = (Date) in.readObject();
this.stopTimestamp = new AtomicReference<>((Date) in.readObject());
Comment thread
lprimak marked this conversation as resolved.
}
if (isFieldPresent(bitMask, LAST_ACCESS_TIME_BIT_MASK)) {
this.lastAccessTime = (Date) in.readObject();
this.lastAccessTime = new AtomicReference<>((Date) in.readObject());
Comment thread
lprimak marked this conversation as resolved.
}
if (isFieldPresent(bitMask, TIMEOUT_BIT_MASK)) {
this.timeout = in.readLong();
this.timeout = new AtomicLong(in.readLong());
}
if (isFieldPresent(bitMask, EXPIRED_BIT_MASK)) {
this.expired = in.readBoolean();
this.expired = new AtomicBoolean(in.readBoolean());
}
if (isFieldPresent(bitMask, HOST_BIT_MASK)) {
this.host = in.readUTF();
Expand All @@ -523,10 +523,10 @@ private short getAlteredFieldsBitMask() {
int bitMask = 0;
bitMask = id != null ? bitMask | ID_BIT_MASK : bitMask;
bitMask = startTimestamp != null ? bitMask | START_TIMESTAMP_BIT_MASK : bitMask;
bitMask = stopTimestamp != null ? bitMask | STOP_TIMESTAMP_BIT_MASK : bitMask;
bitMask = lastAccessTime != null ? bitMask | LAST_ACCESS_TIME_BIT_MASK : bitMask;
bitMask = timeout != 0L ? bitMask | TIMEOUT_BIT_MASK : bitMask;
bitMask = expired ? bitMask | EXPIRED_BIT_MASK : bitMask;
bitMask = stopTimestamp.get() != null ? bitMask | STOP_TIMESTAMP_BIT_MASK : bitMask;
bitMask = lastAccessTime.get() != null ? bitMask | LAST_ACCESS_TIME_BIT_MASK : bitMask;
bitMask = timeout.get() != 0L ? bitMask | TIMEOUT_BIT_MASK : bitMask;
bitMask = expired.get() ? bitMask | EXPIRED_BIT_MASK : bitMask;
bitMask = host != null ? bitMask | HOST_BIT_MASK : bitMask;
bitMask = !CollectionUtils.isEmpty(attributes) ? bitMask | ATTRIBUTES_BIT_MASK : bitMask;
return (short) bitMask;
Expand Down
Loading