8216974: HttpConnection not returned to the pool after 204 response

MultiExchange now call nullBody() on Exchange after receiving 204

Reviewed-by: chegar
This commit is contained in:
Daniel Fuchs 2019-01-15 11:34:20 +00:00
parent 617f75c8cf
commit ca77adf19f
7 changed files with 66 additions and 10 deletions

View file

@ -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 @@ final class Exchange<T> {
}
}
// 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,

View file

@ -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 @@ abstract class ExchangeImpl<T> {
}
}
// 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 class ExchangeImpl<T> {
*/
abstract CompletableFuture<Void> ignoreBody();
/** Gets the response headers. Completes before body is read. */
abstract CompletableFuture<Response> getResponseAsync(Executor executor);

View file

@ -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 @@ package jdk.internal.net.http;
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 @@ class Http1Exchange<T> extends ExchangeImpl<T> {
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();

View file

@ -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 @@ class Http1Response<T> {
}
}
// 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) { }

View file

@ -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 @@ abstract class HttpConnection implements Closeable {
new IdentityHashMap<>();
void add(CompletionStage<?> cf) {
synchronized(operations) {
cf.whenComplete((r,t)-> remove(cf));
operations.put(cf, Boolean.TRUE);
cf.whenComplete((r,t)-> remove(cf));
}
}
boolean remove(CompletionStage<?> cf) {

View file

@ -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 @@ class MultiExchange<T> {
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>>

View file

@ -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.HttpRequest;
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 @@ import java.net.*;
*/
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 @@ public class Response204 {
} 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 @@ public class Response204 {
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();