Simplify http/1.1 client request reset and prevent a reset connection…

… to be reused from the pool
eclipse-vertx · Oct 7, 2017 · 59ec940 · 59ec940
1 parent 1ed7843
commit 59ec940
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 36 deletions.
diff --git a/src/main/java/io/vertx/core/http/impl/ClientConnection.java b/src/main/java/io/vertx/core/http/impl/ClientConnection.java
@@ -254,7 +254,7 @@ private void handshakeComplete(ChannelHandlerContext ctx, FullHttpResponse respo
   }
 
   public boolean isValid() {
-    return chctx.channel().isOpen();
+    return !reset && chctx.channel().isOpen();
   }
 
   int getOutstandingRequestCount() {
@@ -431,23 +431,19 @@ public ContextImpl getContext() {
     return super.getContext();
   }
 
-  public void resetRequest(long code) {
+  @Override
+  public void reset(long code) {
     if (!reset) {
       reset = true;
-      currentRequest = null;
+      if (currentRequest != null) {
-      requests.removeLast();
+        requests.removeLast();
+      }
       if (requests.size() == 0) {
         pool.responseEnded(this, true);
       }
     }
   }
 
-  @Override
-  public void resetResponse(long code) {
-    reset = true;
-    pool.responseEnded(this, true);
-  }
-
   private HttpRequest createRequest(HttpVersion version, HttpMethod method, String rawMethod, String uri, MultiMap headers) {
     DefaultHttpRequest request = new DefaultHttpRequest(HttpUtils.toNettyHttpVersion(version), HttpUtils.toNettyHttpMethod(method, rawMethod), uri, false);
     if (headers != null) {

diff --git a/src/main/java/io/vertx/core/http/impl/Http2ClientConnection.java b/src/main/java/io/vertx/core/http/impl/Http2ClientConnection.java
@@ -373,7 +373,7 @@ public void endRequest() {
     }
 
     @Override
-    public void resetRequest(long code) {
+    public void reset(long code) {
       if (!(requestEnded && responseEnded)) {
         requestEnded = true;
         responseEnded = true;
@@ -384,11 +384,6 @@ public void resetRequest(long code) {
       }
     }
 
-    @Override
-    public void resetResponse(long code) {
-      resetRequest(code);
-    }
-
     @Override
     public HttpClientConnection connection() {
       return conn;

diff --git a/src/main/java/io/vertx/core/http/impl/HttpClientRequestImpl.java b/src/main/java/io/vertx/core/http/impl/HttpClientRequestImpl.java
@@ -392,16 +392,12 @@ public boolean reset(long code) {
         reset = code;
         if (!completed) {
           completed = true;
-          if (stream != null) {
-            stream.resetRequest(code);
-          }
           if (completionHandler != null) {
             completionHandler.handle(null);
           }
-        } else {
+        }
-          if (response != null) {
+        if (stream != null) {
-            stream.resetResponse(code);
+          stream.reset(code);
-          }
         }
         return true;
       }
@@ -510,9 +506,7 @@ else if (completed) {
   }
 
   protected void doHandleResponse(HttpClientResponseImpl resp, long timeoutMs) {
-    if (reset != null) {
+    if (reset == null) {
-      stream.resetResponse(reset);
-    } else {
       response = resp;
       int statusCode = resp.statusCode();
       if (followRedirects > 0 && statusCode >= 300 && statusCode < 400) {

diff --git a/src/main/java/io/vertx/core/http/impl/HttpClientRequestPushPromise.java b/src/main/java/io/vertx/core/http/impl/HttpClientRequestPushPromise.java
@@ -100,7 +100,7 @@ public HttpClientRequest connectionHandler(@Nullable Handler<HttpConnection> han
   @Override
   public boolean reset(long code) {
     synchronized (conn) {
-      stream.resetRequest(code);
+      stream.reset(code);
       return true;
     }
   }

diff --git a/src/main/java/io/vertx/core/http/impl/HttpClientStream.java b/src/main/java/io/vertx/core/http/impl/HttpClientStream.java
@@ -32,7 +32,7 @@ interface HttpClientStream {
    * @return the stream id or -1 for HTTP/1.x
    */
   int id();
-  
+
   /**
    * @return the stream version or null if it's not yet determined
    */
@@ -52,8 +52,7 @@ interface HttpClientStream {
   void doPause();
   void doResume();
 
-  void resetRequest(long code);
+  void reset(long code);
-  void resetResponse(long code);
 
   void beginRequest(HttpClientRequestImpl request);
   void endRequest();

diff --git a/src/test/java/io/vertx/test/core/Http1xTest.java b/src/test/java/io/vertx/test/core/Http1xTest.java
@@ -3299,13 +3299,13 @@ private void testCloseTheConnectionAfterResetBeforeResponseReceived(boolean pipe
           case 0:
             so.handler(buff -> {
               total.appendBuffer(buff);
-              if (total.toString().equals("GET /somepath HTTP/1.1\r\n" +
+              if (total.toString().equals("GET /1 HTTP/1.1\r\n" +
                   "Host: localhost:8080\r\n" +
                   "\r\n")) {
                 requestReceived.complete(null);
                 so.write(Buffer.buffer(
                     "HTTP/1.1 200 OK\r\n" +
-                        "Content-Length: 200\r\n" +
+                        "Content-Length: 11\r\n" +
                         "\r\n" +
                         "Some-Buffer"
                 ));
@@ -3318,7 +3318,7 @@ private void testCloseTheConnectionAfterResetBeforeResponseReceived(boolean pipe
           case 1:
             so.handler(buff -> {
               total.appendBuffer(buff);
-              if (total.toString().equals("GET /somepath HTTP/1.1\r\n" +
+              if (total.toString().equals("GET /2 HTTP/1.1\r\n" +
                   "Host: localhost:8080\r\n" +
                   "\r\n")) {
                 so.write(
@@ -3344,11 +3344,11 @@ private void testCloseTheConnectionAfterResetBeforeResponseReceived(boolean pipe
       awaitLatch(listenLatch);
       client.close();
       client = vertx.createHttpClient(new HttpClientOptions().setMaxPoolSize(1).setPipelining(pipelined).setKeepAlive(true));
-      HttpClientRequest req1 = client.get(DEFAULT_HTTP_PORT, DEFAULT_HTTP_HOST, "/somepath");
+      HttpClientRequest req1 = client.get(DEFAULT_HTTP_PORT, DEFAULT_HTTP_HOST, "/1");
       if (pipelined) {
         requestReceived.thenAccept(v -> {
           req1.reset();
-          client.getNow(DEFAULT_HTTP_PORT, DEFAULT_HTTP_HOST, "/somepath", resp -> {
+          client.getNow(DEFAULT_HTTP_PORT, DEFAULT_HTTP_HOST, "/2", resp -> {
             assertEquals(200, resp.statusCode());
             resp.bodyHandler(body -> {
               assertEquals("Hello world", body.toString());
@@ -3364,7 +3364,7 @@ private void testCloseTheConnectionAfterResetBeforeResponseReceived(boolean pipe
         });
         req1.handler(resp -> fail());
         req1.end();
-        client.getNow(DEFAULT_HTTP_PORT, DEFAULT_HTTP_HOST, "/somepath", resp -> {
+        client.getNow(DEFAULT_HTTP_PORT, DEFAULT_HTTP_HOST, "/2", resp -> {
           assertEquals(200, resp.statusCode());
           resp.bodyHandler(body -> {
             assertEquals("Hello world", body.toString());