8309200: java/net/httpclient/ExecutorShutdown fails intermittently, if connection closed during upgrade

Reviewed-by: jpai, djelinski
This commit is contained in:
Daniel Fuchs 2023-06-02 14:34:54 +00:00
parent dc21e8aa83
commit 931913fbb2
7 changed files with 60 additions and 18 deletions

View File

@ -1,5 +1,5 @@
/* /*
* Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
* *
* This code is free software; you can redistribute it and/or modify it * This code is free software; you can redistribute it and/or modify it
@ -111,6 +111,11 @@ class AsyncSSLConnection extends AbstractAsyncSSLConnection {
plainConnection.close(); plainConnection.close();
} }
@Override
void close(Throwable cause) {
plainConnection.close(cause);
}
@Override @Override
SSLTube getConnectionFlow() { SSLTube getConnectionFlow() {
return flow; return flow;

View File

@ -1,5 +1,5 @@
/* /*
* Copyright (c) 2015, 2020, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
* *
* This code is free software; you can redistribute it and/or modify it * This code is free software; you can redistribute it and/or modify it
@ -113,6 +113,11 @@ class AsyncSSLTunnelConnection extends AbstractAsyncSSLConnection {
plainConnection.close(); plainConnection.close();
} }
@Override
void close(Throwable cause) {
plainConnection.close(cause);
}
@Override @Override
SocketChannel channel() { SocketChannel channel() {
return plainConnection.channel(); return plainConnection.channel();

View File

@ -142,6 +142,7 @@ final class Exchange<T> {
static final class ConnectionAborter { static final class ConnectionAborter {
private volatile HttpConnection connection; private volatile HttpConnection connection;
private volatile boolean closeRequested; private volatile boolean closeRequested;
private volatile Throwable cause;
void connection(HttpConnection connection) { void connection(HttpConnection connection) {
boolean closeRequested; boolean closeRequested;
@ -156,20 +157,27 @@ final class Exchange<T> {
this.closeRequested = false; this.closeRequested = false;
} }
} }
if (closeRequested) closeConnection(connection); if (closeRequested) closeConnection(connection, cause);
} }
void closeConnection() { void closeConnection(Throwable error) {
HttpConnection connection; HttpConnection connection;
Throwable cause;
synchronized (this) { synchronized (this) {
cause = this.cause;
if (cause == null) {
cause = error;
}
connection = this.connection; connection = this.connection;
if (connection == null) { if (connection == null) {
closeRequested = true; closeRequested = true;
this.cause = cause;
} else { } else {
this.connection = null; this.connection = null;
this.cause = null;
} }
} }
closeConnection(connection); closeConnection(connection, cause);
} }
HttpConnection disable() { HttpConnection disable() {
@ -178,14 +186,15 @@ final class Exchange<T> {
connection = this.connection; connection = this.connection;
this.connection = null; this.connection = null;
this.closeRequested = false; this.closeRequested = false;
this.cause = null;
} }
return connection; return connection;
} }
private static void closeConnection(HttpConnection connection) { private static void closeConnection(HttpConnection connection, Throwable cause) {
if (connection != null) { if (connection != null) {
try { try {
connection.close(); connection.close(cause);
} catch (Throwable t) { } catch (Throwable t) {
// ignore // ignore
} }
@ -264,11 +273,19 @@ final class Exchange<T> {
impl.cancel(cause); impl.cancel(cause);
} else { } else {
// no impl yet. record the exception // no impl yet. record the exception
failed = cause; IOException failed = this.failed;
if (failed == null) {
synchronized (this) {
failed = this.failed;
if (failed == null) {
failed = this.failed = cause;
}
}
}
// abort/close the connection if setting up the exchange. This can // abort/close the connection if setting up the exchange. This can
// be important when setting up HTTP/2 // be important when setting up HTTP/2
connectionAborter.closeConnection(); connectionAborter.closeConnection(failed);
// now call checkCancelled to recheck the impl. // now call checkCancelled to recheck the impl.
// if the failed state is set and the impl is not null, reset // if the failed state is set and the impl is not null, reset

View File

@ -1,5 +1,5 @@
/* /*
* Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved. * Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
* *
* This code is free software; you can redistribute it and/or modify it * This code is free software; you can redistribute it and/or modify it
@ -427,11 +427,19 @@ abstract class HttpConnection implements Closeable {
abstract ConnectionPool.CacheKey cacheKey(); abstract ConnectionPool.CacheKey cacheKey();
/** /**
* Closes this connection, by returning the socket to its connection pool. * Closes this connection.
*/ */
@Override @Override
public abstract void close(); public abstract void close();
/**
* Closes this connection due to the given cause.
* @param cause the cause for which the connection is closed, may be null
*/
void close(Throwable cause) {
close();
}
abstract FlowTube getConnectionFlow(); abstract FlowTube getConnectionFlow();
/** /**

View File

@ -401,11 +401,13 @@ class PlainHttpConnection extends HttpConnection {
return "PlainHttpConnection: " + super.toString(); return "PlainHttpConnection: " + super.toString();
} }
/**
* Closes this connection
*/
@Override @Override
public void close() { public void close() {
close(null);
}
@Override
void close(Throwable cause) {
var closed = this.closed; var closed = this.closed;
if (closed) return; if (closed) return;
stateLock.lock(); stateLock.lock();
@ -423,7 +425,7 @@ class PlainHttpConnection extends HttpConnection {
} }
try { try {
chan.close(); chan.close();
tube.signalClosed(); tube.signalClosed(cause);
} finally { } finally {
client().connectionClosed(this); client().connectionClosed(this);
} }

View File

@ -157,7 +157,12 @@ final class PlainTunnelingConnection extends HttpConnection {
@Override @Override
public void close() { public void close() {
delegate.close(); close(null);
}
@Override
void close(Throwable cause) {
delegate.close(cause);
connected = false; connected = false;
} }

View File

@ -149,7 +149,7 @@ final class SocketTube implements FlowTube {
// Events // // Events //
// ======================================================================// // ======================================================================//
void signalClosed() { void signalClosed(Throwable cause) {
// Ensures that the subscriber will be terminated and that future // Ensures that the subscriber will be terminated and that future
// subscribers will be notified when the connection is closed. // subscribers will be notified when the connection is closed.
if (Log.channel()) { if (Log.channel()) {
@ -157,7 +157,7 @@ final class SocketTube implements FlowTube {
channelDescr()); channelDescr());
} }
readPublisher.subscriptionImpl.signalError( readPublisher.subscriptionImpl.signalError(
new IOException("connection closed locally")); new IOException("connection closed locally", cause));
} }
/** /**