On 14/05/2014 01:44, kkoli...@apache.org wrote: > Author: kkolinko > Date: Wed May 14 00:44:33 2014 > New Revision: 1594436 > > URL: http://svn.apache.org/r1594436 > Log: > Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56399 > When recycling a Coyote request, ensure that Catalina request have been > recycled. > > Modified: > tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java > tomcat/trunk/java/org/apache/catalina/connector/LocalStrings.properties > tomcat/trunk/java/org/apache/coyote/Adapter.java > tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java > tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java > tomcat/trunk/webapps/docs/changelog.xml > > Modified: tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1594436&r1=1594435&r2=1594436&view=diff > ============================================================================== > --- tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java > (original) > +++ tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Wed > May 14 00:44:33 2014 > @@ -655,6 +655,41 @@ public class CoyoteAdapter implements Ad > } > > > + private static class RecycleRequiredException extends Exception { > + private static final long serialVersionUID = 1L; > + } > + > + @Override > + public void checkRecycled(org.apache.coyote.Request req, > + org.apache.coyote.Response res) { > + Request request = (Request) req.getNote(ADAPTER_NOTES); > + Response response = (Response) res.getNote(ADAPTER_NOTES); > + try { > + if (request != null) { > + if (request.getContext() != null || request.getHost() != > null) > + throw new RecycleRequiredException(); > + } > + if (response != null) { > + if (response.getContentWritten() != 0) > + throw new RecycleRequiredException(); > + } > + } catch (RecycleRequiredException e) { > + String message = sm.getString("coyoteAdapter.checkRecycled"); > + if (connector.getState().isAvailable()) { > + log.warn(message, e); > + } else { > + // There may be some aborted requests. > + // When connector shuts down, the request and response will > not > + // be reused, so there is no issue to warn about here. > + log.debug(message, e); > + } > + // Log this request, as it has probably skipped the access log. > + // The log() method will take care of recycling. > + log(req, res, 0L); > + } > + }
Why is an exception being used for flow control? Surely a boolean flag would be better here especially as this is being called on every request. I'm close to a -1 on this commit but I'd like to hear the explanation for the use of an exception first. <snip/> > Modified: tomcat/trunk/webapps/docs/changelog.xml > URL: > http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1594436&r1=1594435&r2=1594436&view=diff > ============================================================================== > --- tomcat/trunk/webapps/docs/changelog.xml (original) > +++ tomcat/trunk/webapps/docs/changelog.xml Wed May 14 00:44:33 2014 > @@ -225,6 +225,10 @@ > <bug>56348</bug>: Fix slow asynchronous read when read was performed > on > a non-container thread. (markt) > </fix> > + <add> > + <bug>56399</bug>: Assert that both Coyote and Catalina request > objects > + have been properly recycled. (kkolinko) > + </add> I remain of the view that a better solution would be to recycle both pairs of request and response objects at the same time. Mark --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
