This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/10.1.x by this push:
new ae3914158d Minor fixes from code review
ae3914158d is described below
commit ae3914158def54fe02d29e14fd8b88f8ba6166ee
Author: remm <[email protected]>
AuthorDate: Mon Jun 22 15:44:27 2026 +0200
Minor fixes from code review
Most significant is the rework of the wrapper unload.
---
.../catalina/core/ApplicationDispatcher.java | 3 ---
.../catalina/core/ApplicationHttpRequest.java | 5 ++--
.../catalina/core/ApplicationHttpResponse.java | 2 +-
.../apache/catalina/core/ApplicationRequest.java | 2 +-
.../apache/catalina/core/ApplicationResponse.java | 2 +-
.../core/ApplicationServletRegistration.java | 1 +
.../org/apache/catalina/core/AsyncContextImpl.java | 2 +-
.../core/JreMemoryLeakPreventionListener.java | 2 +-
.../catalina/core/NamingContextListener.java | 15 ++++++-----
.../apache/catalina/core/StandardEngineValve.java | 3 +--
java/org/apache/catalina/core/StandardHost.java | 4 +--
.../org/apache/catalina/core/StandardPipeline.java | 1 +
.../core/StandardVirtualThreadExecutor.java | 5 +++-
java/org/apache/catalina/core/StandardWrapper.java | 14 ++++++----
.../catalina/core/StandardWrapperFacade.java | 4 +--
.../core/ThreadLocalLeakPreventionListener.java | 8 +++---
.../catalina/deploy/NamingResourcesImpl.java | 31 +++++++++++-----------
java/org/apache/catalina/filters/CorsFilter.java | 2 +-
18 files changed, 59 insertions(+), 47 deletions(-)
diff --git a/java/org/apache/catalina/core/ApplicationDispatcher.java
b/java/org/apache/catalina/core/ApplicationDispatcher.java
index c419fedd2a..bc7c617a0d 100644
--- a/java/org/apache/catalina/core/ApplicationDispatcher.java
+++ b/java/org/apache/catalina/core/ApplicationDispatcher.java
@@ -547,9 +547,6 @@ final class ApplicationDispatcher implements
AsyncDispatcher, RequestDispatcher
/**
* Ask the resource represented by this RequestDispatcher to process the
associated request, and create (or append
* to) the associated response.
- * <p>
- * <strong>IMPLEMENTATION NOTE</strong>: This implementation assumes that
no filters are applied to a forwarded or
- * included resource, because they were already done for the original
request.
*
* @param request The servlet request we are processing
* @param response The servlet response we are creating
diff --git a/java/org/apache/catalina/core/ApplicationHttpRequest.java
b/java/org/apache/catalina/core/ApplicationHttpRequest.java
index d6916ed288..7ed4542b7f 100644
--- a/java/org/apache/catalina/core/ApplicationHttpRequest.java
+++ b/java/org/apache/catalina/core/ApplicationHttpRequest.java
@@ -58,7 +58,7 @@ import org.apache.tomcat.util.res.StringManager;
* Wrapper around a <code>jakarta.servlet.http.HttpServletRequest</code> that
transforms an application request object
* (which might be the original one passed to a servlet, or might be based on
the 2.3
* <code>jakarta.servlet.http.HttpServletRequestWrapper</code> class) back
into an internal
- * <code>org.apache.catalina.HttpRequest</code>.
+ * <code>org.apache.catalina.connector.Request</code>.
* <p>
* <strong>WARNING</strong>: Due to Java's lack of support for multiple
inheritance, all of the logic in
* <code>ApplicationRequest</code> is duplicated in
<code>ApplicationHttpRequest</code>. Make sure that you keep these
@@ -629,7 +629,8 @@ class ApplicationHttpRequest extends
HttpServletRequestWrapper {
// -------------------------------------------------------- Package Methods
/**
- * Recycle this request
+ * Recycle this request. Since there is no object reuse, this only ends
+ * the session access.
*/
public void recycle() {
if (session != null) {
diff --git a/java/org/apache/catalina/core/ApplicationHttpResponse.java
b/java/org/apache/catalina/core/ApplicationHttpResponse.java
index 7886b707ec..f9e813b519 100644
--- a/java/org/apache/catalina/core/ApplicationHttpResponse.java
+++ b/java/org/apache/catalina/core/ApplicationHttpResponse.java
@@ -28,7 +28,7 @@ import jakarta.servlet.http.HttpServletResponseWrapper;
* Wrapper around a <code>jakarta.servlet.http.HttpServletResponse</code> that
transforms an application response object
* (which might be the original one passed to a servlet, or might be based on
the 2.3
* <code>jakarta.servlet.http.HttpServletResponseWrapper</code> class) back
into an internal
- * <code>org.apache.catalina.HttpResponse</code>.
+ * <code>org.apache.catalina.connector.Response</code>.
* <p>
* <strong>WARNING</strong>: Due to Java's lack of support for multiple
inheritance, all of the logic in
* <code>ApplicationResponse</code> is duplicated in
<code>ApplicationHttpResponse</code>. Make sure that you keep these
diff --git a/java/org/apache/catalina/core/ApplicationRequest.java
b/java/org/apache/catalina/core/ApplicationRequest.java
index 67767172b7..3b5158cf2b 100644
--- a/java/org/apache/catalina/core/ApplicationRequest.java
+++ b/java/org/apache/catalina/core/ApplicationRequest.java
@@ -33,7 +33,7 @@ import org.apache.tomcat.util.res.StringManager;
* Wrapper around a <code>jakarta.servlet.ServletRequest</code> that
transforms an application request object (which
* might be the original one passed to a servlet, or might be based on the 2.3
* <code>jakarta.servlet.ServletRequestWrapper</code> class) back into an
internal
- * <code>org.apache.catalina.Request</code>.
+ * <code>org.apache.catalina.connector.Request</code>.
* <p>
* <strong>WARNING</strong>: Due to Java's lack of support for multiple
inheritance, all of the logic in
* <code>ApplicationRequest</code> is duplicated in
<code>ApplicationHttpRequest</code>. Make sure that you keep these
diff --git a/java/org/apache/catalina/core/ApplicationResponse.java
b/java/org/apache/catalina/core/ApplicationResponse.java
index c2e91370e2..1c2944b477 100644
--- a/java/org/apache/catalina/core/ApplicationResponse.java
+++ b/java/org/apache/catalina/core/ApplicationResponse.java
@@ -26,7 +26,7 @@ import jakarta.servlet.ServletResponseWrapper;
* Wrapper around a <code>jakarta.servlet.ServletResponse</code> that
transforms an application response object (which
* might be the original one passed to a servlet, or might be based on the 2.3
* <code>jakarta.servlet.ServletResponseWrapper</code> class) back into an
internal
- * <code>org.apache.catalina.Response</code>.
+ * <code>org.apache.catalina.connector.Response</code>.
* <p>
* <strong>WARNING</strong>: Due to Java's lack of support for multiple
inheritance, all of the logic in
* <code>ApplicationResponse</code> is duplicated in
<code>ApplicationHttpResponse</code>. Make sure that you keep these
diff --git a/java/org/apache/catalina/core/ApplicationServletRegistration.java
b/java/org/apache/catalina/core/ApplicationServletRegistration.java
index 3433932abb..93a2c47506 100644
--- a/java/org/apache/catalina/core/ApplicationServletRegistration.java
+++ b/java/org/apache/catalina/core/ApplicationServletRegistration.java
@@ -196,6 +196,7 @@ public class ApplicationServletRegistration implements
ServletRegistration.Dynam
*/
overrides.add(decodedUrlPatterns[i]);
} else {
+ // The conflicts list the original URL patterns passed
conflicts.add(urlPatterns[i]);
}
}
diff --git a/java/org/apache/catalina/core/AsyncContextImpl.java
b/java/org/apache/catalina/core/AsyncContextImpl.java
index ee21964b34..d9280ae729 100644
--- a/java/org/apache/catalina/core/AsyncContextImpl.java
+++ b/java/org/apache/catalina/core/AsyncContextImpl.java
@@ -646,7 +646,7 @@ public class AsyncContextImpl implements AsyncContext,
AsyncContextCallback {
private void check() {
Request request = this.request;
- if (hasBeenRecycled.get()) {
+ if (hasBeenRecycled.get() || request == null) {
// AsyncContext has been recycled and should not be being used
throw new
IllegalStateException(sm.getString("asyncContextImpl.requestEnded"));
}
diff --git a/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
b/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
index 554d7e2c8a..de219d11d5 100644
--- a/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
+++ b/java/org/apache/catalina/core/JreMemoryLeakPreventionListener.java
@@ -238,7 +238,7 @@ public class JreMemoryLeakPreventionListener implements
LifecycleListener {
// Set the default URL caching policy to not to cache
if (urlCacheProtection) {
- URLConnection.setDefaultUseCaches("JAR", false);
+ URLConnection.setDefaultUseCaches("jar", false);
}
/*
diff --git a/java/org/apache/catalina/core/NamingContextListener.java
b/java/org/apache/catalina/core/NamingContextListener.java
index 27f9711a84..8ea24c293c 100644
--- a/java/org/apache/catalina/core/NamingContextListener.java
+++ b/java/org/apache/catalina/core/NamingContextListener.java
@@ -37,13 +37,14 @@ import javax.naming.NamingException;
import javax.naming.Reference;
import javax.naming.StringRefAddr;
+import org.apache.catalina.Container;
import org.apache.catalina.Context;
-import org.apache.catalina.Engine;
import org.apache.catalina.Host;
import org.apache.catalina.Lifecycle;
import org.apache.catalina.LifecycleEvent;
import org.apache.catalina.LifecycleListener;
import org.apache.catalina.Server;
+import org.apache.catalina.Service;
import org.apache.catalina.deploy.NamingResourcesImpl;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
@@ -1083,11 +1084,13 @@ public class NamingContextListener implements
LifecycleListener, PropertyChangeL
private javax.naming.Context getGlobalNamingContext() {
if (container instanceof Context) {
- Engine e = (Engine) ((Context) container).getParent().getParent();
- Server s = e.getService().getServer();
- // When the Service is an embedded Service, there is no Server
- if (s != null) {
- return s.getGlobalNamingContext();
+ Service service = Container.getService((Context) container);
+ if (service != null) {
+ Server s = service.getServer();
+ // When the Service is an embedded Service, there is no Server
+ if (s != null) {
+ return s.getGlobalNamingContext();
+ }
}
}
return null;
diff --git a/java/org/apache/catalina/core/StandardEngineValve.java
b/java/org/apache/catalina/core/StandardEngineValve.java
index 546c8d46ac..cf7ea22f82 100644
--- a/java/org/apache/catalina/core/StandardEngineValve.java
+++ b/java/org/apache/catalina/core/StandardEngineValve.java
@@ -56,8 +56,7 @@ final class StandardEngineValve extends ValveBase {
// Select the Host to be used for this Request
Host host = request.getHost();
if (host == null) {
- // HTTP 0.9 or HTTP 1.0 request without a host when no default host
- // is defined.
+ // No Host found for this request (unknown server name or no
default host defined)
// Don't overwrite an existing error
if (!response.isError()) {
response.sendError(404);
diff --git a/java/org/apache/catalina/core/StandardHost.java
b/java/org/apache/catalina/core/StandardHost.java
index 1449b58cb8..868d1bee28 100644
--- a/java/org/apache/catalina/core/StandardHost.java
+++ b/java/org/apache/catalina/core/StandardHost.java
@@ -503,9 +503,9 @@ public class StandardHost extends ContainerBase implements
Host {
*/
public void setErrorReportValveClass(String errorReportValveClass) {
- String oldErrorReportValveClassClass = this.errorReportValveClass;
+ String oldErrorReportValveClass = this.errorReportValveClass;
this.errorReportValveClass = errorReportValveClass;
- support.firePropertyChange("errorReportValveClass",
oldErrorReportValveClassClass, this.errorReportValveClass);
+ support.firePropertyChange("errorReportValveClass",
oldErrorReportValveClass, this.errorReportValveClass);
}
diff --git a/java/org/apache/catalina/core/StandardPipeline.java
b/java/org/apache/catalina/core/StandardPipeline.java
index 29fa0e5197..ca640db4ff 100644
--- a/java/org/apache/catalina/core/StandardPipeline.java
+++ b/java/org/apache/catalina/core/StandardPipeline.java
@@ -365,6 +365,7 @@ public class StandardPipeline extends LifecycleBase
implements Pipeline {
if (first == basic) {
first = null;
}
+ // Note: Removing the basic valve is done by replacing it using
setBasic
cleanupValve(valve);
diff --git a/java/org/apache/catalina/core/StandardVirtualThreadExecutor.java
b/java/org/apache/catalina/core/StandardVirtualThreadExecutor.java
index 6c8a07282a..76fc79404a 100644
--- a/java/org/apache/catalina/core/StandardVirtualThreadExecutor.java
+++ b/java/org/apache/catalina/core/StandardVirtualThreadExecutor.java
@@ -109,8 +109,11 @@ public class StandardVirtualThreadExecutor extends
LifecycleMBeanBase implements
@Override
protected void stopInternal() throws LifecycleException {
- executor = null;
setState(LifecycleState.STOPPING);
+ if (executor != null) {
+ executor.shutdownNow();
+ }
+ executor = null;
}
@Override
diff --git a/java/org/apache/catalina/core/StandardWrapper.java
b/java/org/apache/catalina/core/StandardWrapper.java
index 941a28c38c..fb517bf716 100644
--- a/java/org/apache/catalina/core/StandardWrapper.java
+++ b/java/org/apache/catalina/core/StandardWrapper.java
@@ -959,6 +959,8 @@ public class StandardWrapper extends ContainerBase
implements ServletConfig, Wra
}
}
+ ServletException servletException = null;
+
if (instanceInitialized) {
PrintStream out = System.out;
if (swallowOutput) {
@@ -980,9 +982,7 @@ public class StandardWrapper extends ContainerBase
implements ServletConfig, Wra
} catch (Throwable t) {
Throwable throwable =
ExceptionUtils.unwrapInvocationTargetException(t);
ExceptionUtils.handleThrowable(throwable);
- fireContainerEvent("unload", this);
- unloading = false;
- throw new
ServletException(sm.getString("standardWrapper.destroyException", getName()),
throwable);
+ servletException = new
ServletException(sm.getString("standardWrapper.destroyException", getName()),
throwable);
} finally {
// Annotation processing
if (!((Context) getParent()).getIgnoreAnnotations()) {
@@ -1004,7 +1004,6 @@ public class StandardWrapper extends ContainerBase
implements ServletConfig, Wra
}
}
}
- instance = null;
instanceInitialized = false;
}
}
@@ -1018,6 +1017,11 @@ public class StandardWrapper extends ContainerBase
implements ServletConfig, Wra
unloading = false;
fireContainerEvent("unload", this);
+
+ if (servletException != null) {
+ throw servletException;
+ }
+
}
@@ -1364,7 +1368,7 @@ public class StandardWrapper extends ContainerBase
implements ServletConfig, Wra
new MBeanNotificationInfo(new String[] {
"j2ee.state.running" }, Notification.class.getName(),
"servlet is running"),
new MBeanNotificationInfo(new String[] {
"j2ee.state.stopped" }, Notification.class.getName(),
- "servlet start to stopped"),
+ "servlet is stopping"),
new MBeanNotificationInfo(new String[] {
"j2ee.object.stopped" }, Notification.class.getName(),
"servlet is stopped"),
new MBeanNotificationInfo(new String[] {
"j2ee.object.deleted" }, Notification.class.getName(),
diff --git a/java/org/apache/catalina/core/StandardWrapperFacade.java
b/java/org/apache/catalina/core/StandardWrapperFacade.java
index 1128fea374..4432f99280 100644
--- a/java/org/apache/catalina/core/StandardWrapperFacade.java
+++ b/java/org/apache/catalina/core/StandardWrapperFacade.java
@@ -72,8 +72,8 @@ public final class StandardWrapperFacade implements
ServletConfig {
@Override
public ServletContext getServletContext() {
/*
- * This method may be called concurrently but the same context object
will always be returned. There is no
- * concurrency issue here.
+ * Multiple threads may initialize context concurrently, but the same
context/facade
+ * object is always returned, so this is functionally safe.
*/
if (context == null) {
context = config.getServletContext();
diff --git
a/java/org/apache/catalina/core/ThreadLocalLeakPreventionListener.java
b/java/org/apache/catalina/core/ThreadLocalLeakPreventionListener.java
index 23c68b1085..9d0b21286e 100644
--- a/java/org/apache/catalina/core/ThreadLocalLeakPreventionListener.java
+++ b/java/org/apache/catalina/core/ThreadLocalLeakPreventionListener.java
@@ -18,9 +18,9 @@ package org.apache.catalina.core;
import java.util.concurrent.Executor;
+import org.apache.catalina.Container;
import org.apache.catalina.ContainerEvent;
import org.apache.catalina.Context;
-import org.apache.catalina.Engine;
import org.apache.catalina.Lifecycle;
import org.apache.catalina.LifecycleEvent;
import org.apache.catalina.LifecycleListener;
@@ -111,8 +111,10 @@ public class ThreadLocalLeakPreventionListener extends
FrameworkListener {
return;
}
- Engine engine = (Engine) context.getParent().getParent();
- Service service = engine.getService();
+ Service service = Container.getService(context);
+ if (service == null) {
+ return;
+ }
Connector[] connectors = service.findConnectors();
if (connectors != null) {
for (Connector connector : connectors) {
diff --git a/java/org/apache/catalina/deploy/NamingResourcesImpl.java
b/java/org/apache/catalina/deploy/NamingResourcesImpl.java
index eca608dbb7..28e0aefee3 100644
--- a/java/org/apache/catalina/deploy/NamingResourcesImpl.java
+++ b/java/org/apache/catalina/deploy/NamingResourcesImpl.java
@@ -23,20 +23,20 @@ import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import javax.naming.NamingException;
import org.apache.catalina.Container;
import org.apache.catalina.Context;
-import org.apache.catalina.Engine;
import org.apache.catalina.JmxEnabled;
import org.apache.catalina.LifecycleException;
import org.apache.catalina.LifecycleState;
import org.apache.catalina.Server;
+import org.apache.catalina.Service;
import org.apache.catalina.mbeans.MBeanUtils;
import org.apache.catalina.util.Introspection;
import org.apache.catalina.util.LifecycleMBeanBase;
@@ -98,7 +98,7 @@ public class NamingResourcesImpl extends LifecycleMBeanBase
implements Serializa
/**
* Set of naming entries, keyed by name.
*/
- private final Set<String> entries = new HashSet<>();
+ private final Set<String> entries = ConcurrentHashMap.newKeySet();
/**
@@ -173,7 +173,7 @@ public class NamingResourcesImpl extends LifecycleMBeanBase
implements Serializa
/**
* Set the container with which the naming resources are associated.
*
- * @param container the associated with the resources
+ * @param container the container associated with the resources
*/
public void setContainer(Object container) {
this.container = container;
@@ -301,15 +301,16 @@ public class NamingResourcesImpl extends
LifecycleMBeanBase implements Serializa
}
// Container should be an instance of Server or Context. If it is anything
- // else, return null which will trigger a NPE.
+ // else, return null.
private Server getServer() {
if (container instanceof Server) {
return (Server) container;
}
if (container instanceof Context) {
- // Could do this in one go. Lots of casts so split out for clarity
- Engine engine = (Engine) ((Context)
container).getParent().getParent();
- return engine.getService().getServer();
+ Service service = Container.getService((Context) container);
+ if (service != null) {
+ return service.getServer();
+ }
}
return null;
}
@@ -909,7 +910,7 @@ public class NamingResourcesImpl extends LifecycleMBeanBase
implements Serializa
// timing issues. Duplication registration is not an issue.
resourceRequireExplicitRegistration = true;
- for (ContextResource cr : resources.values()) {
+ for (ContextResource cr : findResources()) {
try {
MBeanUtils.createMBean(cr);
} catch (Exception e) {
@@ -917,7 +918,7 @@ public class NamingResourcesImpl extends LifecycleMBeanBase
implements Serializa
}
}
- for (ContextEnvironment ce : envs.values()) {
+ for (ContextEnvironment ce : findEnvironments()) {
try {
MBeanUtils.createMBean(ce);
} catch (Exception e) {
@@ -925,7 +926,7 @@ public class NamingResourcesImpl extends LifecycleMBeanBase
implements Serializa
}
}
- for (ContextResourceLink crl : resourceLinks.values()) {
+ for (ContextResourceLink crl : findResourceLinks()) {
try {
MBeanUtils.createMBean(crl);
} catch (Exception e) {
@@ -968,7 +969,7 @@ public class NamingResourcesImpl extends LifecycleMBeanBase
implements Serializa
log.warn(sm.getString("namingResources.cleanupNoContext",
container), e);
return;
}
- for (ContextResource cr : resources.values()) {
+ for (ContextResource cr : findResources()) {
if (cr.getSingleton()) {
String closeMethod = cr.getCloseMethod();
if (closeMethod != null && !closeMethod.isEmpty()) {
@@ -1024,7 +1025,7 @@ public class NamingResourcesImpl extends
LifecycleMBeanBase implements Serializa
resourceRequireExplicitRegistration = false;
// Destroy in reverse order to create, although it should not matter
- for (ContextResourceLink crl : resourceLinks.values()) {
+ for (ContextResourceLink crl : findResourceLinks()) {
try {
MBeanUtils.destroyMBean(crl);
} catch (Exception e) {
@@ -1032,7 +1033,7 @@ public class NamingResourcesImpl extends
LifecycleMBeanBase implements Serializa
}
}
- for (ContextEnvironment ce : envs.values()) {
+ for (ContextEnvironment ce : findEnvironments()) {
try {
MBeanUtils.destroyMBean(ce);
} catch (Exception e) {
@@ -1040,7 +1041,7 @@ public class NamingResourcesImpl extends
LifecycleMBeanBase implements Serializa
}
}
- for (ContextResource cr : resources.values()) {
+ for (ContextResource cr : findResources()) {
try {
MBeanUtils.destroyMBean(cr);
} catch (Exception e) {
diff --git a/java/org/apache/catalina/filters/CorsFilter.java
b/java/org/apache/catalina/filters/CorsFilter.java
index a7ae8fb5da..2915d38244 100644
--- a/java/org/apache/catalina/filters/CorsFilter.java
+++ b/java/org/apache/catalina/filters/CorsFilter.java
@@ -72,7 +72,7 @@ import org.apache.tomcat.util.res.StringManager;
* {@link CorsFilter#doFilter(ServletRequest, ServletResponse, FilterChain)}
and add appropriate locking so that the
* {@code doFilter()} method executes with a consistent configuration.
*
- * @see <a href="http://www.w3.org/TR/cors/";>CORS specification</a>
+ * @see <a href="https://fetch.spec.whatwg.org/#http-cors-protocol";>CORS
specification</a>
*/
public class CorsFilter extends GenericFilter {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
