changeset 53300:54aa3ea04fe8

8216974: HttpConnection not returned to the pool after 204 response Summary: MultiExchange now call nullBody() on Exchange after receiving 204 Reviewed-by: chegar
author dfuchs
date Tue, 15 Jan 2019 11:34:20 +0000
parents 0b2574a2a6c7
children 50355c3d35c0
files src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java src/java.net.http/share/classes/jdk/internal/net/http/ExchangeImpl.java src/java.net.http/share/classes/jdk/internal/net/http/Http1Exchange.java src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java src/java.net.http/share/classes/jdk/internal/net/http/HttpConnection.java src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java test/jdk/java/net/httpclient/Response204.java
diffstat 7 files changed, 66 insertions(+), 10 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java	Tue Jan 15 08:03:30 2019 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java	Tue Jan 15 11:34:20 2019 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -151,6 +151,13 @@
         }
     }
 
+    // Called for 204 response - when no body is permitted
+    // This is actually only needed for HTTP/1.1 in order
+    // to return the connection to the pool (or close it)
+    void nullBody(HttpResponse<T> resp, Throwable t) {
+        exchImpl.nullBody(resp, t);
+    }
+
     public CompletableFuture<T> readBodyAsync(HttpResponse.BodyHandler<T> handler) {
         // If we received a 407 while establishing the exchange
         // there will be no body to read: bodyIgnored will be true,
--- a/src/java.net.http/share/classes/jdk/internal/net/http/ExchangeImpl.java	Tue Jan 15 08:03:30 2019 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/ExchangeImpl.java	Tue Jan 15 11:34:20 2019 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -161,6 +161,12 @@
         }
     }
 
+    // Called for 204 response - when no body is permitted
+    void nullBody(HttpResponse<T> resp, Throwable t) {
+        // only needed for HTTP/1.1 to close the connection
+        // or return it to the pool
+    }
+
     /* The following methods have separate HTTP/1.1 and HTTP/2 implementations */
 
     abstract CompletableFuture<ExchangeImpl<T>> sendHeadersAsync();
@@ -177,6 +183,7 @@
      */
     abstract CompletableFuture<Void> ignoreBody();
 
+
     /** Gets the response headers. Completes before body is read. */
     abstract CompletableFuture<Response> getResponseAsync(Executor executor);
 
--- a/src/java.net.http/share/classes/jdk/internal/net/http/Http1Exchange.java	Tue Jan 15 08:03:30 2019 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http1Exchange.java	Tue Jan 15 11:34:20 2019 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -28,6 +28,7 @@
 import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.net.http.HttpClient;
+import java.net.http.HttpResponse;
 import java.net.http.HttpResponse.BodyHandler;
 import java.net.http.HttpResponse.BodySubscriber;
 import java.nio.ByteBuffer;
@@ -382,6 +383,13 @@
         return response.ignoreBody(executor);
     }
 
+    // Used for those response codes that have no body associated
+    @Override
+    public void nullBody(HttpResponse<T> resp, Throwable t) {
+       response.nullBody(resp, t);
+    }
+
+
     ByteBuffer drainLeftOverBytes() {
         synchronized (lock) {
             asyncReceiver.stop();
--- a/src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java	Tue Jan 15 08:03:30 2019 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java	Tue Jan 15 11:34:20 2019 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -267,6 +267,15 @@
         }
     }
 
+    // Used for those response codes that have no body associated
+    public void nullBody(HttpResponse<T> resp, Throwable t) {
+        if (t != null) connection.close();
+        else {
+            return2Cache = !request.isWebSocket();
+            onFinished();
+        }
+    }
+
     static final Flow.Subscription NOP = new Flow.Subscription() {
         @Override
         public void request(long n) { }
--- a/src/java.net.http/share/classes/jdk/internal/net/http/HttpConnection.java	Tue Jan 15 08:03:30 2019 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/HttpConnection.java	Tue Jan 15 11:34:20 2019 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -85,8 +85,8 @@
                 new IdentityHashMap<>();
         void add(CompletionStage<?> cf) {
             synchronized(operations) {
+                operations.put(cf, Boolean.TRUE);
                 cf.whenComplete((r,t)-> remove(cf));
-                operations.put(cf, Boolean.TRUE);
             }
         }
         boolean remove(CompletionStage<?> cf) {
--- a/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java	Tue Jan 15 08:03:30 2019 +0100
+++ b/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java	Tue Jan 15 11:34:20 2019 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -242,7 +242,8 @@
                 result.complete(this.response);
             }
         });
-        return result;
+        // ensure that the connection is closed or returned to the pool.
+        return result.whenComplete(exch::nullBody);
     }
 
     private CompletableFuture<HttpResponse<T>>
--- a/test/jdk/java/net/httpclient/Response204.java	Tue Jan 15 08:03:30 2019 +0100
+++ b/test/jdk/java/net/httpclient/Response204.java	Tue Jan 15 11:34:20 2019 +0000
@@ -23,7 +23,7 @@
 
 /**
  * @test
- * @bug 8211437
+ * @bug 8211437 8216974
  * @run main/othervm -Djdk.httpclient.HttpClient.log=headers,requests Response204
  * @summary
  */
@@ -35,6 +35,7 @@
 import java.net.http.HttpResponse;
 import java.util.*;
 import java.util.concurrent.*;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.logging.*;
 import java.io.*;
 import java.net.*;
@@ -44,6 +45,9 @@
  */
 public class Response204 {
 
+    // check for 8216974
+    static final AtomicReference<Exception> serverError = new AtomicReference<>();
+
     public static void main (String[] args) throws Exception {
         Logger logger = Logger.getLogger ("com.sun.net.httpserver");
         ConsoleHandler c = new ConsoleHandler();
@@ -80,6 +84,10 @@
             } catch (IOException ioe) {
                 System.out.println("OK 2");
             }
+
+            // check for 8216974
+            Exception error = serverError.get();
+            if (error != null) throw error;
         } finally {
             server.stop(2);
             executor.shutdown();
@@ -90,17 +98,33 @@
 
     static class Handler implements HttpHandler {
         volatile int counter = 0;
+        volatile InetSocketAddress remote;
 
         public void handle(HttpExchange t)
                 throws IOException {
             InputStream is = t.getRequestBody();
             Headers map = t.getRequestHeaders();
             Headers rmap = t.getResponseHeaders();
+            if (counter % 2 == 0) {
+                // store the client's address
+                remote = t.getRemoteAddress();
+                System.out.println("Server received request from: " + remote);
+            }
             while (is.read() != -1) ;
             is.close();
-            if (counter++ == 1) {
+            if ((++counter) % 2 == 0) {
                 // pretend there is a body
                 rmap.set("Content-length", "10");
+                // 8216974: the client should have returned the connection
+                // to the pool and should therefore have the same
+                // remote address.
+                if (!t.getRemoteAddress().equals(remote)) {
+                    String msg = "Unexpected remote address: "
+                            + t.getRemoteAddress()
+                            + " - should have been " + remote;
+                    System.out.println(msg);
+                    serverError.set(new Exception(msg));
+                }
             }
             t.sendResponseHeaders(204, -1);
             t.close();