8191169: java/net/Authenticator/B4769350.java failed intermittently
Summary: fixed a race condition in AuthenticationInfo when serializeAuth=true
Reviewed-by: chegar, michaelm
--- a/src/java.base/share/classes/sun/net/www/protocol/http/AuthenticationInfo.java Mon Aug 19 06:13:52 2019 +0200
+++ b/src/java.base/share/classes/sun/net/www/protocol/http/AuthenticationInfo.java Mon Aug 19 11:14:50 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1995, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1995, 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
@@ -31,6 +31,7 @@
import java.net.URL;
import java.util.HashMap;
import java.util.Objects;
+import java.util.function.Function;
import sun.net.www.HeaderParser;
@@ -125,25 +126,42 @@
*/
private static HashMap<String,Thread> requests = new HashMap<>();
- /* check if a request for this destination is in progress
- * return false immediately if not. Otherwise block until
- * request is finished and return true
+ /*
+ * check if AuthenticationInfo is available in the cache.
+ * If not, check if a request for this destination is in progress
+ * and if so block until the other request is finished authenticating
+ * and returns the cached authentication value.
+ * Otherwise, returns the cached authentication value, which may be null.
*/
- private static boolean requestIsInProgress (String key) {
- if (!serializeAuth) {
- /* behavior is disabled. Revert to concurrent requests */
- return false;
+ private static AuthenticationInfo requestAuthentication(String key, Function<String, AuthenticationInfo> cache) {
+ AuthenticationInfo cached = cache.apply(key);
+ if (cached != null || !serializeAuth) {
+ // either we already have a value in the cache, and we can
+ // use that immediately, or the serializeAuth behavior is disabled,
+ // and we can revert to concurrent requests
+ return cached;
}
synchronized (requests) {
+ // check again after synchronizing, and if available
+ // just return the cached value.
+ cached = cache.apply(key);
+ if (cached != null) return cached;
+
+ // Otherwise, if no request is in progress, record this
+ // thread as performing authentication and returns null.
Thread t, c;
c = Thread.currentThread();
if ((t = requests.get(key)) == null) {
requests.put (key, c);
- return false;
+ assert cached == null;
+ return cached;
}
if (t == c) {
- return false;
+ assert cached == null;
+ return cached;
}
+ // Otherwise, an other thread is currently performing authentication:
+ // wait until it finishes.
while (requests.containsKey(key)) {
try {
requests.wait ();
@@ -151,7 +169,7 @@
}
}
/* entry may be in cache now. */
- return true;
+ return cache.apply(key);
}
/* signal completion of an authentication (whether it succeeded or not)
@@ -318,13 +336,13 @@
return key;
}
+ private static AuthenticationInfo getCachedServerAuth(String key) {
+ return getAuth(key, null);
+ }
+
static AuthenticationInfo getServerAuth(String key) {
- AuthenticationInfo cached = getAuth(key, null);
- if ((cached == null) && requestIsInProgress (key)) {
- /* check the cache again, it might contain an entry */
- cached = getAuth(key, null);
- }
- return cached;
+ if (!serializeAuth) return getCachedServerAuth(key);
+ return requestAuthentication(key, AuthenticationInfo::getCachedServerAuth);
}
@@ -367,13 +385,13 @@
return key;
}
+ private static AuthenticationInfo getCachedProxyAuth(String key) {
+ return (AuthenticationInfo) cache.get(key, null);
+ }
+
static AuthenticationInfo getProxyAuth(String key) {
- AuthenticationInfo cached = (AuthenticationInfo) cache.get(key, null);
- if ((cached == null) && requestIsInProgress (key)) {
- /* check the cache again, it might contain an entry */
- cached = (AuthenticationInfo) cache.get(key, null);
- }
- return cached;
+ if (!serializeAuth) return getCachedProxyAuth(key);
+ return requestAuthentication(key, AuthenticationInfo::getCachedProxyAuth);
}
--- a/test/jdk/java/net/Authenticator/B4769350.java Mon Aug 19 06:13:52 2019 +0200
+++ b/test/jdk/java/net/Authenticator/B4769350.java Mon Aug 19 11:14:50 2019 +0100
@@ -23,7 +23,7 @@
/**
* @test
- * @bug 4769350 8017779
+ * @bug 4769350 8017779 8191169
* @modules jdk.httpserver
* @run main/othervm B4769350 server
* @run main/othervm B4769350 proxy
--- a/test/jdk/java/net/HttpURLConnection/SetAuthenticator/HTTPSetAuthenticatorTest.java Mon Aug 19 06:13:52 2019 +0200
+++ b/test/jdk/java/net/HttpURLConnection/SetAuthenticator/HTTPSetAuthenticatorTest.java Mon Aug 19 11:14:50 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 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
@@ -115,8 +115,8 @@
HttpAuthType mode)
throws IOException
{
- HttpTestAuthenticator authOne = new HttpTestAuthenticator("dublin", "foox");
- HttpTestAuthenticator authTwo = new HttpTestAuthenticator("dublin", "foox");
+ HttpTestAuthenticator authOne = new HttpTestAuthenticator("authOne", "dublin", "foox");
+ HttpTestAuthenticator authTwo = new HttpTestAuthenticator("authTwo", "dublin", "foox");
int expectedIncrement = scheme == HttpSchemeType.NONE
? 0 : EXPECTED_AUTH_CALLS_PER_TEST;
int count;
--- a/test/jdk/java/net/HttpURLConnection/SetAuthenticator/HTTPTest.java Mon Aug 19 06:13:52 2019 +0200
+++ b/test/jdk/java/net/HttpURLConnection/SetAuthenticator/HTTPTest.java Mon Aug 19 11:14:50 2019 +0100
@@ -89,8 +89,10 @@
// count will be incremented every time getPasswordAuthentication()
// is called from the client side.
final AtomicInteger count = new AtomicInteger();
+ private final String name;
- public HttpTestAuthenticator(String realm, String username) {
+ public HttpTestAuthenticator(String name, String realm, String username) {
+ this.name = name;
this.realm = realm;
this.username = username;
}
@@ -98,7 +100,7 @@
@Override
protected PasswordAuthentication getPasswordAuthentication() {
if (skipCount.get() == null || skipCount.get().booleanValue() == false) {
- System.out.println("Authenticator called: " + count.incrementAndGet());
+ System.out.println("Authenticator " + name + " called: " + count.incrementAndGet());
}
return new PasswordAuthentication(getUserName(),
new char[] {'b','a','r'});
@@ -118,6 +120,11 @@
throw new SecurityException("User unknown: " + user);
}
+ @Override
+ public String toString() {
+ return super.toString() + "[name=\"" + name + "\"]";
+ }
+
public final String getUserName() {
return username;
}
@@ -128,7 +135,7 @@
}
public static final HttpTestAuthenticator AUTHENTICATOR;
static {
- AUTHENTICATOR = new HttpTestAuthenticator("dublin", "foox");
+ AUTHENTICATOR = new HttpTestAuthenticator("AUTHENTICATOR","dublin", "foox");
Authenticator.setDefault(AUTHENTICATOR);
}
--- a/test/jdk/java/net/HttpURLConnection/SetAuthenticator/HTTPTestServer.java Mon Aug 19 06:13:52 2019 +0200
+++ b/test/jdk/java/net/HttpURLConnection/SetAuthenticator/HTTPTestServer.java Mon Aug 19 11:14:50 2019 +0100
@@ -165,7 +165,7 @@
for (int i = 1; i <= max; i++) {
B bindable = createBindable();
SocketAddress address = getAddress(bindable);
- String key = address.toString();
+ String key = toString(address);
if (addresses.addIfAbsent(key)) {
System.out.println("Socket bound to: " + key
+ " after " + i + " attempt(s)");
@@ -188,6 +188,16 @@
+ "addresses used before: " + addresses);
}
+ private static String toString(SocketAddress address) {
+ // We don't rely on address.toString(): sometimes it can be
+ // "/127.0.0.1:port", sometimes it can be "localhost/127.0.0.1:port"
+ // Instead we compose our own string representation:
+ InetSocketAddress candidate = (InetSocketAddress) address;
+ String hostAddr = candidate.getAddress().getHostAddress();
+ if (hostAddr.contains(":")) hostAddr = "[" + hostAddr + "]";
+ return hostAddr + ":" + candidate.getPort();
+ }
+
protected abstract B createBindable() throws IOException;
protected abstract SocketAddress getAddress(B bindable);