8216974: HttpConnection not returned to the pool after 204 response
Summary: MultiExchange now call nullBody() on Exchange after receiving 204
Reviewed-by: chegar
--- 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();