This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 634ef994d9 Fix BZ 69781 - make load, save and remove thread-safe
634ef994d9 is described below
commit 634ef994d9fe141a886ebf4d7659775d5c838c89
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Aug 27 10:55:23 2025 +0100
Fix BZ 69781 - make load, save and remove thread-safe
https://bz.apache.org/bugzilla/show_bug.cgi?id=69781
---
java/org/apache/catalina/session/FileStore.java | 33 ++++++++++++++++++----
...currency.java => TestFileStoreConcurrency.java} | 2 +-
webapps/docs/changelog.xml | 6 ++++
3 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/java/org/apache/catalina/session/FileStore.java
b/java/org/apache/catalina/session/FileStore.java
index 8b9ac3baba..957b681ffa 100644
--- a/java/org/apache/catalina/session/FileStore.java
+++ b/java/org/apache/catalina/session/FileStore.java
@@ -26,6 +26,7 @@ import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.util.ArrayList;
import java.util.List;
+import java.util.concurrent.locks.Lock;
import jakarta.servlet.ServletContext;
@@ -33,6 +34,7 @@ import org.apache.catalina.Context;
import org.apache.catalina.Session;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.concurrent.KeyedReentrantReadWriteLock;
import org.apache.tomcat.util.res.StringManager;
/**
@@ -69,6 +71,7 @@ public final class FileStore extends StoreBase {
*/
private File directoryFile = null;
+ private KeyedReentrantReadWriteLock sessionLocksById = new
KeyedReentrantReadWriteLock();
/**
* Name to register for this Store, used for logging.
@@ -182,7 +185,7 @@ public final class FileStore extends StoreBase {
public Session load(String id) throws ClassNotFoundException, IOException {
// Open an input stream to the specified pathname, if any
File file = file(id);
- if (file == null || !file.exists()) {
+ if (file == null) {
return null;
}
@@ -195,8 +198,13 @@ public final class FileStore extends StoreBase {
ClassLoader oldThreadContextCL = context.bind(null);
+ Lock readLock = sessionLocksById.getLock(id).readLock();
+ readLock.lock();
try (FileInputStream fis = new FileInputStream(file.getAbsolutePath());
ObjectInputStream ois = getObjectInputStream(fis)) {
+ if (!file.exists()) {
+ return null;
+ }
StandardSession session = (StandardSession)
manager.createEmptySession();
session.readObjectData(ois);
@@ -208,6 +216,7 @@ public final class FileStore extends StoreBase {
}
return null;
} finally {
+ readLock.unlock();
context.unbind(oldThreadContextCL);
}
}
@@ -224,8 +233,14 @@ public final class FileStore extends StoreBase {
.trace(sm.getString(getStoreName() + ".removing", id,
file.getAbsolutePath()));
}
- if (file.exists() && !file.delete()) {
- throw new
IOException(sm.getString("fileStore.deleteSessionFailed", file));
+ Lock writeLock = sessionLocksById.getLock(id).writeLock();
+ writeLock.lock();
+ try {
+ if (file.exists() && !file.delete()) {
+ throw new
IOException(sm.getString("fileStore.deleteSessionFailed", file));
+ }
+ } finally {
+ writeLock.unlock();
}
}
@@ -242,9 +257,15 @@ public final class FileStore extends StoreBase {
.trace(sm.getString(getStoreName() + ".saving",
session.getIdInternal(), file.getAbsolutePath()));
}
- try (FileOutputStream fos = new
FileOutputStream(file.getAbsolutePath());
- ObjectOutputStream oos = new ObjectOutputStream(new
BufferedOutputStream(fos))) {
- ((StandardSession) session).writeObjectData(oos);
+ Lock writeLock =
sessionLocksById.getLock(session.getIdInternal()).writeLock();
+ writeLock.lock();
+ try {
+ try (FileOutputStream fos = new
FileOutputStream(file.getAbsolutePath());
+ ObjectOutputStream oos = new ObjectOutputStream(new
BufferedOutputStream(fos))) {
+ ((StandardSession) session).writeObjectData(oos);
+ }
+ } finally {
+ writeLock.unlock();
}
}
diff --git a/test/org/apache/catalina/session/TesterFileStoreConcurrency.java
b/test/org/apache/catalina/session/TestFileStoreConcurrency.java
similarity index 99%
rename from test/org/apache/catalina/session/TesterFileStoreConcurrency.java
rename to test/org/apache/catalina/session/TestFileStoreConcurrency.java
index 809c213061..502605f731 100644
--- a/test/org/apache/catalina/session/TesterFileStoreConcurrency.java
+++ b/test/org/apache/catalina/session/TestFileStoreConcurrency.java
@@ -37,7 +37,7 @@ import org.apache.tomcat.unittest.TesterContext;
* This needs to be run manually. It will not run as part of the standard unit
tests as it is named "Tester...". This
* could be changed once the bug has been fixed.
*/
-public class TesterFileStoreConcurrency {
+public class TestFileStoreConcurrency {
private static final int TEST_RUN_TIME_MS = 5000;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 91b552b326..34cb21741f 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -176,6 +176,12 @@
Remove a number of unnecessary packages from the catalina-deployer.jar.
(markt)
</scode>
+ <fix>
+ <bug>69781</bug>: Fix concurrent access issues in the session
+ <code>FileStore</code> implementation that were causing lost sessions
+ when the store was used with the <code>PersistentValve</code>. Based on
+ pull request <pr>882</pr> by Aaron Ogburn. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
