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

Open
lprimak wants to merge 6 commits into
base: 3.x
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all 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
4 changes: 4 additions & 0 deletions core/src/main/java/org/apache/shiro/session/mgt/DefaultSessionContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,24 @@ public DefaultSessionContext(Map<String, Object> map) {
super(map);
}

@Override
public String getHost() {
return getTypedValue(HOST, String.class);
}

@Override
public void setHost(String host) {
if (StringUtils.hasText(host)) {
put(HOST, host);
}
}

@Override
public Serializable getSessionId() {
return getTypedValue(SESSION_ID, Serializable.class);
}

@Override
public void setSessionId(Serializable sessionId) {
nullSafePut(SESSION_ID, sessionId);
}
Expand Down
131 changes: 85 additions & 46 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,25 +94,27 @@ 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;
private transient volatile 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) {
this();
this.host = host;
}

@Override
public Serializable getId() {
return this.id;
}
Expand All @@ -118,14 +123,11 @@ public void setId(Serializable id) {
this.id = id;
}

@Override
public Date getStartTimestamp() {
return startTimestamp;
}

public void setStartTimestamp(Date startTimestamp) {
this.startTimestamp = startTimestamp;
}

/**
* Returns the time the session was stopped, or <tt>null</tt> if the session is still active.
* <p/>
Expand All @@ -144,19 +146,16 @@ public void setStartTimestamp(Date startTimestamp) {
* active.
*/
public Date getStopTimestamp() {
return stopTimestamp;
}

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

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

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

/**
Expand All @@ -166,45 +165,45 @@ 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);
}

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

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

@Override
public String getHost() {
return host;
}

Comment thread
lprimak marked this conversation as resolved.
public void setHost(String host) {
this.host = host;
}

public Map<Object, Object> getAttributes() {
return attributes;
}

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

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

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

protected boolean isStopped() {
Expand All @@ -213,12 +212,13 @@ protected boolean isStopped() {

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

/**
* @since 0.9
*/
@Override
public boolean isValid() {
return !isStopped() && !isExpired();
}
Expand Down Expand Up @@ -267,6 +267,7 @@ protected boolean isTimedOut() {
return false;
}

@Override
public void validate() throws InvalidSessionException {
//check for stopped:
if (isStopped()) {
Expand Down Expand Up @@ -301,14 +302,20 @@ 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);
Map<Object, Object> local = attributes;
if (local == null) {
synchronized (this) {
local = attributes;
if (local == null) {
local = new ConcurrentHashMap<>();
attributes = local;
}
}
}
return attributes;
return local;
}

@Override
public Collection<Object> getAttributeKeys() throws InvalidSessionException {
Map<Object, Object> attributes = getAttributes();
if (attributes == null) {
Expand All @@ -317,6 +324,7 @@ public Collection<Object> getAttributeKeys() throws InvalidSessionException {
return attributes.keySet();
}

@Override
public Object getAttribute(Object key) {
Map<Object, Object> attributes = getAttributes();
if (attributes == null) {
Expand All @@ -325,6 +333,7 @@ public Object getAttribute(Object key) {
return attributes.get(key);
}

@Override
public void setAttribute(Object key, Object value) {
if (value == null) {
removeAttribute(key);
Expand All @@ -333,6 +342,7 @@ public void setAttribute(Object key, Object value) {
}
}

@Override
public Object removeAttribute(Object key) {
Map<Object, Object> attributes = getAttributes();
if (attributes == null) {
Expand Down Expand Up @@ -433,6 +443,10 @@ public String toString() {
return sb.toString();
}

void setStartTimestamp(Date startTimestamp) {
Comment thread
lprimak marked this conversation as resolved.
this.startTimestamp = startTimestamp;
}

/**
* Serializes this object to the specified output stream for JDK Serialization.
*
Expand All @@ -443,29 +457,41 @@ public String toString() {
@SuppressWarnings("checkstyle:NPathComplexity")
private void writeObject(ObjectOutputStream out) throws IOException {
out.defaultWriteObject();
short alteredFieldsBitMask = getAlteredFieldsBitMask();

var stopTimestamp = getStopTimestamp();
var lastAccessTime = getLastAccessTime();
var timeout = getTimeout();
var expired = isExpired();
var attributes = getAttributes();

short alteredFieldsBitMask = getAlteredFieldsBitMask(stopTimestamp, lastAccessTime, timeout, expired, attributes);
out.writeShort(alteredFieldsBitMask);
if (id != null) {
out.writeObject(id);
}
if (startTimestamp != null) {
out.writeObject(startTimestamp);
}

if (stopTimestamp != null) {
out.writeObject(stopTimestamp);
}

if (lastAccessTime != null) {
out.writeObject(lastAccessTime);
}

if (timeout != 0L) {
out.writeLong(timeout);
}

if (expired) {
out.writeBoolean(expired);
}
if (host != null) {
out.writeUTF(host);
}

if (!CollectionUtils.isEmpty(attributes)) {
out.writeObject(attributes);
}
Expand All @@ -491,22 +517,35 @@ 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.
} else {
this.stopTimestamp = new AtomicReference<>();
}
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.
} else {
this.lastAccessTime = new AtomicReference<>();
}
if (isFieldPresent(bitMask, TIMEOUT_BIT_MASK)) {
this.timeout = in.readLong();
this.timeout = new AtomicLong(in.readLong());
} else {
this.timeout = new AtomicLong();
}
if (isFieldPresent(bitMask, EXPIRED_BIT_MASK)) {
this.expired = in.readBoolean();
this.expired = new AtomicBoolean(in.readBoolean());
} else {
this.expired = new AtomicBoolean();
}
if (isFieldPresent(bitMask, HOST_BIT_MASK)) {
this.host = in.readUTF();
}
if (isFieldPresent(bitMask, ATTRIBUTES_BIT_MASK)) {
this.attributes = (Map<Object, Object>) in.readObject();
var attributes = (Map<Object, Object>) in.readObject();
if (attributes instanceof ConcurrentHashMap<Object, Object>) {
this.attributes = attributes;
} else {
this.attributes = new ConcurrentHashMap<>(attributes);
}
}
}

Expand All @@ -519,7 +558,8 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE
* @since 1.0
*/
@SuppressWarnings("checkstyle:NPathComplexity")
private short getAlteredFieldsBitMask() {
private short getAlteredFieldsBitMask(Date stopTimestamp, Date lastAccessTime, long timeout, boolean expired,
Map<Object, Object> attributes) {
int bitMask = 0;
bitMask = id != null ? bitMask | ID_BIT_MASK : bitMask;
bitMask = startTimestamp != null ? bitMask | START_TIMESTAMP_BIT_MASK : bitMask;
Expand Down Expand Up @@ -547,5 +587,4 @@ private short getAlteredFieldsBitMask() {
private static boolean isFieldPresent(short bitMask, int fieldBitMask) {
return (bitMask & fieldBitMask) != 0;
}

}
8 changes: 1 addition & 7 deletions core/src/main/java/org/apache/shiro/session/mgt/SimpleSessionFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,6 @@ public class SimpleSessionFactory implements SessionFactory {
* @return a new {@link SimpleSession SimpleSession} instance
*/
public Session createSession(SessionContext initData) {
if (initData != null) {
String host = initData.getHost();
if (host != null) {
return new SimpleSession(host);
}
}
return new SimpleSession();
return initData != null ? new SimpleSession(initData.getHost()) : new SimpleSession();
}
}
Loading
Loading