You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2026/06/03 19:23:37 UTC
(tomcat) branch 10.1.x updated (fd19d85024 -> 4b7c7dc47e)
This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a change to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
from fd19d85024 use getEnd() not getLength() when filtering ctls in header writers
new 627250e4bd Fix BZ 70048. Correctly handle asynchronous requests.
new b1c1fbbc24 Add test case for BZ 70048 written by CoPilot / GPT-5.4
new fef6a0e6fa Additional tests written by CoPilot / GPT-5.4
new 4b7c7dc47e Fix async handling
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../apache/catalina/valves/LocalStrings.properties | 1 +
.../apache/catalina/valves/PersistentValve.java | 230 ++++++---
.../catalina/valves/TestPersistentValveAsync.java | 552 +++++++++++++++++++++
webapps/docs/changelog.xml | 4 +
4 files changed, 711 insertions(+), 76 deletions(-)
create mode 100644 test/org/apache/catalina/valves/TestPersistentValveAsync.java
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 04/04: Fix async handling
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 4b7c7dc47e4b0ba0d8d73ae3051ec81ecfb5a6f0
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Jun 3 19:27:44 2026 +0100
Fix async handling
---
.../apache/catalina/valves/PersistentValve.java | 115 ++++++++++++---------
1 file changed, 65 insertions(+), 50 deletions(-)
diff --git a/java/org/apache/catalina/valves/PersistentValve.java b/java/org/apache/catalina/valves/PersistentValve.java
index ed905a661f..aef465abdc 100644
--- a/java/org/apache/catalina/valves/PersistentValve.java
+++ b/java/org/apache/catalina/valves/PersistentValve.java
@@ -132,69 +132,79 @@ public class PersistentValve extends ValveBase {
return;
}
+ boolean asyncOnEntry = request.isAsync();
+
String sessionId = request.getRequestedSessionId();
UsageCountingSemaphore semaphore = null;
boolean mustReleaseSemaphore = true;
try {
- // Acquire the per session semaphore
- if (sessionId != null) {
- semaphore = sessionToSemaphoreMap.compute(sessionId,
- (k, v) -> v == null ? new UsageCountingSemaphore(semaphoreFairness) : v.incrementUsageCount());
- if (semaphoreBlockOnAcquire) {
- if (semaphoreAcquireUninterruptibly) {
- semaphore.acquireUninterruptibly();
+ /*
+ * If the request was in asynchronous mode when it entered the Valve, the semaphore was acquired during the
+ * original request where asynchronous processing started and does not need to be acquired again.
+ */
+ if (!asyncOnEntry) {
+ /*
+ * Acquire the per session semaphore.
+ */
+ if (sessionId != null) {
+ semaphore = sessionToSemaphoreMap.compute(sessionId,
+ (k, v) -> v == null ? new UsageCountingSemaphore(semaphoreFairness) : v.incrementUsageCount());
+ if (semaphoreBlockOnAcquire) {
+ if (semaphoreAcquireUninterruptibly) {
+ semaphore.acquireUninterruptibly();
+ } else {
+ try {
+ semaphore.acquire();
+ } catch (InterruptedException e) {
+ mustReleaseSemaphore = false;
+ onSemaphoreNotAcquired(request, response);
+ if (containerLog.isDebugEnabled()) {
+ containerLog.debug(sm.getString("persistentValve.acquireInterrupted",
+ request.getDecodedRequestURI()));
+ }
+ return;
+ }
+ }
} else {
- try {
- semaphore.acquire();
- } catch (InterruptedException e) {
+ if (!semaphore.tryAcquire()) {
mustReleaseSemaphore = false;
onSemaphoreNotAcquired(request, response);
if (containerLog.isDebugEnabled()) {
- containerLog.debug(sm.getString("persistentValve.acquireInterrupted",
- request.getDecodedRequestURI()));
+ containerLog.debug(
+ sm.getString("persistentValve.acquireFailed", request.getDecodedRequestURI()));
}
return;
}
}
- } else {
- if (!semaphore.tryAcquire()) {
- mustReleaseSemaphore = false;
- onSemaphoreNotAcquired(request, response);
- if (containerLog.isDebugEnabled()) {
- containerLog.debug(
- sm.getString("persistentValve.acquireFailed", request.getDecodedRequestURI()));
- }
- return;
- }
}
- }
- // Update the session last access time for our session (if any)
- Manager manager = context.getManager();
- if (sessionId != null && manager instanceof StoreManager) {
- Store store = ((StoreManager) manager).getStore();
- if (store != null) {
- Session session = null;
- try {
- session = store.load(sessionId);
- } catch (Exception e) {
- containerLog.error(sm.getString("persistentValve.sessionLoadFail", sessionId));
- }
- if (session != null) {
- if (!session.isValid() || isSessionStale(session, System.currentTimeMillis())) {
- if (containerLog.isTraceEnabled()) {
- containerLog.trace("session swapped in is invalid or expired");
+ // Update the session last access time for our session (if any)
+ Manager manager = context.getManager();
+ if (sessionId != null && manager instanceof StoreManager) {
+ Store store = ((StoreManager) manager).getStore();
+ if (store != null) {
+ Session session = null;
+ try {
+ session = store.load(sessionId);
+ } catch (Exception e) {
+ containerLog.error(sm.getString("persistentValve.sessionLoadFail", sessionId));
+ }
+ if (session != null) {
+ if (!session.isValid() || isSessionStale(session, System.currentTimeMillis())) {
+ if (containerLog.isTraceEnabled()) {
+ containerLog.trace("session swapped in is invalid or expired");
+ }
+ session.expire();
+ store.remove(sessionId);
+ } else {
+ session.setManager(manager);
+ // session.setId(sessionId); Only if new ???
+ manager.add(session);
+ // ((StandardSession)session).activate();
+ session.access();
+ session.endAccess();
}
- session.expire();
- store.remove(sessionId);
- } else {
- session.setManager(manager);
- // session.setId(sessionId); Only if new ???
- manager.add(session);
- // ((StandardSession)session).activate();
- session.access();
- session.endAccess();
}
}
}
@@ -211,10 +221,15 @@ public class PersistentValve extends ValveBase {
* Need to continue to hold the semaphore until asynchronous processing is complete. Register a listener
* that will release the Semaphore once asynchronous processing is complete. Also need to delay session
* persistence until completion of the asynchronous processing.
+ *
+ * The listener must only be added once so it is only added when the request was not in asynchronous
+ * mode on entry.
*/
- AsyncContext asyncContext = request.getAsyncContext();
- asyncContext.addListener(
- new StoreSessionAsyncListener(request, context, sessionId, semaphore, mustReleaseSemaphore));
+ if (!asyncOnEntry) {
+ AsyncContext asyncContext = request.getAsyncContext();
+ asyncContext.addListener(
+ new StoreSessionAsyncListener(request, context, sessionId, semaphore, mustReleaseSemaphore));
+ }
} else {
storeSession(request, context, sessionId, semaphore, mustReleaseSemaphore);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 03/04: Additional tests written by CoPilot / GPT-5.4
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit fef6a0e6fa4a865e318d4c6cf28c00f1edf72975
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Jun 3 19:27:33 2026 +0100
Additional tests written by CoPilot / GPT-5.4
---
.../catalina/valves/TestPersistentValveAsync.java | 301 ++++++++++++++++++---
1 file changed, 257 insertions(+), 44 deletions(-)
diff --git a/test/org/apache/catalina/valves/TestPersistentValveAsync.java b/test/org/apache/catalina/valves/TestPersistentValveAsync.java
index 9910982f89..b852917857 100644
--- a/test/org/apache/catalina/valves/TestPersistentValveAsync.java
+++ b/test/org/apache/catalina/valves/TestPersistentValveAsync.java
@@ -28,6 +28,7 @@ import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import jakarta.servlet.AsyncContext;
+import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
@@ -48,6 +49,9 @@ import org.apache.tomcat.util.buf.ByteChunk;
public class TestPersistentValveAsync extends TomcatBaseTest {
+ private static final String TEST_SESSION_ID = "TEST-SESSION-ID";
+ private static final long TEST_TIMEOUT_MS = 1000;
+
@Test
public void testAsyncRequestStoresSessionOnComplete() throws Exception {
Tomcat tomcat = getTomcatInstance();
@@ -98,66 +102,61 @@ public class TestPersistentValveAsync extends TomcatBaseTest {
@Test
public void testSemaphoreHeldWhileAsyncRequestInProgress() throws Exception {
+ CountDownLatch asyncStarted = new CountDownLatch(1);
+ CountDownLatch allowAsyncComplete = new CountDownLatch(1);
+
Tomcat tomcat = getTomcatInstance();
- StandardContext context = (StandardContext) getProgrammaticRootContext();
- context.setDistributable(true);
+ addSemaphoreTestServlets("async-block",
+ new BlockingAsyncServlet(asyncStarted, allowAsyncComplete), "/async-block");
+ tomcat.start();
+
+ assertSemaphoreHeldUntilAsyncRequestCompletes("/async-block", asyncStarted, allowAsyncComplete,
+ HttpServletResponse.SC_OK, TEST_SESSION_ID);
+ }
- Tomcat.addServlet(context, "session", new SessionServlet());
- context.addServletMappingDecoded("/session", "session");
+ @Test
+ public void testSemaphoreHeldAcrossAsyncDispatchDispatchComplete() throws Exception {
CountDownLatch asyncStarted = new CountDownLatch(1);
CountDownLatch allowAsyncComplete = new CountDownLatch(1);
- Wrapper asyncWrapper =
- Tomcat.addServlet(context, "async-block", new BlockingAsyncServlet(asyncStarted, allowAsyncComplete));
- asyncWrapper.setAsyncSupported(true);
- context.addServletMappingDecoded("/async-block", "async-block");
-
- TesterStore store = new TesterStore();
- PersistentValve persistentValve = new PersistentValve();
- persistentValve.setSemaphoreBlockOnAcquire(false);
- configurePersistentManager(context, store, persistentValve);
+ Tomcat tomcat = getTomcatInstance();
+ addSemaphoreTestServlets("async-dispatch-dispatch-complete",
+ new DispatchingAsyncServlet(2, asyncStarted, allowAsyncComplete, TerminalAction.COMPLETE), "/async-ddc");
tomcat.start();
- String sessionId = "TEST-SESSION-ID";
-
- Map<String,List<String>> requestHeaders = cookieHeaders(sessionId);
+ assertSemaphoreHeldUntilAsyncRequestCompletes("/async-ddc", asyncStarted, allowAsyncComplete,
+ HttpServletResponse.SC_OK, TEST_SESSION_ID);
+ }
- ByteChunk asyncResponseBody = new ByteChunk();
- AtomicInteger asyncResponseCode = new AtomicInteger(-1);
- AtomicReference<Throwable> asyncFailure = new AtomicReference<>();
- Thread asyncClientThread = new Thread(() -> {
- try {
- asyncResponseCode.set(getUrl("http://localhost:" + getPort() + "/async-block", asyncResponseBody,
- requestHeaders, null));
- } catch (Throwable t) {
- asyncFailure.set(t);
- }
- });
- asyncClientThread.start();
+ @Test
+ public void testSemaphoreHeldUntilAsyncError() throws Exception {
+ CountDownLatch asyncStarted = new CountDownLatch(1);
+ CountDownLatch allowAsyncError = new CountDownLatch(1);
- Assert.assertTrue(asyncStarted.await(10, TimeUnit.SECONDS));
+ Tomcat tomcat = getTomcatInstance();
+ StandardContext context = addSemaphoreTestServlets("async-error",
+ new ErrorDispatchingAsyncServlet(asyncStarted, allowAsyncError), "/async-error");
+ Tomcat.addServlet(context, "async-error-target", new ErrorServlet());
+ context.addServletMappingDecoded("/async-error-target", "async-error-target");
+ tomcat.start();
- ByteChunk rejectedOne = new ByteChunk();
- int rejectedOneStatus = getUrl("http://localhost:" + getPort() + "/session", rejectedOne, requestHeaders, null);
- Assert.assertEquals(HttpServletResponse.SC_TOO_MANY_REQUESTS, rejectedOneStatus);
+ assertSemaphoreHeldUntilAsyncRequestCompletes("/async-error", asyncStarted, allowAsyncError,
+ HttpServletResponse.SC_INTERNAL_SERVER_ERROR, null);
+ }
- ByteChunk rejectedTwo = new ByteChunk();
- int rejectedTwoStatus = getUrl("http://localhost:" + getPort() + "/session", rejectedTwo, requestHeaders, null);
- Assert.assertEquals(HttpServletResponse.SC_TOO_MANY_REQUESTS, rejectedTwoStatus);
- allowAsyncComplete.countDown();
- asyncClientThread.join(10000);
+ @Test
+ public void testSemaphoreHeldUntilAsyncTimeout() throws Exception {
+ CountDownLatch asyncStarted = new CountDownLatch(1);
- Assert.assertNull(asyncFailure.get());
- Assert.assertEquals(HttpServletResponse.SC_OK, asyncResponseCode.get());
- Assert.assertEquals(sessionId, asyncResponseBody.toString());
+ Tomcat tomcat = getTomcatInstance();
+ addSemaphoreTestServlets("async-timeout", new TimeoutAsyncServlet(asyncStarted), "/async-timeout");
+ tomcat.start();
- ByteChunk success = new ByteChunk();
- int successStatus = getUrl("http://localhost:" + getPort() + "/session", success, requestHeaders, null);
- Assert.assertEquals(HttpServletResponse.SC_OK, successStatus);
- Assert.assertFalse(success.isNull());
+ assertSemaphoreHeldUntilAsyncRequestCompletes("/async-timeout", asyncStarted, null,
+ HttpServletResponse.SC_INTERNAL_SERVER_ERROR, null);
}
@@ -173,6 +172,27 @@ public class TestPersistentValveAsync extends TomcatBaseTest {
}
+ private StandardContext addSemaphoreTestServlets(String asyncServletName, HttpServlet asyncServlet,
+ String asyncPath) {
+ StandardContext context = (StandardContext) getProgrammaticRootContext();
+ context.setDistributable(true);
+
+ Tomcat.addServlet(context, "session", new SessionServlet());
+ context.addServletMappingDecoded("/session", "session");
+
+ Wrapper asyncWrapper = Tomcat.addServlet(context, asyncServletName, asyncServlet);
+ asyncWrapper.setAsyncSupported(true);
+ context.addServletMappingDecoded(asyncPath, asyncServletName);
+
+ TesterStore store = new TesterStore();
+ PersistentValve persistentValve = new PersistentValve();
+ persistentValve.setSemaphoreBlockOnAcquire(false);
+ configurePersistentManager(context, store, persistentValve);
+
+ return context;
+ }
+
+
private Map<String,List<String>> cookieHeaders(String sessionId) {
Map<String,List<String>> result = new HashMap<>();
result.put("Cookie", List.of("JSESSIONID=" + sessionId));
@@ -180,6 +200,43 @@ public class TestPersistentValveAsync extends TomcatBaseTest {
}
+ private void assertSemaphoreHeldUntilAsyncRequestCompletes(String path, CountDownLatch asyncStarted,
+ CountDownLatch allowTerminalAction, int expectedStatus, String expectedResponseBody) throws Exception {
+ Map<String,List<String>> requestHeaders = cookieHeaders(TEST_SESSION_ID);
+
+ AsyncRequest asyncRequest = new AsyncRequest(path, requestHeaders);
+ asyncRequest.start();
+
+ Assert.assertTrue(asyncStarted.await(10, TimeUnit.SECONDS));
+
+ assertSessionRequestRejected(requestHeaders);
+ assertSessionRequestRejected(requestHeaders);
+
+ if (allowTerminalAction != null) {
+ allowTerminalAction.countDown();
+ }
+
+ asyncRequest.await();
+ asyncRequest.assertResponse(expectedStatus, expectedResponseBody);
+ assertSessionRequestSucceeds(requestHeaders);
+ }
+
+
+ private void assertSessionRequestRejected(Map<String,List<String>> requestHeaders) throws Exception {
+ ByteChunk rejected = new ByteChunk();
+ int status = getUrl("http://localhost:" + getPort() + "/session", rejected, requestHeaders, null);
+ Assert.assertEquals(429, status);
+ }
+
+
+ private void assertSessionRequestSucceeds(Map<String,List<String>> requestHeaders) throws Exception {
+ ByteChunk success = new ByteChunk();
+ int status = getUrl("http://localhost:" + getPort() + "/session", success, requestHeaders, null);
+ Assert.assertEquals(HttpServletResponse.SC_OK, status);
+ Assert.assertFalse(success.isNull());
+ }
+
+
private static class SessionServlet extends HttpServlet {
private static final long serialVersionUID = 1L;
@@ -275,6 +332,162 @@ public class TestPersistentValveAsync extends TomcatBaseTest {
}
+ private static class DispatchingAsyncServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ private final int dispatchCount;
+ private final CountDownLatch asyncStarted;
+ private final CountDownLatch allowTerminalAction;
+ private final TerminalAction terminalAction;
+
+ private DispatchingAsyncServlet(int dispatchCount, CountDownLatch asyncStarted, CountDownLatch allowTerminalAction,
+ TerminalAction terminalAction) {
+ this.dispatchCount = dispatchCount;
+ this.asyncStarted = asyncStarted;
+ this.allowTerminalAction = allowTerminalAction;
+ this.terminalAction = terminalAction;
+ }
+
+ @Override
+ protected void doGet(HttpServletRequest request, HttpServletResponse response) {
+ Integer dispatches = (Integer) request.getAttribute("dispatches");
+ int dispatchesSoFar = dispatches == null ? 0 : dispatches.intValue();
+
+ if (dispatchesSoFar < dispatchCount) {
+ request.setAttribute("dispatches", Integer.valueOf(dispatchesSoFar + 1));
+ AsyncContext asyncContext = request.startAsync();
+ asyncContext.start(asyncContext::dispatch);
+ return;
+ }
+
+ if (terminalAction == TerminalAction.COMPLETE) {
+ String sessionId = request.getRequestedSessionId();
+ AsyncContext asyncContext = request.startAsync();
+ asyncContext.start(() -> {
+ asyncStarted.countDown();
+ try {
+ Assert.assertTrue(allowTerminalAction.await(10, TimeUnit.SECONDS));
+ response.getWriter().print(sessionId);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new IllegalStateException(e);
+ } catch (IOException e) {
+ throw new IllegalStateException(e);
+ } finally {
+ asyncContext.complete();
+ }
+ });
+ }
+ }
+ }
+
+
+ private static class ErrorDispatchingAsyncServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ private final CountDownLatch asyncStarted;
+ private final CountDownLatch allowAsyncError;
+
+ private ErrorDispatchingAsyncServlet(CountDownLatch asyncStarted, CountDownLatch allowAsyncError) {
+ this.asyncStarted = asyncStarted;
+ this.allowAsyncError = allowAsyncError;
+ }
+
+ @Override
+ protected void doGet(HttpServletRequest request, HttpServletResponse response) {
+ AsyncContext asyncContext = request.startAsync();
+ asyncContext.start(() -> {
+ asyncStarted.countDown();
+ try {
+ Assert.assertTrue(allowAsyncError.await(10, TimeUnit.SECONDS));
+ asyncContext.dispatch("/async-error-target");
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new IllegalStateException(e);
+ }
+ });
+ }
+ }
+
+
+ private static class TimeoutAsyncServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ private final CountDownLatch asyncStarted;
+
+ private TimeoutAsyncServlet(CountDownLatch asyncStarted) {
+ this.asyncStarted = asyncStarted;
+ }
+
+ @Override
+ protected void doGet(HttpServletRequest request, HttpServletResponse response) {
+ AsyncContext asyncContext = request.startAsync();
+ asyncContext.setTimeout(TEST_TIMEOUT_MS);
+ asyncStarted.countDown();
+ }
+ }
+
+
+ private static class ErrorServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException {
+ throw new ServletException("Async test error");
+ }
+ }
+
+
+ private class AsyncRequest {
+
+ private final String path;
+ private final Map<String,List<String>> requestHeaders;
+ private final ByteChunk responseBody = new ByteChunk();
+ private final AtomicInteger responseCode = new AtomicInteger(-1);
+ private final AtomicReference<Throwable> failure = new AtomicReference<>();
+ private final Thread thread;
+
+ private AsyncRequest(String path, Map<String,List<String>> requestHeaders) {
+ this.path = path;
+ this.requestHeaders = requestHeaders;
+ this.thread = new Thread(this::doRequest);
+ }
+
+ private void start() {
+ thread.start();
+ }
+
+ private void await() throws InterruptedException {
+ thread.join(10000);
+ }
+
+ private void assertResponse(int expectedStatus, String expectedResponseBody) {
+ Assert.assertNull(failure.get());
+ Assert.assertEquals(expectedStatus, responseCode.get());
+ if (expectedResponseBody != null) {
+ Assert.assertEquals(expectedResponseBody, responseBody.toString());
+ }
+ }
+
+ private void doRequest() {
+ try {
+ responseCode.set(getUrl("http://localhost:" + getPort() + path, responseBody, requestHeaders, null));
+ } catch (Throwable t) {
+ failure.set(t);
+ }
+ }
+ }
+
+
+ private enum TerminalAction {
+ COMPLETE
+ }
+
+
private static class TesterStore implements Store {
private Manager manager;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 01/04: Fix BZ 70048. Correctly handle asynchronous requests.
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 627250e4bd628d21341b42769c93173a80508851
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Jun 3 17:55:00 2026 +0100
Fix BZ 70048. Correctly handle asynchronous requests.
---
.../apache/catalina/valves/LocalStrings.properties | 1 +
.../apache/catalina/valves/PersistentValve.java | 121 ++++++++++++++++-----
webapps/docs/changelog.xml | 4 +
3 files changed, 97 insertions(+), 29 deletions(-)
diff --git a/java/org/apache/catalina/valves/LocalStrings.properties b/java/org/apache/catalina/valves/LocalStrings.properties
index 908cb6f3ab..771bcc6f63 100644
--- a/java/org/apache/catalina/valves/LocalStrings.properties
+++ b/java/org/apache/catalina/valves/LocalStrings.properties
@@ -150,6 +150,7 @@ persistentValve.filter.failure=Unable to compile filter=[{0}]
persistentValve.requestIgnore=The request for [{0}] was ignored by this Valve as it matches the configured filter
persistentValve.requestProcess=The request for [{0}] will be processed by this Valve as it does not match the configured filter
persistentValve.sessionLoadFail=Loading session [{0}] from the store failed
+persistentValve.sessionSaveFail=Saving session [{0}] to the store failed
proxyErrorReportValve.error=Proxy error to [{0}]
diff --git a/java/org/apache/catalina/valves/PersistentValve.java b/java/org/apache/catalina/valves/PersistentValve.java
index 0e171a13bb..ed905a661f 100644
--- a/java/org/apache/catalina/valves/PersistentValve.java
+++ b/java/org/apache/catalina/valves/PersistentValve.java
@@ -24,6 +24,9 @@ import java.util.concurrent.atomic.AtomicLong;
import java.util.regex.Pattern;
import java.util.regex.PatternSyntaxException;
+import jakarta.servlet.AsyncContext;
+import jakarta.servlet.AsyncEvent;
+import jakarta.servlet.AsyncListener;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletResponse;
@@ -202,31 +205,48 @@ public class PersistentValve extends ValveBase {
// Ask the next valve to process the request.
getNext().invoke(request, response);
+ } finally {
+ if (request.isAsync()) {
+ /*
+ * Need to continue to hold the semaphore until asynchronous processing is complete. Register a listener
+ * that will release the Semaphore once asynchronous processing is complete. Also need to delay session
+ * persistence until completion of the asynchronous processing.
+ */
+ AsyncContext asyncContext = request.getAsyncContext();
+ asyncContext.addListener(
+ new StoreSessionAsyncListener(request, context, sessionId, semaphore, mustReleaseSemaphore));
+ } else {
+ storeSession(request, context, sessionId, semaphore, mustReleaseSemaphore);
+ }
+ }
+ }
- // If still processing async, don't try to store the session
- if (!request.isAsync()) {
- // Read the sessionid after the response.
- // HttpSession hsess = hreq.getSession(false);
- Session hsess;
- try {
- hsess = request.getSessionInternal(false);
- } catch (Exception e) {
- hsess = null;
- }
- String newsessionId = null;
- if (hsess != null) {
- newsessionId = hsess.getIdInternal();
- }
- if (containerLog.isTraceEnabled()) {
- containerLog.trace("newsessionId: " + newsessionId);
- }
- if (newsessionId != null) {
- try {
- bind(context);
+ private void storeSession(Request request, Context context, String originalSessionId,
+ UsageCountingSemaphore semaphore, boolean mustReleaseSemaphore) {
+ try {
+ Manager manager = context.getManager();
+ Session hsess;
+ try {
+ hsess = request.getSessionInternal(false);
+ } catch (Exception e) {
+ hsess = null;
+ }
+ String newsessionId = null;
+ if (hsess != null) {
+ newsessionId = hsess.getIdInternal();
+ }
+
+ if (containerLog.isTraceEnabled()) {
+ containerLog.trace("newsessionId: " + newsessionId);
+ }
+ if (newsessionId != null) {
+ try {
+ bind(context);
- /* store the session and remove it from the manager */
- if (manager instanceof StoreManager) {
+ /* store the session and remove it from the manager */
+ if (manager instanceof StoreManager) {
+ try {
Session session = manager.findSession(newsessionId);
Store store = ((StoreManager) manager).getStore();
boolean stored = false;
@@ -247,14 +267,16 @@ public class PersistentValve extends ValveBase {
" stale: " + isSessionStale(session, System.currentTimeMillis()));
}
}
- } else {
- if (containerLog.isTraceEnabled()) {
- containerLog.trace("newsessionId Manager: " + manager);
- }
+ } catch (IOException ioe) {
+ containerLog.warn(sm.getString("persistentValve.sessionSaveFail", newsessionId));
+ }
+ } else {
+ if (containerLog.isTraceEnabled()) {
+ containerLog.trace("newsessionId Manager: " + manager);
}
- } finally {
- unbind(context);
}
+ } finally {
+ unbind(context);
}
}
} finally {
@@ -262,7 +284,7 @@ public class PersistentValve extends ValveBase {
if (mustReleaseSemaphore) {
semaphore.release();
}
- sessionToSemaphoreMap.computeIfPresent(sessionId,
+ sessionToSemaphoreMap.computeIfPresent(originalSessionId,
(k, v) -> v.decrementAndGetUsageCount() == 0 ? null : v);
}
}
@@ -322,6 +344,7 @@ public class PersistentValve extends ValveBase {
* Determines whether the given URI should bypass session persistence based on the configured filter.
*
* @param uri the request URI to check
+ *
* @return {@code true} if the request should bypass session persistence, otherwise {@code false}
*/
protected boolean isRequestWithoutSession(String uri) {
@@ -466,4 +489,44 @@ public class PersistentValve extends ValveBase {
semaphore.release();
}
}
+
+
+ private class StoreSessionAsyncListener implements AsyncListener {
+
+ private final Request request;
+ private final Context context;
+ private final String originalSessionId;
+ private final UsageCountingSemaphore semaphore;
+ private final boolean mustReleaseSemaphore;
+
+ StoreSessionAsyncListener(Request request, Context context, String originalSessionId,
+ UsageCountingSemaphore semaphore, boolean mustReleaseSemaphore) {
+ this.request = request;
+ this.context = context;
+ this.originalSessionId = originalSessionId;
+ this.semaphore = semaphore;
+ this.mustReleaseSemaphore = mustReleaseSemaphore;
+ }
+
+
+ @Override
+ public void onComplete(AsyncEvent event) throws IOException {
+ storeSession(request, context, originalSessionId, semaphore, mustReleaseSemaphore);
+ }
+
+ @Override
+ public void onTimeout(AsyncEvent event) throws IOException {
+ // NO-OP.
+ }
+
+ @Override
+ public void onError(AsyncEvent event) throws IOException {
+ // NO-OP.
+ }
+
+ @Override
+ public void onStartAsync(AsyncEvent event) throws IOException {
+ event.getAsyncContext().addListener(this);
+ }
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 3978b76e2c..5b9032ec5f 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -231,6 +231,10 @@
Fix <code>CombinedRealm</code> <code>isAvailable</code>, it allows
authentication if at least one sub realm is available. (remm)
</fix>
+ <fix>
+ <bug>70048</bug>: Correctly handle asynchronous requests in
+ <code>PersistentValve</code>. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 02/04: Add test case for BZ 70048 written by CoPilot / GPT-5.4
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit b1c1fbbc2406879e903e1393a9fb4e4948158d87
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Wed Jun 3 17:55:33 2026 +0100
Add test case for BZ 70048 written by CoPilot / GPT-5.4
---
.../catalina/valves/TestPersistentValveAsync.java | 339 +++++++++++++++++++++
1 file changed, 339 insertions(+)
diff --git a/test/org/apache/catalina/valves/TestPersistentValveAsync.java b/test/org/apache/catalina/valves/TestPersistentValveAsync.java
new file mode 100644
index 0000000000..9910982f89
--- /dev/null
+++ b/test/org/apache/catalina/valves/TestPersistentValveAsync.java
@@ -0,0 +1,339 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.valves;
+
+import java.beans.PropertyChangeListener;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
+
+import jakarta.servlet.AsyncContext;
+import jakarta.servlet.http.HttpServlet;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import jakarta.servlet.http.HttpSession;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.catalina.Manager;
+import org.apache.catalina.Session;
+import org.apache.catalina.Store;
+import org.apache.catalina.Wrapper;
+import org.apache.catalina.core.StandardContext;
+import org.apache.catalina.session.PersistentManager;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.tomcat.util.buf.ByteChunk;
+
+public class TestPersistentValveAsync extends TomcatBaseTest {
+
+ @Test
+ public void testAsyncRequestStoresSessionOnComplete() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+ StandardContext context = (StandardContext) getProgrammaticRootContext();
+ context.setDistributable(true);
+
+ Wrapper wrapper = Tomcat.addServlet(context, "async-complete", new AsyncCompleteServlet());
+ wrapper.setAsyncSupported(true);
+ context.addServletMappingDecoded("/async-complete", "async-complete");
+
+ TesterStore store = new TesterStore();
+ PersistentManager manager = configurePersistentManager(context, store, new PersistentValve());
+
+ tomcat.start();
+
+ ByteChunk responseBody = getUrl("http://localhost:" + getPort() + "/async-complete");
+ String sessionId = responseBody.toString();
+
+ Assert.assertEquals(List.of(sessionId), store.getSavedIds());
+ Assert.assertNotNull(store.load(sessionId));
+ Assert.assertEquals(0, manager.getActiveSessions());
+ }
+
+
+ @Test
+ public void testSecondAsyncCycleStoresSessionOnFinalComplete() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+ StandardContext context = (StandardContext) getProgrammaticRootContext();
+ context.setDistributable(true);
+
+ Wrapper wrapper = Tomcat.addServlet(context, "async-dispatch", new AsyncDispatchServlet());
+ wrapper.setAsyncSupported(true);
+ context.addServletMappingDecoded("/async-dispatch", "async-dispatch");
+
+ TesterStore store = new TesterStore();
+ PersistentManager manager = configurePersistentManager(context, store, new PersistentValve());
+
+ tomcat.start();
+
+ ByteChunk responseBody = getUrl("http://localhost:" + getPort() + "/async-dispatch");
+ String sessionId = responseBody.toString();
+
+ Assert.assertEquals(List.of(sessionId), store.getSavedIds());
+ Assert.assertNotNull(store.load(sessionId));
+ Assert.assertEquals(0, manager.getActiveSessions());
+ }
+
+
+ @Test
+ public void testSemaphoreHeldWhileAsyncRequestInProgress() throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+ StandardContext context = (StandardContext) getProgrammaticRootContext();
+ context.setDistributable(true);
+
+ Tomcat.addServlet(context, "session", new SessionServlet());
+ context.addServletMappingDecoded("/session", "session");
+
+ CountDownLatch asyncStarted = new CountDownLatch(1);
+ CountDownLatch allowAsyncComplete = new CountDownLatch(1);
+ Wrapper asyncWrapper =
+ Tomcat.addServlet(context, "async-block", new BlockingAsyncServlet(asyncStarted, allowAsyncComplete));
+ asyncWrapper.setAsyncSupported(true);
+ context.addServletMappingDecoded("/async-block", "async-block");
+
+ TesterStore store = new TesterStore();
+ PersistentValve persistentValve = new PersistentValve();
+ persistentValve.setSemaphoreBlockOnAcquire(false);
+ configurePersistentManager(context, store, persistentValve);
+
+ tomcat.start();
+
+ String sessionId = "TEST-SESSION-ID";
+
+ Map<String,List<String>> requestHeaders = cookieHeaders(sessionId);
+
+ ByteChunk asyncResponseBody = new ByteChunk();
+ AtomicInteger asyncResponseCode = new AtomicInteger(-1);
+ AtomicReference<Throwable> asyncFailure = new AtomicReference<>();
+ Thread asyncClientThread = new Thread(() -> {
+ try {
+ asyncResponseCode.set(getUrl("http://localhost:" + getPort() + "/async-block", asyncResponseBody,
+ requestHeaders, null));
+ } catch (Throwable t) {
+ asyncFailure.set(t);
+ }
+ });
+
+ asyncClientThread.start();
+
+ Assert.assertTrue(asyncStarted.await(10, TimeUnit.SECONDS));
+
+ ByteChunk rejectedOne = new ByteChunk();
+ int rejectedOneStatus = getUrl("http://localhost:" + getPort() + "/session", rejectedOne, requestHeaders, null);
+ Assert.assertEquals(HttpServletResponse.SC_TOO_MANY_REQUESTS, rejectedOneStatus);
+
+ ByteChunk rejectedTwo = new ByteChunk();
+ int rejectedTwoStatus = getUrl("http://localhost:" + getPort() + "/session", rejectedTwo, requestHeaders, null);
+ Assert.assertEquals(HttpServletResponse.SC_TOO_MANY_REQUESTS, rejectedTwoStatus);
+
+ allowAsyncComplete.countDown();
+ asyncClientThread.join(10000);
+
+ Assert.assertNull(asyncFailure.get());
+ Assert.assertEquals(HttpServletResponse.SC_OK, asyncResponseCode.get());
+ Assert.assertEquals(sessionId, asyncResponseBody.toString());
+
+ ByteChunk success = new ByteChunk();
+ int successStatus = getUrl("http://localhost:" + getPort() + "/session", success, requestHeaders, null);
+ Assert.assertEquals(HttpServletResponse.SC_OK, successStatus);
+ Assert.assertFalse(success.isNull());
+ }
+
+
+ private PersistentManager configurePersistentManager(StandardContext context, TesterStore store,
+ PersistentValve valve) {
+ PersistentManager manager = new PersistentManager();
+ manager.setStore(store);
+ manager.setMaxIdleBackup(0);
+ manager.setSessionActivityCheck(true);
+ context.setManager(manager);
+ context.addValve(valve);
+ return manager;
+ }
+
+
+ private Map<String,List<String>> cookieHeaders(String sessionId) {
+ Map<String,List<String>> result = new HashMap<>();
+ result.put("Cookie", List.of("JSESSIONID=" + sessionId));
+ return result;
+ }
+
+
+ private static class SessionServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException {
+ HttpSession session = request.getSession();
+ response.getWriter().print(session.getId());
+ }
+ }
+
+
+ private static class AsyncCompleteServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doGet(HttpServletRequest request, HttpServletResponse response) {
+ HttpSession session = request.getSession();
+ AsyncContext asyncContext = request.startAsync();
+ asyncContext.start(() -> {
+ try {
+ response.getWriter().print(session.getId());
+ } catch (IOException e) {
+ throw new IllegalStateException(e);
+ } finally {
+ asyncContext.complete();
+ }
+ });
+ }
+ }
+
+
+ private static class AsyncDispatchServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ protected void doGet(HttpServletRequest request, HttpServletResponse response) {
+ HttpSession session = request.getSession();
+
+ if (request.getAttribute("asyncCycleStarted") == null) {
+ request.setAttribute("asyncCycleStarted", Boolean.TRUE);
+ AsyncContext asyncContext = request.startAsync();
+ asyncContext.start(asyncContext::dispatch);
+ } else {
+ AsyncContext asyncContext = request.startAsync();
+ asyncContext.start(() -> {
+ try {
+ response.getWriter().print(session.getId());
+ } catch (IOException e) {
+ throw new IllegalStateException(e);
+ } finally {
+ asyncContext.complete();
+ }
+ });
+ }
+ }
+ }
+
+
+ private static class BlockingAsyncServlet extends HttpServlet {
+
+ private static final long serialVersionUID = 1L;
+
+ private final CountDownLatch asyncStarted;
+ private final CountDownLatch allowAsyncComplete;
+
+ private BlockingAsyncServlet(CountDownLatch asyncStarted, CountDownLatch allowAsyncComplete) {
+ this.asyncStarted = asyncStarted;
+ this.allowAsyncComplete = allowAsyncComplete;
+ }
+
+ @Override
+ protected void doGet(HttpServletRequest request, HttpServletResponse response) {
+ String sessionId = request.getRequestedSessionId();
+ AsyncContext asyncContext = request.startAsync();
+ asyncContext.start(() -> {
+ asyncStarted.countDown();
+ try {
+ Assert.assertTrue(allowAsyncComplete.await(10, TimeUnit.SECONDS));
+ response.getWriter().print(sessionId);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new IllegalStateException(e);
+ } catch (IOException e) {
+ throw new IllegalStateException(e);
+ } finally {
+ asyncContext.complete();
+ }
+ });
+ }
+ }
+
+
+ private static class TesterStore implements Store {
+
+ private Manager manager;
+ private final Map<String,Session> sessions = new HashMap<>();
+ private final List<String> savedIds = new ArrayList<>();
+
+ private List<String> getSavedIds() {
+ return savedIds;
+ }
+
+ @Override
+ public Manager getManager() {
+ return manager;
+ }
+
+ @Override
+ public void setManager(Manager manager) {
+ this.manager = manager;
+ }
+
+ @Override
+ public int getSize() {
+ return sessions.size();
+ }
+
+ @Override
+ public void addPropertyChangeListener(PropertyChangeListener listener) {
+ // NO-OP
+ }
+
+ @Override
+ public String[] keys() {
+ return sessions.keySet().toArray(new String[0]);
+ }
+
+ @Override
+ public Session load(String id) {
+ return sessions.get(id);
+ }
+
+ @Override
+ public void remove(String id) {
+ sessions.remove(id);
+ }
+
+ @Override
+ public void clear() {
+ sessions.clear();
+ }
+
+ @Override
+ public void removePropertyChangeListener(PropertyChangeListener listener) {
+ // NO-OP
+ }
+
+ @Override
+ public void save(Session session) {
+ sessions.put(session.getId(), session);
+ savedIds.add(session.getId());
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org