hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ol...@apache.org
Subject svn commit: r660239 - in /httpcomponents/httpcore/trunk: ./ module-nio/src/main/java/org/apache/http/nio/protocol/ module-nio/src/test/java/org/apache/http/nio/protocol/
Date Mon, 26 May 2008 17:29:52 GMT
Author: olegk
Date: Mon May 26 10:29:50 2008
New Revision: 660239

URL: http://svn.apache.org/viewvc?rev=660239&view=rev
Log:
Fixed concurrency bug in the ThrottlingHttpServerHandler protocol handler

Modified:
    httpcomponents/httpcore/trunk/RELEASE_NOTES.txt
    httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/nio/protocol/ThrottlingHttpClientHandler.java
    httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/nio/protocol/ThrottlingHttpServiceHandler.java
    httpcomponents/httpcore/trunk/module-nio/src/test/java/org/apache/http/nio/protocol/TestThrottlingNHttpHandlers.java

Modified: httpcomponents/httpcore/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/RELEASE_NOTES.txt?rev=660239&r1=660238&r2=660239&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpcore/trunk/RELEASE_NOTES.txt Mon May 26 10:29:50 2008
@@ -1,5 +1,8 @@
 Changes since 4.0 Beta 1
 -------------------
+* Fixed concurrency bug in the ThrottlingHttpServerHandler protocol handler.
+  Oleg Kalnichevski <olegk at apache.org> 
+
 * Fixed bug in SharedInputBuffer that caused input events to be
   incorrectly suspended.
   Contributed by Asankha C. Perera <asankha at wso2.com>

Modified: httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/nio/protocol/ThrottlingHttpClientHandler.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/nio/protocol/ThrottlingHttpClientHandler.java?rev=660239&r1=660238&r2=660239&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/nio/protocol/ThrottlingHttpClientHandler.java
(original)
+++ httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/nio/protocol/ThrottlingHttpClientHandler.java
Mon May 26 10:29:50 2008
@@ -47,7 +47,6 @@
 import org.apache.http.nio.IOControl;
 import org.apache.http.nio.NHttpClientConnection;
 import org.apache.http.nio.NHttpClientHandler;
-import org.apache.http.nio.NHttpConnection;
 import org.apache.http.nio.entity.ContentBufferEntity;
 import org.apache.http.nio.entity.ContentOutputStream;
 import org.apache.http.nio.params.NIOReactorPNames;
@@ -135,7 +134,19 @@
 
     public void closed(final NHttpClientConnection conn) {
         HttpContext context = conn.getContext();
+        ClientConnState connState = (ClientConnState) context.getAttribute(CONN_STATE);
+        
+        if (connState != null) {
+            synchronized (connState) {
+                connState.shutdown();
+                connState.notifyAll();
+            }
+        }
 
+        if (this.eventListener != null) {
+            this.eventListener.connectionClosed(conn);
+        }
+        
         this.execHandler.finalizeContext(context);
         
         if (this.eventListener != null) {
@@ -532,19 +543,6 @@
         
     }
     
-    @Override
-    protected void shutdownConnection(final NHttpConnection conn, final Throwable cause)
{
-        HttpContext context = conn.getContext();
-
-        ClientConnState connState = (ClientConnState) context.getAttribute(CONN_STATE);
-        
-        super.shutdownConnection(conn, cause);
-        
-        if (connState != null) {
-            connState.shutdown();
-        }
-    }
-    
     static class ClientConnState {
         
         public static final int SHUTDOWN                   = -1;

Modified: httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/nio/protocol/ThrottlingHttpServiceHandler.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/nio/protocol/ThrottlingHttpServiceHandler.java?rev=660239&r1=660238&r2=660239&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/nio/protocol/ThrottlingHttpServiceHandler.java
(original)
+++ httpcomponents/httpcore/trunk/module-nio/src/main/java/org/apache/http/nio/protocol/ThrottlingHttpServiceHandler.java
Mon May 26 10:29:50 2008
@@ -154,7 +154,10 @@
         ServerConnState connState = (ServerConnState) context.getAttribute(CONN_STATE);
         
         if (connState != null) {
-            connState.shutdown();
+            synchronized (connState) {
+                connState.shutdown();
+                connState.notifyAll();
+            }
         }
 
         if (this.eventListener != null) {
@@ -299,9 +302,8 @@
         ServerConnState connState = (ServerConnState) context.getAttribute(CONN_STATE);
 
         try {
-        
-            synchronized (connState) {
-                
+            
+            synchronized (connState) {                
                 if (connState.isExpectationFailed()) {
                     // Server expection failed
                     // Well-behaved client will not be sending
@@ -321,11 +323,6 @@
 
                     if (statusCode >= 200 && entity == null) {
                         connState.setOutputState(ServerConnState.RESPONSE_DONE);
-                        if (!connState.isWorkerRunning()) {
-                            connState.resetOutput();
-                            connState.resetInput();
-                            conn.requestInput();
-                        }
 
                         if (!this.connStrategy.keepAlive(response, context)) {
                             conn.close();
@@ -366,12 +363,6 @@
                 if (encoder.isCompleted()) {
                     connState.setOutputState(ServerConnState.RESPONSE_BODY_DONE);
 
-                    if (!connState.isWorkerRunning()) {
-                        connState.resetOutput();
-                        connState.resetInput();
-                        conn.requestInput();
-                    }
-
                     if (!this.connStrategy.keepAlive(response, context)) {
                         conn.close();
                     }
@@ -419,7 +410,7 @@
             try {
                 for (;;) {
                     int currentState = connState.getOutputState();
-                    if (!connState.isWorkerRunning()) {
+                    if (currentState == ServerConnState.READY) {
                         break;
                     }
                     if (currentState == ServerConnState.SHUTDOWN) {
@@ -433,7 +424,6 @@
             }
             connState.setInputState(ServerConnState.REQUEST_RECEIVED);
             connState.setRequest(request);
-            connState.setWorkerRunning(true);
         }
 
         request.setParams(new DefaultedHttpParams(request.getParams(), this.params));
@@ -557,6 +547,9 @@
             }
         }
         
+        // It should be safe to reset the input state at this point
+        connState.resetInput();
+        
         this.httpProcessor.process(response, context);
 
         if (!canResponseHaveBody(request, response)) {
@@ -578,13 +571,23 @@
         }
         
         synchronized (connState) {
-            if (connState.getOutputState() == ServerConnState.RESPONSE_DONE 
-                    && conn.isOpen()) {
-                connState.resetInput();
-                connState.resetOutput();
-                conn.requestInput();
+            try {
+                for (;;) {
+                    int currentState = connState.getOutputState();
+                    if (currentState == ServerConnState.RESPONSE_DONE) {
+                        break;
+                    }
+                    if (currentState == ServerConnState.SHUTDOWN) {
+                        return;
+                    }
+                    connState.wait();
+                }
+            } catch (InterruptedException ex) {
+                connState.shutdown();
+                return;
             }
-            connState.setWorkerRunning(false);
+            connState.resetOutput();
+            conn.requestInput();
             connState.notifyAll();
         }
     }
@@ -624,7 +627,6 @@
         private volatile HttpResponse response;
 
         private volatile boolean expectationFailure;
-        private volatile boolean workerRunning; 
         
         public ServerConnState(
                 int bufsize, 
@@ -677,14 +679,6 @@
             this.response = response;
         }
 
-        public boolean isWorkerRunning() {
-            return this.workerRunning;
-        }
-
-        public void setWorkerRunning(boolean b) {
-            this.workerRunning = b;
-        }
-
         public boolean isExpectationFailed() {
             return expectationFailure;
         }
@@ -704,13 +698,13 @@
             this.inbuffer.reset();
             this.request = null;
             this.inputState = READY;
-            this.expectationFailure = false;
         }
         
         public void resetOutput() {
             this.outbuffer.reset();
             this.response = null;
             this.outputState = READY;
+            this.expectationFailure = false;
         }
         
     }    

Modified: httpcomponents/httpcore/trunk/module-nio/src/test/java/org/apache/http/nio/protocol/TestThrottlingNHttpHandlers.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/module-nio/src/test/java/org/apache/http/nio/protocol/TestThrottlingNHttpHandlers.java?rev=660239&r1=660238&r2=660239&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/module-nio/src/test/java/org/apache/http/nio/protocol/TestThrottlingNHttpHandlers.java
(original)
+++ httpcomponents/httpcore/trunk/module-nio/src/test/java/org/apache/http/nio/protocol/TestThrottlingNHttpHandlers.java
Mon May 26 10:29:50 2008
@@ -31,6 +31,7 @@
 package org.apache.http.nio.protocol;
 
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.InterruptedIOException;
 import java.io.PipedInputStream;
 import java.io.PipedOutputStream;
@@ -42,6 +43,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Random;
 import java.util.concurrent.Executor;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
@@ -1507,4 +1509,134 @@
 
     }
 
+    public void testInputThrottling() throws Exception {
+
+        final int connNo = 3;
+        final int reqNo = 20;
+        final RequestCount requestCount = new RequestCount(connNo * reqNo);
+
+        HttpRequestHandler requestHandler = new HttpRequestHandler() {
+
+            public void handle(
+                    final HttpRequest request,
+                    final HttpResponse response,
+                    final HttpContext context) throws HttpException, IOException {
+
+                if (request instanceof HttpEntityEnclosingRequest) {
+                    HttpEntity incoming = ((HttpEntityEnclosingRequest) request).getEntity();
+                    byte[] data = EntityUtils.toByteArray(incoming);
+                    NByteArrayEntity outgoing = new NByteArrayEntity(data);
+                    outgoing.setChunked(true);
+                    response.setEntity(outgoing);
+                } else {
+                    NStringEntity outgoing = new NStringEntity("No content");
+                    response.setEntity(outgoing);
+                }
+            }
+
+        };
+        
+        HttpRequestExecutionHandler requestExecutionHandler = new HttpRequestExecutionHandler()
{
+
+            public void initalizeContext(final HttpContext context, final Object attachment)
{
+                context.setAttribute("REQ-COUNT", new Integer(0));
+                context.setAttribute("RES-COUNT", new Integer(0));
+            }
+
+            public void finalizeContext(final HttpContext context) {
+            }
+
+            public HttpRequest submitRequest(final HttpContext context) {
+                int i = ((Integer) context.getAttribute("REQ-COUNT")).intValue();
+                BasicHttpEntityEnclosingRequest post = null;
+                if (i < reqNo) {
+                    post = new BasicHttpEntityEnclosingRequest("POST", "/?" + i);
+                    Random rnd = new Random();
+                    int size = rnd.nextInt(100000);
+                    byte[] data = new byte[size];
+                    rnd.nextBytes(data);
+                    NByteArrayEntity outgoing = new NByteArrayEntity(data);
+                    outgoing.setChunked(i % 2 == 0);
+                    post.setEntity(outgoing);
+                    context.setAttribute("REQ-COUNT", new Integer(i + 1));
+                }
+                return post;
+            }
+
+            public void handleResponse(final HttpResponse response, final HttpContext context)
{
+                NHttpConnection conn = (NHttpConnection) context.getAttribute(
+                        ExecutionContext.HTTP_CONNECTION);
+
+                int i = ((Integer) context.getAttribute("RES-COUNT")).intValue();
+                i++;
+                context.setAttribute("RES-COUNT", new Integer(i));
+
+                HttpEntity entity = response.getEntity();
+                if (entity != null) {
+                    try {
+                        // Simulate slow response handling in order to cause the 
+                        // internal content buffer to fill up, forcing the 
+                        // protocol handler to throttle input rate 
+                        InputStream instream = entity.getContent();
+                        byte[] tmp = new byte[2048];
+                        while(instream.read(tmp) != -1) {
+                            Thread.sleep(1);
+                        }
+                        instream.close();
+                    } catch (InterruptedException ex) {
+                        requestCount.abort();
+                        return;
+                    } catch (IOException ex) {
+                        requestCount.abort();
+                        return;
+                    }
+                }
+
+                requestCount.decrement();
+
+                if (i < reqNo) {
+                    conn.requestInput();
+                }
+            }
+            
+        };
+
+        NHttpServiceHandler serviceHandler = createHttpServiceHandler(
+                requestHandler,
+                null,
+                this.execService);
+
+        NHttpClientHandler clientHandler = createHttpClientHandler(
+                requestExecutionHandler,
+                this.execService);
+
+        this.server.setRequestCount(requestCount);
+        this.client.setRequestCount(requestCount);
+        
+        this.server.start(serviceHandler);
+        this.client.start(clientHandler);
+
+        ListenerEndpoint endpoint = this.server.getListenerEndpoint();
+        endpoint.waitFor();
+        InetSocketAddress serverAddress = (InetSocketAddress) endpoint.getAddress();
+
+        for (int i = 0; i < connNo; i++) {
+            this.client.openConnection(
+                    new InetSocketAddress("localhost", serverAddress.getPort()),
+                    null);
+        }
+
+        requestCount.await(10000);
+        if (requestCount.isAborted()) {
+            System.out.println("Test case aborted");
+        }
+        assertEquals(0, requestCount.getValue());
+
+        this.client.shutdown();
+        this.server.shutdown();
+
+        this.execService.shutdown();
+        this.execService.awaitTermination(10, TimeUnit.SECONDS);
+    }
+    
 }



Mime
View raw message