On 15/01/2015 14:28, r...@apache.org wrote: > Author: remm > Date: Thu Jan 15 14:28:13 2015 > New Revision: 1652108 > > URL: http://svn.apache.org/r1652108 > Log: > Fix use of the semaphore (it seems equivalent to not using it). This fixes > the ab corruption I was experiencing.
Thanks. That fixes the performance issue I was seeing (which was caused by an exception during socket close so the lower maxKeepAliveRequests the more obvious the problem became). It certainly drives home the message that you never, ever want exceptions routinely occurring on the critical path. I'll run the unit tests locally as well as let the CI system do its thing to see if I still get the unit test failures I got when I tried to the patch you just applied. My best guess right now is the patch below exposed another error that, if I am lucky, I have already fixed. Mark > Modified: > tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java > > Modified: tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java?rev=1652108&r1=1652107&r2=1652108&view=diff > ============================================================================== > --- tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java (original) > +++ tomcat/trunk/java/org/apache/tomcat/util/net/Nio2Endpoint.java Thu Jan 15 > 14:28:13 2015 > @@ -744,11 +744,13 @@ public class Nio2Endpoint extends Abstra > failed(new ClosedChannelException(), attachment); > return; > } > + readPending.release(); > getEndpoint().processSocket(attachment, > SocketStatus.OPEN_READ, true); > } > > @Override > public void failed(Throwable exc, SocketWrapperBase<Nio2Channel> > attachment) { > + readPending.release(); > getEndpoint().processSocket(attachment, > SocketStatus.DISCONNECT, true); > } > }; > @@ -1310,9 +1312,6 @@ public class Nio2Endpoint extends Abstra > getSocket().getBufHandler().configureReadBufferForWrite(); > getSocket().read(getSocket().getBufHandler().getReadBuffer(), > getTimeout(), TimeUnit.MILLISECONDS, this, > awaitBytesHandler); > - // TODO Figure out why moving this to the awaitBytesHandler > - // causes test failures. > - readPending.release(); > } > } > } > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
