From a5258fef165859be579d1f265b0646fa376afe9d Mon Sep 17 00:00:00 2001 From: lprimak Date: Wed, 20 May 2026 23:49:21 -0500 Subject: [PATCH 1/5] bugfix: using atomics for session updates --- .../shiro/session/mgt/SimpleSession.java | 81 ++++++++++--------- 1 file changed, 41 insertions(+), 40 deletions(-) diff --git a/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java b/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java index fbe8df02a1..939f3d1375 100644 --- a/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java +++ b/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java @@ -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; /** @@ -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 stopTimestamp; + private transient AtomicReference lastAccessTime; + private transient AtomicLong timeout; + private transient AtomicBoolean expired = new AtomicBoolean(); private transient String host; private transient Map 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); } public SimpleSession(String host) { @@ -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); } /** @@ -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() { @@ -194,17 +198,15 @@ public Map getAttributes() { } public void setAttributes(Map attributes) { - this.attributes = attributes; + this.attributes = new ConcurrentHashMap<>(attributes); } 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() { @@ -213,7 +215,7 @@ protected boolean isStopped() { protected void expire() { stop(); - this.expired = true; + this.expired.set(true); } /** @@ -303,10 +305,9 @@ public void validate() throws InvalidSessionException { private Map getAttributesLazy() { Map attributes = getAttributes(); if (attributes == null) { - attributes = new HashMap(); - setAttributes(attributes); + this.attributes = new ConcurrentHashMap<>(); } - return attributes; + return this.attributes; } public Collection getAttributeKeys() throws InvalidSessionException { @@ -451,17 +452,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()); } if (host != null) { out.writeUTF(host); @@ -491,16 +492,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()); } if (isFieldPresent(bitMask, LAST_ACCESS_TIME_BIT_MASK)) { - this.lastAccessTime = (Date) in.readObject(); + this.lastAccessTime = new AtomicReference<>((Date) in.readObject()); } 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(); @@ -523,10 +524,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; From 5b3ad3061be048574e55628f26c1300ebfc832ed Mon Sep 17 00:00:00 2001 From: lprimak Date: Wed, 20 May 2026 23:58:55 -0500 Subject: [PATCH 2/5] simplify getAttributesLazy() --- .../java/org/apache/shiro/session/mgt/SimpleSession.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java b/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java index 939f3d1375..bc62a20902 100644 --- a/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java +++ b/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java @@ -303,11 +303,10 @@ public void validate() throws InvalidSessionException { } private Map getAttributesLazy() { - Map attributes = getAttributes(); if (attributes == null) { - this.attributes = new ConcurrentHashMap<>(); + attributes = new ConcurrentHashMap<>(); } - return this.attributes; + return attributes; } public Collection getAttributeKeys() throws InvalidSessionException { From 991de8c478460d1c439f877df4f113055f2ad1d6 Mon Sep 17 00:00:00 2001 From: lprimak Date: Sat, 23 May 2026 22:34:20 -0500 Subject: [PATCH 3/5] enh: code review comments --- .../shiro/session/mgt/SimpleSession.java | 25 +++++++++++++++---- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java b/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java index bc62a20902..f7c5c9ba76 100644 --- a/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java +++ b/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java @@ -99,7 +99,7 @@ public class SimpleSession implements ValidatingSession, Serializable { private transient AtomicLong timeout; private transient AtomicBoolean expired = new AtomicBoolean(); private transient String host; - private transient Map attributes; + private transient volatile Map attributes; public SimpleSession() { //TODO - remove concrete reference to DefaultSessionManager @@ -198,7 +198,7 @@ public Map getAttributes() { } public void setAttributes(Map attributes) { - this.attributes = new ConcurrentHashMap<>(attributes); + this.attributes = attributes == null ? null : new ConcurrentHashMap<>(attributes); } public void touch() { @@ -303,10 +303,17 @@ public void validate() throws InvalidSessionException { } private Map getAttributesLazy() { - if (attributes == null) { - attributes = new ConcurrentHashMap<>(); + Map local = attributes; + if (local == null) { + synchronized (this) { + local = attributes; + if (local == null) { + local = new ConcurrentHashMap<>(); + attributes = local; + } + } } - return attributes; + return local; } public Collection getAttributeKeys() throws InvalidSessionException { @@ -492,15 +499,23 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE } if (isFieldPresent(bitMask, STOP_TIMESTAMP_BIT_MASK)) { this.stopTimestamp = new AtomicReference<>((Date) in.readObject()); + } else { + this.stopTimestamp = new AtomicReference<>(); } if (isFieldPresent(bitMask, LAST_ACCESS_TIME_BIT_MASK)) { this.lastAccessTime = new AtomicReference<>((Date) in.readObject()); + } else { + this.lastAccessTime = new AtomicReference<>(); } if (isFieldPresent(bitMask, TIMEOUT_BIT_MASK)) { this.timeout = new AtomicLong(in.readLong()); + } else { + this.timeout = new AtomicLong(); } if (isFieldPresent(bitMask, EXPIRED_BIT_MASK)) { this.expired = new AtomicBoolean(in.readBoolean()); + } else { + this.expired = new AtomicBoolean(); } if (isFieldPresent(bitMask, HOST_BIT_MASK)) { this.host = in.readUTF(); From 7aac58ca9228de134b4aa286d6e1ee3a5a9b572b Mon Sep 17 00:00:00 2001 From: lprimak Date: Sat, 23 May 2026 22:54:41 -0500 Subject: [PATCH 4/5] more code review comments --- .../shiro/session/mgt/SimpleSession.java | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java b/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java index f7c5c9ba76..0d7c85e8f4 100644 --- a/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java +++ b/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java @@ -458,21 +458,31 @@ private void writeObject(ObjectOutputStream out) throws IOException { if (startTimestamp != null) { out.writeObject(startTimestamp); } - if (stopTimestamp.get() != null) { - out.writeObject(stopTimestamp.get()); + + var stopTimestamp = getStopTimestamp(); + if (stopTimestamp != null) { + out.writeObject(stopTimestamp); } - if (lastAccessTime.get() != null) { - out.writeObject(lastAccessTime.get()); + + var lastAccessTime = getLastAccessTime(); + if (lastAccessTime != null) { + out.writeObject(lastAccessTime); } - if (timeout.get() != 0L) { - out.writeLong(timeout.get()); + + var timeout = getTimeout(); + if (timeout != 0L) { + out.writeLong(timeout); } - if (expired.get()) { - out.writeBoolean(expired.get()); + + var expired = isExpired(); + if (expired) { + out.writeBoolean(expired); } if (host != null) { out.writeUTF(host); } + + var attributes = getAttributes(); if (!CollectionUtils.isEmpty(attributes)) { out.writeObject(attributes); } From 1a30c96835780206514fbcd7f003787a8499131c Mon Sep 17 00:00:00 2001 From: lprimak Date: Sun, 24 May 2026 19:56:43 -0500 Subject: [PATCH 5/5] more code review changes --- .../shiro/session/mgt/SimpleSession.java | 47 +++++++++---------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java b/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java index 0d7c85e8f4..251c0fed0f 100644 --- a/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java +++ b/core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java @@ -126,10 +126,6 @@ public Date getStartTimestamp() { return startTimestamp; } - public void setStartTimestamp(Date startTimestamp) { - this.startTimestamp = startTimestamp; - } - /** * Returns the time the session was stopped, or null if the session is still active. *

@@ -151,10 +147,6 @@ public Date getStopTimestamp() { return stopTimestamp.get(); } - public void setStopTimestamp(Date stopTimestamp) { - this.stopTimestamp.set(stopTimestamp); - } - public Date getLastAccessTime() { return lastAccessTime.get(); } @@ -189,16 +181,13 @@ public String getHost() { return host; } - public void setHost(String host) { - this.host = host; - } - public Map getAttributes() { return attributes; } public void setAttributes(Map attributes) { - this.attributes = attributes == null ? null : new ConcurrentHashMap<>(attributes); + this.attributes = attributes == null ? null : attributes instanceof ConcurrentHashMap ? attributes + : new ConcurrentHashMap<>(attributes); } public void touch() { @@ -440,6 +429,10 @@ public String toString() { return sb.toString(); } + void setStartTimestamp(Date startTimestamp) { + this.startTimestamp = startTimestamp; + } + /** * Serializes this object to the specified output stream for JDK Serialization. * @@ -450,7 +443,14 @@ 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); @@ -459,22 +459,18 @@ private void writeObject(ObjectOutputStream out) throws IOException { out.writeObject(startTimestamp); } - var stopTimestamp = getStopTimestamp(); if (stopTimestamp != null) { out.writeObject(stopTimestamp); } - var lastAccessTime = getLastAccessTime(); if (lastAccessTime != null) { out.writeObject(lastAccessTime); } - var timeout = getTimeout(); if (timeout != 0L) { out.writeLong(timeout); } - var expired = isExpired(); if (expired) { out.writeBoolean(expired); } @@ -482,7 +478,6 @@ private void writeObject(ObjectOutputStream out) throws IOException { out.writeUTF(host); } - var attributes = getAttributes(); if (!CollectionUtils.isEmpty(attributes)) { out.writeObject(attributes); } @@ -531,7 +526,7 @@ private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundE this.host = in.readUTF(); } if (isFieldPresent(bitMask, ATTRIBUTES_BIT_MASK)) { - this.attributes = (Map) in.readObject(); + this.attributes = (ConcurrentHashMap) in.readObject(); } } @@ -544,14 +539,15 @@ 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 attributes) { int bitMask = 0; bitMask = id != null ? bitMask | ID_BIT_MASK : bitMask; bitMask = startTimestamp != null ? bitMask | START_TIMESTAMP_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 = 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 = host != null ? bitMask | HOST_BIT_MASK : bitMask; bitMask = !CollectionUtils.isEmpty(attributes) ? bitMask | ATTRIBUTES_BIT_MASK : bitMask; return (short) bitMask; @@ -572,5 +568,4 @@ private short getAlteredFieldsBitMask() { private static boolean isFieldPresent(short bitMask, int fieldBitMask) { return (bitMask & fieldBitMask) != 0; } - }