Author: markt Date: Tue Aug 11 20:34:00 2015 New Revision: 1695371 URL: http://svn.apache.org/r1695371 Log: Fix possible cause of intermittent CI failures and bug 58157
Modified: tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java Modified: tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java?rev=1695371&r1=1695370&r2=1695371&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java (original) +++ tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java Tue Aug 11 20:34:00 2015 @@ -58,6 +58,15 @@ public class AsyncContextImpl implements protected static final StringManager sm = StringManager.getManager(Constants.Package); + /* When a request uses a sequence of multiple start(); dispatch() with + * non-container threads it is possible for a previous dispatch() to + * interfere with a following start(). This lock prevents that from + * happening. It is a dedicated object as user code may lock on the + * AsyncContext so if container code also locks on that object deadlocks may + * occur. + */ + private final Object asyncContextLock = new Object(); + private volatile ServletRequest servletRequest = null; private volatile ServletResponse servletResponse = null; private final List<AsyncListenerWrapper> listeners = new ArrayList<>(); @@ -180,46 +189,48 @@ public class AsyncContextImpl implements @Override public void dispatch(ServletContext context, String path) { - if (log.isDebugEnabled()) { - logDebug("dispatch "); - } - check(); - if (dispatch != null) { - throw new IllegalStateException( - sm.getString("asyncContextImpl.dispatchingStarted")); - } - if (request.getAttribute(ASYNC_REQUEST_URI)==null) { - request.setAttribute(ASYNC_REQUEST_URI, request.getRequestURI()); - request.setAttribute(ASYNC_CONTEXT_PATH, request.getContextPath()); - request.setAttribute(ASYNC_SERVLET_PATH, request.getServletPath()); - request.setAttribute(ASYNC_PATH_INFO, request.getPathInfo()); - request.setAttribute(ASYNC_QUERY_STRING, request.getQueryString()); - } - final RequestDispatcher requestDispatcher = context.getRequestDispatcher(path); - if (!(requestDispatcher instanceof AsyncDispatcher)) { - throw new UnsupportedOperationException( - sm.getString("asyncContextImpl.noAsyncDispatcher")); - } - final AsyncDispatcher applicationDispatcher = - (AsyncDispatcher) requestDispatcher; - final ServletRequest servletRequest = getRequest(); - final ServletResponse servletResponse = getResponse(); - Runnable run = new Runnable() { - @Override - public void run() { - request.getCoyoteRequest().action(ActionCode.ASYNC_DISPATCHED, null); - try { - applicationDispatcher.dispatch(servletRequest, servletResponse); - }catch (Exception x) { - //log.error("Async.dispatch",x); - throw new RuntimeException(x); - } + synchronized (asyncContextLock) { + if (log.isDebugEnabled()) { + logDebug("dispatch "); } - }; + check(); + if (dispatch != null) { + throw new IllegalStateException( + sm.getString("asyncContextImpl.dispatchingStarted")); + } + if (request.getAttribute(ASYNC_REQUEST_URI)==null) { + request.setAttribute(ASYNC_REQUEST_URI, request.getRequestURI()); + request.setAttribute(ASYNC_CONTEXT_PATH, request.getContextPath()); + request.setAttribute(ASYNC_SERVLET_PATH, request.getServletPath()); + request.setAttribute(ASYNC_PATH_INFO, request.getPathInfo()); + request.setAttribute(ASYNC_QUERY_STRING, request.getQueryString()); + } + final RequestDispatcher requestDispatcher = context.getRequestDispatcher(path); + if (!(requestDispatcher instanceof AsyncDispatcher)) { + throw new UnsupportedOperationException( + sm.getString("asyncContextImpl.noAsyncDispatcher")); + } + final AsyncDispatcher applicationDispatcher = + (AsyncDispatcher) requestDispatcher; + final ServletRequest servletRequest = getRequest(); + final ServletResponse servletResponse = getResponse(); + Runnable run = new Runnable() { + @Override + public void run() { + request.getCoyoteRequest().action(ActionCode.ASYNC_DISPATCHED, null); + try { + applicationDispatcher.dispatch(servletRequest, servletResponse); + }catch (Exception x) { + //log.error("Async.dispatch",x); + throw new RuntimeException(x); + } + } + }; - this.dispatch = run; - this.request.getCoyoteRequest().action(ActionCode.ASYNC_DISPATCH, null); - clearServletRequestResponse(); + this.dispatch = run; + this.request.getCoyoteRequest().action(ActionCode.ASYNC_DISPATCH, null); + clearServletRequestResponse(); + } } @Override @@ -332,25 +343,27 @@ public class AsyncContextImpl implements public void setStarted(Context context, ServletRequest request, ServletResponse response, boolean originalRequestResponse) { - this.request.getCoyoteRequest().action( - ActionCode.ASYNC_START, this); - - this.context = context; - this.servletRequest = request; - this.servletResponse = response; - this.hasOriginalRequestAndResponse = originalRequestResponse; - this.event = new AsyncEvent(this, request, response); - - List<AsyncListenerWrapper> listenersCopy = new ArrayList<>(); - listenersCopy.addAll(listeners); - listeners.clear(); - for (AsyncListenerWrapper listener : listenersCopy) { - try { - listener.fireOnStartAsync(event); - } catch (Throwable t) { - ExceptionUtils.handleThrowable(t); - log.warn("onStartAsync() failed for listener of type [" + - listener.getClass().getName() + "]", t); + synchronized (asyncContextLock) { + this.request.getCoyoteRequest().action( + ActionCode.ASYNC_START, this); + + this.context = context; + this.servletRequest = request; + this.servletResponse = response; + this.hasOriginalRequestAndResponse = originalRequestResponse; + this.event = new AsyncEvent(this, request, response); + + List<AsyncListenerWrapper> listenersCopy = new ArrayList<>(); + listenersCopy.addAll(listeners); + listeners.clear(); + for (AsyncListenerWrapper listener : listenersCopy) { + try { + listener.fireOnStartAsync(event); + } catch (Throwable t) { + ExceptionUtils.handleThrowable(t); + log.warn("onStartAsync() failed for listener of type [" + + listener.getClass().getName() + "]", t); + } } } } --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org