On Thu, May 21, 2026 at 12:50 PM Mark Thomas <[email protected]> wrote:
>
> On 20/05/2026 14:44, [email protected] wrote:
> > This is an automated email from the ASF dual-hosted git repository.
> >
> > rmaucher 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 f0a584792e Generalize null checks in AsyncContext
> > f0a584792e is described below
> >
> > commit f0a584792e32da7d18708de13f0652cf84dded65
> > Author: remm <[email protected]>
> > AuthorDate: Wed May 20 15:44:17 2026 +0200
> >
> > Generalize null checks in AsyncContext
>
> Is this the right approach?
>
> In most (all?) of these cases if context is null, the method should not
> have been called in the first place. Wouldn't throwing
> IllegalStateException be better here. That would be consistent with
> getRequest() and getResponse().
Unsure, we started adding some in the paths that were commonly
reported by users, so it simply generalizes this. If we add ISE, then
it means lots of logging and people may complain.
Rémy
> Mark
>
>
> > ---
> > .../org/apache/catalina/core/AsyncContextImpl.java | 54
> > ++++++++++++++++------
> > 1 file changed, 41 insertions(+), 13 deletions(-)
> >
> > diff --git a/java/org/apache/catalina/core/AsyncContextImpl.java
> > b/java/org/apache/catalina/core/AsyncContextImpl.java
> > index 96a333be78..74307785e1 100644
> > --- a/java/org/apache/catalina/core/AsyncContextImpl.java
> > +++ b/java/org/apache/catalina/core/AsyncContextImpl.java
> > @@ -111,8 +111,12 @@ public class AsyncContextImpl implements AsyncContext,
> > AsyncContextCallback {
> > log.trace(sm.getString("asyncContextImpl.fireOnComplete"));
> > }
> > List<AsyncListenerWrapper> listenersCopy = new
> > ArrayList<>(listeners);
> > + Context context = this.context;
> >
> > - ClassLoader oldCL = context.bind(null);
> > + ClassLoader oldCL = null;
> > + if (context != null) {
> > + oldCL = context.bind(null);
> > + }
> > try {
> > for (AsyncListenerWrapper listener : listenersCopy) {
> > try {
> > @@ -123,9 +127,13 @@ public class AsyncContextImpl implements AsyncContext,
> > AsyncContextCallback {
> > }
> > }
> > } finally {
> > - context.fireRequestDestroyEvent(request.getRequest());
> > + if (context != null) {
> > + context.fireRequestDestroyEvent(request.getRequest());
> > + }
> > clearServletRequestResponse();
> > - context.unbind(oldCL);
> > + if (context != null) {
> > + context.unbind(oldCL);
> > + }
> > }
> > }
> >
> > @@ -145,7 +153,10 @@ public class AsyncContextImpl implements AsyncContext,
> > AsyncContextCallback {
> > if (log.isTraceEnabled()) {
> > log.trace(sm.getString("asyncContextImpl.fireOnTimeout"));
> > }
> > - ClassLoader oldCL = context.bind(null);
> > + ClassLoader oldCL = null;
> > + if (context != null) {
> > + oldCL = context.bind(null);
> > + }
> > try {
> > List<AsyncListenerWrapper> listenersCopy = new
> > ArrayList<>(listeners);
> > for (AsyncListenerWrapper listener : listenersCopy) {
> > @@ -158,7 +169,9 @@ public class AsyncContextImpl implements AsyncContext,
> > AsyncContextCallback {
> > }
> >
> > request.getCoyoteRequest().action(ActionCode.ASYNC_IS_TIMINGOUT, result);
> > } finally {
> > - context.unbind(oldCL);
> > + if (context != null) {
> > + context.unbind(oldCL);
> > + }
> > }
> > }
> > return !result.get();
> > @@ -180,7 +193,8 @@ public class AsyncContextImpl implements AsyncContext,
> > AsyncContextCallback {
> > if (pathInfo != null && !pathInfo.isEmpty()) {
> > path = path + pathInfo;
> > }
> > - if (context.getDispatchersUseEncodedPaths()) {
> > + Context context = this.context;
> > + if (context == null || context.getDispatchersUseEncodedPaths()) {
> > path = URLEncoder.DEFAULT.encode(path,
> > StandardCharsets.UTF_8);
> > }
> > dispatch(path);
> > @@ -273,7 +287,12 @@ public class AsyncContextImpl implements AsyncContext,
> > AsyncContextCallback {
> > check();
> > T listener;
> > try {
> > - listener = (T)
> > context.getInstanceManager().newInstance(clazz.getName(),
> > clazz.getClassLoader());
> > + Context context = this.context;
> > + if (context != null) {
> > + listener = (T)
> > context.getInstanceManager().newInstance(clazz.getName(),
> > clazz.getClassLoader());
> > + } else {
> > + listener = (T) Class.forName(clazz.getName(), true,
> > clazz.getClassLoader()).getConstructor().newInstance();
> > + }
> > } catch (ReflectiveOperationException | NamingException e) {
> > throw new ServletException(e);
> > } catch (Exception e) {
> > @@ -477,10 +496,13 @@ public class AsyncContextImpl implements
> > AsyncContext, AsyncContextCallback {
> > ((HttpServletResponse)
> > servletResponse).setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
> > }
> >
> > - Host host = (Host) context.getParent();
> > - Valve stdHostValve = host.getPipeline().getBasic();
> > - if (stdHostValve instanceof StandardHostValve) {
> > - ((StandardHostValve) stdHostValve).throwable(request,
> > request.getResponse(), t);
> > + Context context = this.context;
> > + if (context != null) {
> > + Host host = (Host) context.getParent();
> > + Valve stdHostValve = host.getPipeline().getBasic();
> > + if (stdHostValve instanceof StandardHostValve) {
> > + ((StandardHostValve) stdHostValve).throwable(request,
> > request.getResponse(), t);
> > + }
> > }
> >
> > request.getCoyoteRequest().action(ActionCode.ASYNC_IS_ERROR,
> > result);
> > @@ -501,13 +523,19 @@ public class AsyncContextImpl implements
> > AsyncContext, AsyncContextCallback {
> >
> > @Override
> > public void incrementInProgressAsyncCount() {
> > - context.incrementInProgressAsyncCount();
> > + Context context = this.context;
> > + if (context != null) {
> > + context.incrementInProgressAsyncCount();
> > + }
> > }
> >
> >
> > @Override
> > public void decrementInProgressAsyncCount() {
> > - context.decrementInProgressAsyncCount();
> > + Context context = this.context;
> > + if (context != null) {
> > + context.decrementInProgressAsyncCount();
> > + }
> > }
> >
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [email protected]
> > For additional commands, e-mail: [email protected]
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
