Skip to content

Commit 6f17c23

Browse files
GoeLinGoeLin
committed
Backport f3f078846feae66d3504d50081353f74bd4891d7
1 parent e299091 commit 6f17c23

File tree

8 files changed

+169
-85
lines changed

8 files changed

+169
-85
lines changed

‎src/java.base/share/classes/module-info.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,7 @@
226226
jdk.jfr;
227227
exports jdk.internal.ref to
228228
java.desktop,
229+
java.net.http,
229230
jdk.incubator.foreign;
230231
exports jdk.internal.reflect to
231232
java.logging,

‎src/java.base/share/lib/security/default.policy‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ grant codeBase "jrt:/java.net.http" {
1919
permission java.lang.RuntimePermission "accessClassInPackage.sun.net.util";
2020
permission java.lang.RuntimePermission "accessClassInPackage.sun.net.www";
2121
permission java.lang.RuntimePermission "accessClassInPackage.jdk.internal.misc";
22+
permission java.lang.RuntimePermission "accessClassInPackage.jdk.internal.ref";
2223
permission java.lang.RuntimePermission "modifyThread";
2324
permission java.net.SocketPermission "*","connect,resolve";
2425
permission java.net.URLPermission "http:*","*:*";

‎src/java.net.http/share/classes/jdk/internal/net/http/HttpClientFacade.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -26,6 +26,7 @@
2626
package jdk.internal.net.http;
2727

2828
import java.io.IOException;
29+
import java.lang.ref.Cleaner;
2930
import java.lang.ref.Reference;
3031
import java.net.Authenticator;
3132
import java.net.CookieHandler;
@@ -44,20 +45,29 @@
4445
import java.net.http.WebSocket;
4546
import jdk.internal.net.http.common.OperationTrackers.Trackable;
4647
import jdk.internal.net.http.common.OperationTrackers.Tracker;
48+
import jdk.internal.ref.CleanerFactory;
4749

4850
/**
4951
* An HttpClientFacade is a simple class that wraps an HttpClient implementation
5052
* and delegates everything to its implementation delegate.
53+
* @implSpec
54+
* Though the facade strongly reference its implementation, the
55+
* implementation MUST NOT strongly reference the facade.
56+
* It MAY use weak references if needed.
5157
*/
5258
public final class HttpClientFacade extends HttpClient implements Trackable {
5359

60+
static final Cleaner cleaner = CleanerFactory.cleaner();
61+
5462
final HttpClientImpl impl;
5563

5664
/**
5765
* Creates an HttpClientFacade.
5866
*/
5967
HttpClientFacade(HttpClientImpl impl) {
6068
this.impl = impl;
69+
// wakeup the impl when the facade is gc'ed
70+
cleaner.register(this, impl::facadeCleanup);
6171
}
6272

6373
@Override // for tests

‎src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,13 @@ private HttpClientImpl(HttpClientBuilderImpl builder,
491491
assert facadeRef.get() != null;
492492
}
493493

494+
// called when the facade is GC'ed.
495+
// Just wakes up the selector to cleanup...
496+
void facadeCleanup() {
497+
SelectorManager selmgr = this.selmgr;
498+
if (selmgr != null) selmgr.wakeupSelector();
499+
}
500+
494501
void onSubmitFailure(Runnable command, Throwable failure) {
495502
selmgr.abort(failure);
496503
}
@@ -723,7 +730,7 @@ public long getOutstandingWebSocketOperations() {
723730
}
724731
@Override
725732
public boolean isFacadeReferenced() {
726-
return reference.get() != null;
733+
return !reference.refersTo(null);
727734
}
728735
@Override
729736
public boolean isSelectorAlive() { return isAlive.get(); }
@@ -749,8 +756,7 @@ public Tracker getOperationsTracker() {
749756
// Called by the SelectorManager thread to figure out whether it's time
750757
// to terminate.
751758
boolean isReferenced() {
752-
HttpClient facade = facade();
753-
return facade != null || referenceCount() > 0;
759+
return !facadeRef.refersTo(null) || referenceCount() > 0;
754760
}
755761

756762
/**

‎test/jdk/java/net/httpclient/AsFileDownloadTest.java‎

Lines changed: 68 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import java.io.InputStream;
3030
import java.io.OutputStream;
3131
import java.io.UncheckedIOException;
32+
import java.lang.ref.ReferenceQueue;
33+
import java.lang.ref.WeakReference;
3234
import java.net.InetAddress;
3335
import java.net.InetSocketAddress;
3436
import java.net.URI;
@@ -77,7 +79,6 @@
7779
* @run testng/othervm AsFileDownloadTest
7880
* @run testng/othervm/java.security.policy=AsFileDownloadTest.policy AsFileDownloadTest
7981
*/
80-
8182
public class AsFileDownloadTest {
8283

8384
SSLContext sslContext;
@@ -89,6 +90,7 @@ public class AsFileDownloadTest {
8990
String httpsURI;
9091
String http2URI;
9192
String https2URI;
93+
final ReferenceTracker TRACKER = ReferenceTracker.INSTANCE;
9294

9395
Path tempDir;
9496

@@ -164,37 +166,49 @@ void test(String uriString, String contentDispositionValue, String expectedFilen
164166
{
165167
out.printf("test(%s, %s, %s): starting", uriString, contentDispositionValue, expectedFilename);
166168
HttpClient client = HttpClient.newBuilder().sslContext(sslContext).build();
169+
TRACKER.track(client);
170+
ReferenceQueue<HttpClient> queue = new ReferenceQueue<>();
171+
WeakReference<HttpClient> ref = new WeakReference<>(client, queue);
172+
try {
173+
URI uri = URI.create(uriString);
174+
HttpRequest request = HttpRequest.newBuilder(uri)
175+
.POST(BodyPublishers.ofString("May the luck of the Irish be with you!"))
176+
.build();
167177

168-
URI uri = URI.create(uriString);
169-
HttpRequest request = HttpRequest.newBuilder(uri)
170-
.POST(BodyPublishers.ofString("May the luck of the Irish be with you!"))
171-
.build();
172-
173-
BodyHandler bh = ofFileDownload(tempDir.resolve(uri.getPath().substring(1)),
174-
CREATE, TRUNCATE_EXISTING, WRITE);
175-
HttpResponse<Path> response = client.send(request, bh);
176-
177-
Path body = response.body();
178-
out.println("Got response: " + response);
179-
out.println("Got body Path: " + body);
180-
String fileContents = new String(Files.readAllBytes(response.body()), UTF_8);
181-
out.println("Got body: " + fileContents);
182-
183-
assertEquals(response.statusCode(),200);
184-
assertEquals(body.getFileName().toString(), expectedFilename);
185-
assertTrue(response.headers().firstValue("Content-Disposition").isPresent());
186-
assertEquals(response.headers().firstValue("Content-Disposition").get(),
187-
contentDispositionValue);
188-
assertEquals(fileContents, "May the luck of the Irish be with you!");
189-
190-
if (!body.toAbsolutePath().startsWith(tempDir.toAbsolutePath())) {
191-
System.out.println("Tempdir = " + tempDir.toAbsolutePath());
192-
System.out.println("body = " + body.toAbsolutePath());
193-
throw new AssertionError("body in wrong location");
178+
BodyHandler bh = ofFileDownload(tempDir.resolve(uri.getPath().substring(1)),
179+
CREATE, TRUNCATE_EXISTING, WRITE);
180+
HttpResponse<Path> response = client.send(request, bh);
181+
Path body = response.body();
182+
out.println("Got response: " + response);
183+
out.println("Got body Path: " + body);
184+
String fileContents = new String(Files.readAllBytes(response.body()), UTF_8);
185+
out.println("Got body: " + fileContents);
186+
187+
assertEquals(response.statusCode(), 200);
188+
assertEquals(body.getFileName().toString(), expectedFilename);
189+
assertTrue(response.headers().firstValue("Content-Disposition").isPresent());
190+
assertEquals(response.headers().firstValue("Content-Disposition").get(),
191+
contentDispositionValue);
192+
assertEquals(fileContents, "May the luck of the Irish be with you!");
193+
194+
if (!body.toAbsolutePath().startsWith(tempDir.toAbsolutePath())) {
195+
System.out.println("Tempdir = " + tempDir.toAbsolutePath());
196+
System.out.println("body = " + body.toAbsolutePath());
197+
throw new AssertionError("body in wrong location");
198+
}
199+
// additional checks unrelated to file download
200+
caseInsensitivityOfHeaders(request.headers());
201+
caseInsensitivityOfHeaders(response.headers());
202+
} finally {
203+
client = null;
204+
System.gc();
205+
while (!ref.refersTo(null)) {
206+
System.gc();
207+
if (queue.remove(100) == ref) break;
208+
}
209+
AssertionError failed = TRACKER.checkShutdown(1000);
210+
if (failed != null) throw failed;
194211
}
195-
// additional checks unrelated to file download
196-
caseInsensitivityOfHeaders(request.headers());
197-
caseInsensitivityOfHeaders(response.headers());
198212
}
199213

200214
// --- Negative
@@ -246,18 +260,32 @@ void negativeTest(String uriString, String contentDispositionValue)
246260
{
247261
out.printf("negativeTest(%s, %s): starting", uriString, contentDispositionValue);
248262
HttpClient client = HttpClient.newBuilder().sslContext(sslContext).build();
263+
TRACKER.track(client);
264+
ReferenceQueue<HttpClient> queue = new ReferenceQueue<>();
265+
WeakReference<HttpClient> ref = new WeakReference<>(client, queue);
249266

250-
URI uri = URI.create(uriString);
251-
HttpRequest request = HttpRequest.newBuilder(uri)
252-
.POST(BodyPublishers.ofString("Does not matter"))
253-
.build();
254-
255-
BodyHandler bh = ofFileDownload(tempDir, CREATE, TRUNCATE_EXISTING, WRITE);
256267
try {
257-
HttpResponse<Path> response = client.send(request, bh);
258-
fail("UNEXPECTED response: " + response + ", path:" + response.body());
259-
} catch (UncheckedIOException | IOException ioe) {
260-
System.out.println("Caught expected: " + ioe);
268+
URI uri = URI.create(uriString);
269+
HttpRequest request = HttpRequest.newBuilder(uri)
270+
.POST(BodyPublishers.ofString("Does not matter"))
271+
.build();
272+
273+
BodyHandler bh = ofFileDownload(tempDir, CREATE, TRUNCATE_EXISTING, WRITE);
274+
try {
275+
HttpResponse<Path> response = client.send(request, bh);
276+
fail("UNEXPECTED response: " + response + ", path:" + response.body());
277+
} catch (UncheckedIOException | IOException ioe) {
278+
System.out.println("Caught expected: " + ioe);
279+
}
280+
} finally {
281+
client = null;
282+
System.gc();
283+
while (!ref.refersTo(null)) {
284+
System.gc();
285+
if (queue.remove(100) == ref) break;
286+
}
287+
AssertionError failed = TRACKER.checkShutdown(1000);
288+
if (failed != null) throw failed;
261289
}
262290
}
263291

‎test/jdk/java/net/httpclient/DigestEchoClient.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323

2424
import java.io.IOException;
2525
import java.io.UncheckedIOException;
26+
import java.lang.ref.ReferenceQueue;
27+
import java.lang.ref.WeakReference;
2628
import java.math.BigInteger;
2729
import java.net.ProxySelector;
2830
import java.net.URI;
@@ -32,7 +34,6 @@
3234
import java.net.http.HttpRequest.BodyPublisher;
3335
import java.net.http.HttpRequest.BodyPublishers;
3436
import java.net.http.HttpResponse;
35-
import java.net.http.HttpResponse.BodyHandler;
3637
import java.net.http.HttpResponse.BodyHandlers;
3738
import java.nio.charset.StandardCharsets;
3839
import java.security.NoSuchAlgorithmException;
@@ -55,6 +56,7 @@
5556
import jdk.test.lib.net.SimpleSSLContext;
5657
import sun.net.NetProperties;
5758
import sun.net.www.HeaderParser;
59+
5860
import static java.lang.System.out;
5961
import static java.lang.String.format;
6062

@@ -386,6 +388,8 @@ void testBasic(Version clientVersion, Version serverVersion, boolean async,
386388
server.getServerAddress(), "/foo/");
387389

388390
HttpClient client = newHttpClient(server);
391+
ReferenceQueue<HttpClient> queue = new ReferenceQueue<>();
392+
WeakReference<HttpClient> ref = new WeakReference<>(client, queue);
389393
HttpResponse<String> r;
390394
CompletableFuture<HttpResponse<String>> cf1;
391395
String auth = null;
@@ -497,6 +501,14 @@ void testBasic(Version clientVersion, Version serverVersion, boolean async,
497501
}
498502
}
499503
} finally {
504+
client = null;
505+
System.gc();
506+
while (!ref.refersTo(null)) {
507+
System.gc();
508+
if (queue.remove(100) == ref) break;
509+
}
510+
var error = TRACKER.checkShutdown(500);
511+
if (error != null) throw error;
500512
}
501513
System.out.println("OK");
502514
}

‎test/jdk/java/net/httpclient/ReferenceTracker.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,13 +179,15 @@ public AssertionError check(long graceDelayMs,
179179
boolean printThreads) {
180180
AssertionError fail = null;
181181
graceDelayMs = Math.max(graceDelayMs, 100);
182-
long delay = Math.min(graceDelayMs, 500);
182+
long delay = Math.min(graceDelayMs, 10);
183183
var count = delay > 0 ? graceDelayMs / delay : 1;
184184
for (int i = 0; i < count; i++) {
185185
if (TRACKERS.stream().anyMatch(hasOutstanding)) {
186186
System.gc();
187187
try {
188-
System.out.println("Waiting for HTTP operations to terminate...");
188+
if (i == 0) {
189+
System.out.println("Waiting for HTTP operations to terminate...");
190+
}
189191
Thread.sleep(Math.min(graceDelayMs, Math.max(delay, 1)));
190192
} catch (InterruptedException x) {
191193
// OK

0 commit comments

Comments
 (0)