From c09c1414d1ab132ef5cbc9d3b160997684ed6c4b Mon Sep 17 00:00:00 2001 From: markt Date: Sun, 4 Jul 2010 15:38:40 +0000 Subject: [PATCH] Revert r960283, r960316, r960318. r960283 broke the spec and the other commits were sufficiently tightly coupled to it that it was easier to revert all of them than try to unpick them. git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@960347 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/catalina/core/AsyncContextImpl.java | 239 ++++++++------------- .../apache/catalina/core/TestAsyncContextImpl.java | 120 ----------- webapps/docs/changelog.xml | 4 - 3 files changed, 92 insertions(+), 271 deletions(-) delete mode 100644 test/org/apache/catalina/core/TestAsyncContextImpl.java diff --git a/java/org/apache/catalina/core/AsyncContextImpl.java b/java/org/apache/catalina/core/AsyncContextImpl.java index ca531dbc4..b3c800f8a 100644 --- a/java/org/apache/catalina/core/AsyncContextImpl.java +++ b/java/org/apache/catalina/core/AsyncContextImpl.java @@ -1,13 +1,13 @@ -/** +/* * 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. @@ -40,21 +40,19 @@ import org.apache.catalina.connector.Request; import org.apache.coyote.ActionCode; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; - /** - * + * * @author fhanik * */ public class AsyncContextImpl implements AsyncContext { - + public static enum AsyncState { - NOT_STARTED, STARTED, DISPATCHING, - DISPATCHED, COMPLETING, TIMING_OUT, ERROR_DISPATCHING + NOT_STARTED, STARTED, DISPATCHING, DISPATCHED, COMPLETING, TIMING_OUT, ERROR_DISPATCHING } - + private static final Log log = LogFactory.getLog(AsyncContextImpl.class); - + private ServletRequest servletRequest = null; private ServletResponse servletResponse = null; private List listeners = new ArrayList(); @@ -64,15 +62,12 @@ public class AsyncContextImpl implements AsyncContext { private AtomicReference state = new AtomicReference(AsyncState.NOT_STARTED); private long timeout = -1; private AsyncEvent event = null; - + private Request request; - + public AsyncContextImpl(Request request) { if (log.isDebugEnabled()) { - log.debug("AsyncContext created[" - + request.getRequestURI() - + "?" + request.getQueryString() + "]", - new DebugException()); + log.debug("AsyncContext created["+request.getRequestURI()+"?"+request.getQueryString()+"]", new DebugException()); } //TODO SERVLET3 - async this.request = request; @@ -81,102 +76,81 @@ public class AsyncContextImpl implements AsyncContext { @Override public void complete() { if (log.isDebugEnabled()) { - log.debug("AsyncContext Complete Called[" - + state.get() + "; " - + request.getRequestURI() - + "?" + request.getQueryString() + "]", - new DebugException()); + log.debug("AsyncContext Complete Called["+state.get()+"; "+request.getRequestURI()+"?"+request.getQueryString()+"]", new DebugException()); } - if (state.get() == AsyncState.COMPLETING) { + if (state.get()==AsyncState.COMPLETING) { //do nothing - } else if (state.compareAndSet(AsyncState.DISPATCHED, AsyncState.COMPLETING) - || state.compareAndSet(AsyncState.STARTED, AsyncState.COMPLETING)) { + } else if (state.compareAndSet(AsyncState.DISPATCHED, AsyncState.COMPLETING) || + state.compareAndSet(AsyncState.STARTED, AsyncState.COMPLETING)) { // TODO SERVLET3 - async AtomicBoolean dispatched = new AtomicBoolean(false); - request.getCoyoteRequest().action( - ActionCode.ACTION_ASYNC_COMPLETE, dispatched); - if (!dispatched.get()) { - doInternalComplete(false); - } + request.getCoyoteRequest().action(ActionCode.ACTION_ASYNC_COMPLETE,dispatched); + if (!dispatched.get()) doInternalComplete(false); } else { - throw new IllegalStateException( - "Complete not allowed. Invalid state:" + state.get()); + throw new IllegalStateException("Complete not allowed. Invalid state:"+state.get()); } - + } @Override public void dispatch() { - HttpServletRequest sr = (HttpServletRequest) getServletRequest(); + HttpServletRequest sr = (HttpServletRequest)getServletRequest(); String path = sr.getRequestURI(); - final String cpath = sr.getContextPath(); - if (cpath.length() > 1) { - path = path.substring(cpath.length()); - } + String cpath = sr.getContextPath(); + if (cpath.length()>1) path = path.substring(cpath.length()); dispatch(path); } @Override - public void dispatch(final String path) { - dispatch(request.getServletContext(), path); + public void dispatch(String path) { + dispatch(request.getServletContext(),path); } @Override - public void dispatch(final ServletContext dispachContext, - final String path) { + public void dispatch(ServletContext context, String path) { if (log.isDebugEnabled()) { - log.debug("AsyncContext Dispatch Called[" - + state.get() + "; " - + path + "; " - + request.getRequestURI() - + "?" + request.getQueryString() + "]", - new DebugException()); + log.debug("AsyncContext Dispatch Called["+state.get()+"; "+path+"; "+request.getRequestURI()+"?"+request.getQueryString()+"]", new DebugException()); } // TODO SERVLET3 - async - if (state.compareAndSet(AsyncState.STARTED, AsyncState.DISPATCHING) - || state.compareAndSet(AsyncState.DISPATCHED, AsyncState.DISPATCHING)) { - - if (request.getAttribute(ASYNC_REQUEST_URI) == null) { - request.setAttribute(ASYNC_REQUEST_URI, - request.getRequestURI() + "?" + request.getQueryString()); - request.setAttribute(ASYNC_CONTEXT_PATH, - request.getContextPath()); - request.setAttribute(ASYNC_SERVLET_PATH, - request.getServletPath()); - request.setAttribute(ASYNC_QUERY_STRING, - request.getQueryString()); + if (state.compareAndSet(AsyncState.STARTED, AsyncState.DISPATCHING) || + state.compareAndSet(AsyncState.DISPATCHED, AsyncState.DISPATCHING)) { + + if (request.getAttribute(ASYNC_REQUEST_URI)==null) { + request.setAttribute(ASYNC_REQUEST_URI, request.getRequestURI()+"?"+request.getQueryString()); + request.setAttribute(ASYNC_CONTEXT_PATH, request.getContextPath()); + request.setAttribute(ASYNC_SERVLET_PATH, request.getServletPath()); + request.setAttribute(ASYNC_QUERY_STRING, request.getQueryString()); } - final RequestDispatcher requestDispatcher = dispachContext.getRequestDispatcher(path); - final HttpServletRequest sRequest = (HttpServletRequest) getRequest(); - final HttpServletResponse sResponse = (HttpServletResponse) getResponse(); + final RequestDispatcher requestDispatcher = context.getRequestDispatcher(path); + final HttpServletRequest servletRequest = (HttpServletRequest)getRequest(); + final HttpServletResponse servletResponse = (HttpServletResponse)getResponse(); Runnable run = new Runnable() { public void run() { - DispatcherType type = (DispatcherType) request.getAttribute(Globals.DISPATCHER_TYPE_ATTR); + DispatcherType type = (DispatcherType)request.getAttribute(Globals.DISPATCHER_TYPE_ATTR); try { //piggy back on the request dispatcher to ensure that filters etc get called. //TODO SERVLET3 - async should this be include/forward or a new dispatch type //javadoc suggests include with the type of DispatcherType.ASYNC request.setAttribute(Globals.DISPATCHER_TYPE_ATTR, DispatcherType.ASYNC); - requestDispatcher.include(sRequest, sResponse); - } catch (Exception x) { + requestDispatcher.include(servletRequest, servletResponse); + }catch (Exception x) { //log.error("Async.dispatch",x); throw new RuntimeException(x); - } finally { + }finally { request.setAttribute(Globals.DISPATCHER_TYPE_ATTR, type); } } }; this.dispatch = run; AtomicBoolean dispatched = new AtomicBoolean(false); - request.getCoyoteRequest().action( - ActionCode.ACTION_ASYNC_DISPATCH, dispatched); + request.getCoyoteRequest().action(ActionCode.ACTION_ASYNC_DISPATCH, dispatched ); if (!dispatched.get()) { try { doInternalDispatch(); - } catch (ServletException sx) { + }catch (ServletException sx) { throw new RuntimeException(sx); - } catch (IOException ix) { + }catch (IOException ix) { throw new RuntimeException(ix); } } @@ -184,8 +158,7 @@ public class AsyncContextImpl implements AsyncContext { complete(); } } else { - throw new IllegalStateException( - "Dispatch not allowed. Invalid state:" + state.get()); + throw new IllegalStateException("Dispatch not allowed. Invalid state:"+state.get()); } } @@ -202,59 +175,51 @@ public class AsyncContextImpl implements AsyncContext { @Override public void start(final Runnable run) { if (log.isDebugEnabled()) { - log.debug("AsyncContext Start Called[" - + state.get() + "; " - + request.getRequestURI() - + "?" + request.getQueryString() + "]", - new DebugException()); + log.debug("AsyncContext Start Called["+state.get()+"; "+request.getRequestURI()+"?"+request.getQueryString()+"]", new DebugException()); } - if (state.compareAndSet(AsyncState.STARTED, AsyncState.DISPATCHING) - || state.compareAndSet(AsyncState.DISPATCHED, AsyncState.DISPATCHING)) { + if (state.compareAndSet(AsyncState.STARTED, AsyncState.DISPATCHING) || + state.compareAndSet(AsyncState.DISPATCHED, AsyncState.DISPATCHING)) { // TODO SERVLET3 - async final ServletContext sctx = getServletRequest().getServletContext(); Runnable r = new Runnable() { public void run() { - //TODO SERVLET3 - - //async - set context class loader when running the task. + //TODO SERVLET3 - async - set context class loader when running the task. try { - + run.run(); - } catch (Exception x) { - log.error("Unable to run async task.", x); + }catch (Exception x) { + log.error("Unable to run async task.",x); } } }; this.dispatch = r; AtomicBoolean dispatched = new AtomicBoolean(false); - request.getCoyoteRequest().action( - ActionCode.ACTION_ASYNC_DISPATCH, dispatched); + request.getCoyoteRequest().action(ActionCode.ACTION_ASYNC_DISPATCH, dispatched ); if (!dispatched.get()) { try { doInternalDispatch(); - } catch (ServletException sx) { + }catch (ServletException sx) { throw new RuntimeException(sx); - } catch (IOException ix) { + }catch (IOException ix) { throw new RuntimeException(ix); } } } else { - throw new IllegalStateException( - "Dispatch not allowed. Invalid state:" + state.get()); + throw new IllegalStateException("Dispatch not allowed. Invalid state:"+state.get()); } } - + @Override - public void addListener(final AsyncListener listener) { + public void addListener(AsyncListener listener) { AsyncListenerWrapper wrapper = new AsyncListenerWrapper(); wrapper.setListener(listener); listeners.add(wrapper); } @Override - public void addListener(final AsyncListener listener, - final ServletRequest sRequest, - final ServletResponse sResponse) { + public void addListener(AsyncListener listener, ServletRequest servletRequest, + ServletResponse servletResponse) { AsyncListenerWrapper wrapper = new AsyncListenerWrapper(); wrapper.setListener(listener); listeners.add(wrapper); @@ -275,7 +240,7 @@ public class AsyncContextImpl implements AsyncContext { } return listener; } - + public void recycle() { servletRequest = null; servletResponse = null; @@ -288,18 +253,15 @@ public class AsyncContextImpl implements AsyncContext { } public boolean isStarted() { - return (state.get() == AsyncState.STARTED || - state.get() == AsyncState.DISPATCHING || - state.get() == AsyncState.DISPATCHED); + return (state.get() == AsyncState.STARTED || state.get() == AsyncState.DISPATCHING); } public void setStarted(Context context) { if (state.compareAndSet(AsyncState.NOT_STARTED, AsyncState.STARTED) || - state.compareAndSet(AsyncState.DISPATCHED, AsyncState.STARTED)) { + state.compareAndSet(AsyncState.DISPATCHED, AsyncState.STARTED)) { this.context = context; } else { - throw new IllegalStateException("Start illegal. Invalid state: " + - state.get()); + throw new IllegalStateException("Start illegal. Invalid state: "+state.get()); } } @@ -316,8 +278,7 @@ public class AsyncContextImpl implements AsyncContext { return hasOriginalRequestAndResponse; } - public void setHasOriginalRequestAndResponse( - boolean hasOriginalRequestAndResponse) { + public void setHasOriginalRequestAndResponse(boolean hasOriginalRequestAndResponse) { this.hasOriginalRequestAndResponse = hasOriginalRequestAndResponse; } @@ -328,12 +289,10 @@ public class AsyncContextImpl implements AsyncContext { public void setCompleted() { this.state.set(AsyncState.NOT_STARTED); } - + public void doInternalDispatch() throws ServletException, IOException { - if (this.state.compareAndSet( - AsyncState.TIMING_OUT, AsyncState.COMPLETING)) { - if (log.isDebugEnabled()) - log.debug("TIMING OUT!"); + if (this.state.compareAndSet(AsyncState.TIMING_OUT, AsyncState.COMPLETING)) { + log.debug("TIMING OUT!"); boolean listenerInvoked = false; for (AsyncListenerWrapper listener : listeners) { listener.fireOnTimeout(event); @@ -343,20 +302,16 @@ public class AsyncContextImpl implements AsyncContext { ((HttpServletResponse)servletResponse).setStatus(500); } doInternalComplete(true); - } else if (this.state.compareAndSet( - AsyncState.ERROR_DISPATCHING, AsyncState.COMPLETING)) { - if (log.isDebugEnabled()) - log.debug("ON ERROR!"); + } else if (this.state.compareAndSet(AsyncState.ERROR_DISPATCHING, AsyncState.COMPLETING)) { + log.debug("ON ERROR!"); boolean listenerInvoked = false; for (AsyncListenerWrapper listener : listeners) { try { listener.fireOnError(event); }catch (IllegalStateException x) { - if (log.isDebugEnabled()) - log.debug("Listener invoked invalid state.",x); + log.debug("Listener invoked invalid state.",x); }catch (Exception x) { - if (log.isDebugEnabled()) - log.debug("Exception during onError.",x); + log.debug("Exception during onError.",x); } listenerInvoked = true; } @@ -364,18 +319,15 @@ public class AsyncContextImpl implements AsyncContext { ((HttpServletResponse)servletResponse).setStatus(500); } doInternalComplete(true); - - } else if (this.state.compareAndSet( - AsyncState.DISPATCHING, AsyncState.DISPATCHED)) { + + } else if (this.state.compareAndSet(AsyncState.DISPATCHING, AsyncState.DISPATCHED)) { if (this.dispatch!=null) { try { dispatch.run(); } catch (RuntimeException x) { doInternalComplete(true); - if (x.getCause() instanceof ServletException) - throw (ServletException)x.getCause(); - if (x.getCause() instanceof IOException) - throw (IOException)x.getCause(); + if (x.getCause() instanceof ServletException) throw (ServletException)x.getCause(); + if (x.getCause() instanceof IOException) throw (IOException)x.getCause(); else throw new ServletException(x); } finally { dispatch = null; @@ -384,26 +336,23 @@ public class AsyncContextImpl implements AsyncContext { } else if (this.state.get()==AsyncState.COMPLETING) { doInternalComplete(false); } else { - throw new IllegalStateException( - "Dispatch illegal. Invalid state: " + - state.get()); + throw new IllegalStateException("Dispatch illegal. Invalid state: "+state.get()); } } - + public void doInternalComplete(boolean error) { if (isCompleted()) return; if (state.compareAndSet(AsyncState.STARTED, AsyncState.NOT_STARTED)) { //this is the same as //request.startAsync().complete(); recycle(); - } else if (state.compareAndSet( - AsyncState.COMPLETING, AsyncState.NOT_STARTED)) { + } else if (state.compareAndSet(AsyncState.COMPLETING, AsyncState.NOT_STARTED)) { for (AsyncListenerWrapper wrapper : listeners) { try { wrapper.fireOnComplete(event); }catch (IOException x) { //how does this propagate, or should it? - //TODO SERVLET3 - async + //TODO SERVLET3 - async log.error("",x); } } @@ -413,46 +362,42 @@ public class AsyncContextImpl implements AsyncContext { log.error("",x); } recycle(); - - } else { - throw new IllegalStateException("Complete illegal. Invalid state:" + - state.get()); + + } else { + throw new IllegalStateException("Complete illegal. Invalid state:"+state.get()); } } - + public AsyncState getState() { return state.get(); } - + protected void setState(AsyncState st) { state.set(st); } - + public long getTimeout() { return timeout; } - + public void setTimeout(long timeout) { this.timeout = timeout; - request.getCoyoteRequest().action(ActionCode.ACTION_ASYNC_SETTIMEOUT, - new Long(timeout)); + request.getCoyoteRequest().action(ActionCode.ACTION_ASYNC_SETTIMEOUT,new Long(timeout)); } - + public void setTimeoutState() { state.set(AsyncState.TIMING_OUT); } - + public void setErrorState(Throwable t) { if (t!=null) request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, t); state.set(AsyncState.ERROR_DISPATCHING); } - + public void init(ServletRequest request, ServletResponse response) { this.servletRequest = request; this.servletResponse = response; - event = new AsyncEvent(this, request, response); + event = new AsyncEvent(this, request, response); } - - @SuppressWarnings("serial") public static class DebugException extends Exception {} } diff --git a/test/org/apache/catalina/core/TestAsyncContextImpl.java b/test/org/apache/catalina/core/TestAsyncContextImpl.java deleted file mode 100644 index 8e4f642c7..000000000 --- a/test/org/apache/catalina/core/TestAsyncContextImpl.java +++ /dev/null @@ -1,120 +0,0 @@ -/* - * 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.core; - -import java.io.File; -import java.io.IOException; - -import javax.servlet.ServletException; - -import javax.servlet.http.HttpServlet; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -import org.apache.catalina.Context; -import org.apache.catalina.Wrapper; -import org.apache.catalina.startup.Tomcat; -import org.apache.catalina.startup.TomcatBaseTest; - -/** - * Simulate Bug 49528. - * - * @author Peter Rossbach - * @version $Revision$ - */ -public class TestAsyncContextImpl extends TomcatBaseTest { - - public void testIsAsyncStarted() throws Exception { - // Setup Tomcat instance - Tomcat tomcat = getTomcatInstance(); - - BUG49528Servlet servlet = createTestApp(tomcat); - tomcat.start(); - getUrl("http://localhost:" + getPort() + "/async"); - // currently the bug show that we don't start a thread. All message log the same thread id! - assertTrue(servlet.asyncStartBeforeDispatched); - assertTrue(servlet.asyncStartRun); - assertFalse(servlet.asyncStartAfterComplete); - } - - private BUG49528Servlet createTestApp(Tomcat tomcat) { - // Must have a real docBase - just use temp - File docBase = new File(System.getProperty("java.io.tmpdir")); - - // Create the folder that will trigger the redirect - File foo = new File(docBase, "async"); - if (!foo.exists() && !foo.mkdirs()) { - fail("Unable to create async directory in docBase"); - } - - Context ctx = tomcat.addContext("/", docBase.getAbsolutePath()); - - BUG49528Servlet servlet = new BUG49528Servlet(); - Wrapper wrapper = Tomcat.addServlet(ctx, "test", servlet); - wrapper.setAsyncSupported(true); - ctx.addServletMapping("/async", "test"); - return servlet; - } - - // see BUG 49528 report TestCase servlet - private static class BUG49528Servlet extends HttpServlet { - - private static final long serialVersionUID = 1L; - - protected volatile boolean asyncStartBeforeDispatched = false; - - protected volatile boolean asyncStartRun = false; - - protected volatile boolean asyncStartAfterComplete = true; - - @Override - protected void doGet(final HttpServletRequest request, HttpServletResponse response) - throws ServletException, IOException { - - long rtid = Thread.currentThread().getId(); - log(rtid + " Start async()"); - request.startAsync(); - log(rtid + " Dispatching start()"); - log(rtid + " request.isAsyncStarted()1" + request.isAsyncStarted()); - asyncStartBeforeDispatched = request.isAsyncStarted(); - request.getAsyncContext().start(new Runnable() { - @Override - public void run() { - try { - long tid = Thread.currentThread().getId(); - asyncStartRun = request.isAsyncStarted(); - log(tid + " request.isAsyncStarted()2" + request.isAsyncStarted()); - log(tid + " Before sleep()"); - Thread.sleep(500); - log(tid + " After sleep()"); - log(tid + " request.isAsyncStarted()3" + request.isAsyncStarted()); - request.getAsyncContext().complete(); - asyncStartAfterComplete = request.isAsyncStarted(); - log(tid + " Returning from run()"); - log(tid + " request.isAsyncStarted()4" + request.isAsyncStarted()); - } catch (InterruptedException e) { - // TODO Auto-generated catch block - e.printStackTrace(); - } - } - }); - log(rtid + " Returning from doGet()"); - } - - } - -} diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 886ce4666..9671a4dde 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -38,10 +38,6 @@ - 49528: HttpServletRequest.isAsyncStarted() now returns true when a Runnable is started. - Bug reported by Pieter Libin. (pero) - - GSOC 2010. Continue work to align MBean descriptors with reality. Patch provided by Chamith Buddhika. (markt) -- 2.11.0