--- a/src/java.base/share/classes/sun/net/www/MessageHeader.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/share/classes/sun/net/www/MessageHeader.java Fri Aug 30 15:42:27 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1995, 2013, 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
@@ -311,12 +311,23 @@
return (k.substring(i+1, len-3).equalsIgnoreCase("HTTP/"));
}
+ /** Prints the key-value pairs represented by this
+ header. Also prints the RFC required blank line
+ at the end. Omits pairs with a null key. Omits
+ colon if key-value pair is the requestline. */
+ public void print(PrintStream p) {
+ // no synchronization: use cloned arrays instead.
+ String[] k; String[] v; int n;
+ synchronized (this) { n = nkeys; k = keys.clone(); v = values.clone(); }
+ print(n, k, v, p);
+ }
+
/** Prints the key-value pairs represented by this
header. Also prints the RFC required blank line
at the end. Omits pairs with a null key. Omits
colon if key-value pair is the requestline. */
- public synchronized void print(PrintStream p) {
+ private void print(int nkeys, String[] keys, String[] values, PrintStream p) {
for (int i = 0; i < nkeys; i++)
if (keys[i] != null) {
StringBuilder sb = new StringBuilder(keys[i]);
--- a/src/java.base/share/classes/sun/net/www/MeteredStream.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/share/classes/sun/net/www/MeteredStream.java Fri Aug 30 15:42:27 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1994, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1994, 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
@@ -25,9 +25,8 @@
package sun.net.www;
-import java.net.URL;
-import java.util.*;
import java.io.*;
+import java.util.concurrent.locks.ReentrantLock;
import sun.net.ProgressSource;
import sun.net.www.http.ChunkedInputStream;
@@ -44,6 +43,7 @@
protected long markedCount = 0;
protected int markLimit = -1;
protected ProgressSource pi;
+ protected final ReentrantLock readLock = new ReentrantLock();
public MeteredStream(InputStream is, ProgressSource pi, long expected)
{
@@ -57,7 +57,9 @@
}
}
- private final void justRead(long n) throws IOException {
+ private final void justRead(long n) throws IOException {
+ assert readLock.isHeldByCurrentThread();
+
if (n == -1) {
/*
@@ -99,6 +101,7 @@
* Returns true if the mark is valid, false otherwise
*/
private boolean isMarked() {
+ assert readLock.isHeldByCurrentThread();
if (markLimit < 0) {
return false;
@@ -113,94 +116,128 @@
return true;
}
- public synchronized int read() throws java.io.IOException {
- if (closed) {
- return -1;
+ public int read() throws java.io.IOException {
+ if (closed) return -1;
+ readLock.lock();
+ try {
+ if (closed) return -1;
+ int c = in.read();
+ if (c != -1) {
+ justRead(1);
+ } else {
+ justRead(c);
+ }
+ return c;
+ } finally {
+ readLock.unlock();
}
- int c = in.read();
- if (c != -1) {
- justRead(1);
- } else {
- justRead(c);
- }
- return c;
}
- public synchronized int read(byte b[], int off, int len)
+ public int read(byte b[], int off, int len)
throws java.io.IOException {
- if (closed) {
- return -1;
+ if (closed) return -1;
+ readLock.lock();
+ try {
+ if (closed) return -1;
+
+ int n = in.read(b, off, len);
+ justRead(n);
+ return n;
+ } finally {
+ readLock.unlock();
}
- int n = in.read(b, off, len);
- justRead(n);
- return n;
}
- public synchronized long skip(long n) throws IOException {
+ public long skip(long n) throws IOException {
- // REMIND: what does skip do on EOF????
- if (closed) {
- return 0;
- }
+ if (closed) return 0;
+ readLock.lock();
+ try {
+ // REMIND: what does skip do on EOF????
+ if (closed) return 0;
- if (in instanceof ChunkedInputStream) {
- n = in.skip(n);
+ if (in instanceof ChunkedInputStream) {
+ n = in.skip(n);
+ } else {
+ // just skip min(n, num_bytes_left)
+ long min = (n > expected - count) ? expected - count : n;
+ n = in.skip(min);
+ }
+ justRead(n);
+ return n;
+ } finally {
+ readLock.unlock();
}
- else {
- // just skip min(n, num_bytes_left)
- long min = (n > expected - count) ? expected - count: n;
- n = in.skip(min);
- }
- justRead(n);
- return n;
}
public void close() throws IOException {
- if (closed) {
- return;
+ if (closed) return;
+ readLock.lock();
+ try {
+ if (closed) return;
+ if (pi != null)
+ pi.finishTracking();
+
+ closed = true;
+ in.close();
+ } finally {
+ readLock.unlock();
}
- if (pi != null)
- pi.finishTracking();
-
- closed = true;
- in.close();
}
- public synchronized int available() throws IOException {
- return closed ? 0: in.available();
+ public int available() throws IOException {
+ if (closed) return 0;
+ readLock.lock();
+ try {
+ return closed ? 0 : in.available();
+ } finally {
+ readLock.unlock();
+ }
}
- public synchronized void mark(int readLimit) {
- if (closed) {
- return;
- }
- super.mark(readLimit);
+ public void mark(int readLimit) {
+ if (closed) return;
+ readLock.lock();
+ try {
+ if (closed) return;
+ super.mark(readLimit);
- /*
- * mark the count to restore upon reset
- */
- markedCount = count;
- markLimit = readLimit;
+ /*
+ * mark the count to restore upon reset
+ */
+ markedCount = count;
+ markLimit = readLimit;
+ } finally {
+ readLock.unlock();
+ }
}
- public synchronized void reset() throws IOException {
- if (closed) {
- return;
- }
+ public void reset() throws IOException {
+ if (closed) return;
- if (!isMarked()) {
- throw new IOException ("Resetting to an invalid mark");
+ readLock.lock();
+ try {
+ if (closed) return;
+ if (!isMarked()) {
+ throw new IOException("Resetting to an invalid mark");
+ }
+
+ count = markedCount;
+ super.reset();
+ } finally {
+ readLock.unlock();
}
-
- count = markedCount;
- super.reset();
}
public boolean markSupported() {
- if (closed) {
- return false;
+ if (closed) return false;
+ readLock.lock();
+ try {
+ if (closed) return false;
+ return super.markSupported();
+ } finally {
+ readLock.unlock();
}
- return super.markSupported();
}
@SuppressWarnings("deprecation")
--- a/src/java.base/share/classes/sun/net/www/http/ChunkedInputStream.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/share/classes/sun/net/www/http/ChunkedInputStream.java Fri Aug 30 15:42:27 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1999, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 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
@@ -25,9 +25,7 @@
package sun.net.www.http;
import java.io.*;
-import java.util.*;
-
-import sun.net.*;
+import java.util.concurrent.locks.ReentrantLock;
import sun.net.www.*;
/**
@@ -40,8 +38,7 @@
* can be hurried to the end of the stream if the bytes are available on
* the underlying stream.
*/
-public
-class ChunkedInputStream extends InputStream implements Hurryable {
+public class ChunkedInputStream extends InputStream implements Hurryable {
/**
* The underlying stream
@@ -125,11 +122,14 @@
*/
private boolean closed;
+ final ReentrantLock readLock = new ReentrantLock();
+
/*
* Maximum chunk header size of 2KB + 2 bytes for CRLF
*/
private static final int MAX_CHUNK_HEADER_SIZE = 2050;
+
/**
* State to indicate that next field should be :-
* chunk-size [ chunk-extension ] CRLF
@@ -645,14 +645,19 @@
* @exception IOException if an I/O error occurs.
* @see java.io.FilterInputStream#in
*/
- public synchronized int read() throws IOException {
- ensureOpen();
- if (chunkPos >= chunkCount) {
- if (readAhead(true) <= 0) {
- return -1;
+ public int read() throws IOException {
+ readLock.lock();
+ try {
+ ensureOpen();
+ if (chunkPos >= chunkCount) {
+ if (readAhead(true) <= 0) {
+ return -1;
+ }
}
+ return chunkData[chunkPos++] & 0xff;
+ } finally {
+ readLock.unlock();
}
- return chunkData[chunkPos++] & 0xff;
}
@@ -667,42 +672,47 @@
* the stream has been reached.
* @exception IOException if an I/O error occurs.
*/
- public synchronized int read(byte b[], int off, int len)
+ public int read(byte b[], int off, int len)
throws IOException
{
- ensureOpen();
- if ((off < 0) || (off > b.length) || (len < 0) ||
- ((off + len) > b.length) || ((off + len) < 0)) {
- throw new IndexOutOfBoundsException();
- } else if (len == 0) {
- return 0;
- }
-
- int avail = chunkCount - chunkPos;
- if (avail <= 0) {
- /*
- * Optimization: if we're in the middle of the chunk read
- * directly from the underlying stream into the caller's
- * buffer
- */
- if (state == STATE_READING_CHUNK) {
- return fastRead( b, off, len );
+ readLock.lock();
+ try {
+ ensureOpen();
+ if ((off < 0) || (off > b.length) || (len < 0) ||
+ ((off + len) > b.length) || ((off + len) < 0)) {
+ throw new IndexOutOfBoundsException();
+ } else if (len == 0) {
+ return 0;
}
- /*
- * We're not in the middle of a chunk so we must read ahead
- * until there is some chunk data available.
- */
- avail = readAhead(true);
- if (avail < 0) {
- return -1; /* EOF */
+ int avail = chunkCount - chunkPos;
+ if (avail <= 0) {
+ /*
+ * Optimization: if we're in the middle of the chunk read
+ * directly from the underlying stream into the caller's
+ * buffer
+ */
+ if (state == STATE_READING_CHUNK) {
+ return fastRead(b, off, len);
+ }
+
+ /*
+ * We're not in the middle of a chunk so we must read ahead
+ * until there is some chunk data available.
+ */
+ avail = readAhead(true);
+ if (avail < 0) {
+ return -1; /* EOF */
+ }
}
+ int cnt = (avail < len) ? avail : len;
+ System.arraycopy(chunkData, chunkPos, b, off, cnt);
+ chunkPos += cnt;
+
+ return cnt;
+ } finally {
+ readLock.unlock();
}
- int cnt = (avail < len) ? avail : len;
- System.arraycopy(chunkData, chunkPos, b, off, cnt);
- chunkPos += cnt;
-
- return cnt;
}
/**
@@ -714,20 +724,25 @@
* @exception IOException if an I/O error occurs.
* @see java.io.FilterInputStream#in
*/
- public synchronized int available() throws IOException {
- ensureOpen();
+ public int available() throws IOException {
+ readLock.lock();
+ try {
+ ensureOpen();
+
+ int avail = chunkCount - chunkPos;
+ if (avail > 0) {
+ return avail;
+ }
- int avail = chunkCount - chunkPos;
- if(avail > 0) {
- return avail;
- }
+ avail = readAhead(false);
- avail = readAhead(false);
-
- if (avail < 0) {
- return 0;
- } else {
- return avail;
+ if (avail < 0) {
+ return 0;
+ } else {
+ return avail;
+ }
+ } finally {
+ readLock.unlock();
}
}
@@ -742,12 +757,18 @@
*
* @exception IOException if an I/O error occurs.
*/
- public synchronized void close() throws IOException {
- if (closed) {
- return;
+ public void close() throws IOException {
+ if (closed) return;
+ readLock.lock();
+ try {
+ if (closed) {
+ return;
+ }
+ closeUnderlying();
+ closed = true;
+ } finally {
+ readLock.unlock();
}
- closeUnderlying();
- closed = true;
}
/**
@@ -759,22 +780,27 @@
* without blocking then this stream can't be hurried and should be
* closed.
*/
- public synchronized boolean hurry() {
- if (in == null || error) {
- return false;
- }
-
+ public boolean hurry() {
+ readLock.lock();
try {
- readAhead(false);
- } catch (Exception e) {
- return false;
- }
+ if (in == null || error) {
+ return false;
+ }
- if (error) {
- return false;
+ try {
+ readAhead(false);
+ } catch (Exception e) {
+ return false;
+ }
+
+ if (error) {
+ return false;
+ }
+
+ return (state == STATE_DONE);
+ } finally {
+ readLock.unlock();
}
-
- return (state == STATE_DONE);
}
}
--- a/src/java.base/share/classes/sun/net/www/http/ChunkedOutputStream.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/share/classes/sun/net/www/http/ChunkedOutputStream.java Fri Aug 30 15:42:27 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2004, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2004, 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
@@ -25,6 +25,8 @@
package sun.net.www.http;
import java.io.*;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
/**
* OutputStream that sends the output to the underlying stream using chunked
@@ -61,6 +63,8 @@
/* header for a complete Chunk */
private byte[] completeHeader;
+ private final Lock writeLock = new ReentrantLock();
+
/* return the size of the header for a particular chunk size */
private static int getHeaderSize(int size) {
return (Integer.toHexString(size)).length() + CRLF_SIZE;
@@ -197,77 +201,92 @@
* The size of the data is of course smaller than preferredChunkSize.
*/
@Override
- public synchronized void write(byte b[], int off, int len) {
- ensureOpen();
- if ((off < 0) || (off > b.length) || (len < 0) ||
- ((off + len) > b.length) || ((off + len) < 0)) {
- throw new IndexOutOfBoundsException();
- } else if (len == 0) {
- return;
- }
-
- /* if b[] contains enough data then one loop cycle creates one complete
- * data chunk with a header, body and a footer, and then flushes the
- * chunk to the underlying stream. Otherwise, the last loop cycle
- * creates incomplete data chunk with empty header and with no footer
- * and stores this incomplete chunk in an internal buffer buf[]
- */
- int bytesToWrite = len;
- int inputIndex = off; /* the index of the byte[] currently being written */
-
- do {
- /* enough data to complete a chunk */
- if (bytesToWrite >= spaceInCurrentChunk) {
-
- /* header */
- for (int i=0; i<completeHeader.length; i++)
- buf[i] = completeHeader[i];
-
- /* data */
- System.arraycopy(b, inputIndex, buf, count, spaceInCurrentChunk);
- inputIndex += spaceInCurrentChunk;
- bytesToWrite -= spaceInCurrentChunk;
- count += spaceInCurrentChunk;
-
- /* footer */
- buf[count++] = FOOTER[0];
- buf[count++] = FOOTER[1];
- spaceInCurrentChunk = 0; //chunk is complete
-
- flush(false);
- if (checkError()){
- break;
- }
+ public void write(byte b[], int off, int len) {
+ writeLock.lock();
+ try {
+ ensureOpen();
+ if ((off < 0) || (off > b.length) || (len < 0) ||
+ ((off + len) > b.length) || ((off + len) < 0)) {
+ throw new IndexOutOfBoundsException();
+ } else if (len == 0) {
+ return;
}
- /* not enough data to build a chunk */
- else {
- /* header */
- /* do not write header if not enough bytes to build a chunk yet */
+ /* if b[] contains enough data then one loop cycle creates one complete
+ * data chunk with a header, body and a footer, and then flushes the
+ * chunk to the underlying stream. Otherwise, the last loop cycle
+ * creates incomplete data chunk with empty header and with no footer
+ * and stores this incomplete chunk in an internal buffer buf[]
+ */
+ int bytesToWrite = len;
+ int inputIndex = off; /* the index of the byte[] currently being written */
+
+ do {
+ /* enough data to complete a chunk */
+ if (bytesToWrite >= spaceInCurrentChunk) {
+
+ /* header */
+ for (int i = 0; i < completeHeader.length; i++)
+ buf[i] = completeHeader[i];
+
+ /* data */
+ System.arraycopy(b, inputIndex, buf, count, spaceInCurrentChunk);
+ inputIndex += spaceInCurrentChunk;
+ bytesToWrite -= spaceInCurrentChunk;
+ count += spaceInCurrentChunk;
- /* data */
- System.arraycopy(b, inputIndex, buf, count, bytesToWrite);
- count += bytesToWrite;
- size += bytesToWrite;
- spaceInCurrentChunk -= bytesToWrite;
- bytesToWrite = 0;
+ /* footer */
+ buf[count++] = FOOTER[0];
+ buf[count++] = FOOTER[1];
+ spaceInCurrentChunk = 0; //chunk is complete
+
+ flush(false);
+ if (checkError()) {
+ break;
+ }
+ }
- /* footer */
- /* do not write header if not enough bytes to build a chunk yet */
- }
- } while (bytesToWrite > 0);
+ /* not enough data to build a chunk */
+ else {
+ /* header */
+ /* do not write header if not enough bytes to build a chunk yet */
+
+ /* data */
+ System.arraycopy(b, inputIndex, buf, count, bytesToWrite);
+ count += bytesToWrite;
+ size += bytesToWrite;
+ spaceInCurrentChunk -= bytesToWrite;
+ bytesToWrite = 0;
+
+ /* footer */
+ /* do not write header if not enough bytes to build a chunk yet */
+ }
+ } while (bytesToWrite > 0);
+ } finally {
+ writeLock.unlock();
+ }
}
@Override
- public synchronized void write(int _b) {
- byte b[] = {(byte)_b};
- write(b, 0, 1);
+ public void write(int _b) {
+ writeLock.lock();
+ try {
+ byte b[] = {(byte) _b};
+ write(b, 0, 1);
+ } finally {
+ writeLock.unlock();
+ }
}
- public synchronized void reset() {
- count = preferedHeaderSize;
- size = 0;
- spaceInCurrentChunk = preferredChunkDataSize;
+ public void reset() {
+ writeLock.lock();
+ try {
+ count = preferedHeaderSize;
+ size = 0;
+ spaceInCurrentChunk = preferredChunkDataSize;
+ } finally {
+ writeLock.unlock();
+ }
}
public int size() {
@@ -275,26 +294,36 @@
}
@Override
- public synchronized void close() {
- ensureOpen();
+ public void close() {
+ writeLock.lock();
+ try {
+ ensureOpen();
- /* if we have buffer a chunked send it */
- if (size > 0) {
+ /* if we have buffer a chunked send it */
+ if (size > 0) {
+ flush(true);
+ }
+
+ /* send a zero length chunk */
flush(true);
+
+ /* don't close the underlying stream */
+ out = null;
+ } finally {
+ writeLock.unlock();
}
-
- /* send a zero length chunk */
- flush(true);
-
- /* don't close the underlying stream */
- out = null;
}
@Override
- public synchronized void flush() {
- ensureOpen();
- if (size > 0) {
- flush(true);
+ public void flush() {
+ writeLock.lock();
+ try {
+ ensureOpen();
+ if (size > 0) {
+ flush(true);
+ }
+ } finally {
+ writeLock.unlock();
}
}
}
--- a/src/java.base/share/classes/sun/net/www/http/HttpCapture.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/share/classes/sun/net/www/http/HttpCapture.java Fri Aug 30 15:42:27 2019 +0100
@@ -53,6 +53,7 @@
*
* @author jccollet
*/
+// Using synchronized should be safe here.
public class HttpCapture {
private File file;
private boolean incoming = true;
--- a/src/java.base/share/classes/sun/net/www/http/HttpClient.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/share/classes/sun/net/www/http/HttpClient.java Fri Aug 30 15:42:27 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1994, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1994, 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
@@ -30,6 +30,9 @@
import java.util.Locale;
import java.util.Objects;
import java.util.Properties;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
+
import sun.net.NetworkClient;
import sun.net.ProgressSource;
import sun.net.www.MessageHeader;
@@ -47,6 +50,8 @@
* @author Dave Brown
*/
public class HttpClient extends NetworkClient {
+ protected final ReentrantLock clientLock = new ReentrantLock();
+
// whether this httpclient comes from the cache
protected boolean cachedHttpClient = false;
@@ -315,23 +320,30 @@
: httpuc.getAuthenticatorKey();
boolean compatible = Objects.equals(ret.proxy, p)
&& Objects.equals(ret.getAuthenticatorKey(), ak);
+ final Lock lock = ret.clientLock;
if (compatible) {
- synchronized (ret) {
+ lock.lock();
+ try {
ret.cachedHttpClient = true;
assert ret.inCache;
ret.inCache = false;
if (httpuc != null && ret.needsTunneling())
httpuc.setTunnelState(TUNNELING);
logFinest("KeepAlive stream retrieved from the cache, " + ret);
+ } finally {
+ lock.unlock();
}
} else {
// We cannot return this connection to the cache as it's
// KeepAliveTimeout will get reset. We simply close the connection.
// This should be fine as it is very rare that a connection
// to the same host will not use the same proxy.
- synchronized(ret) {
+ lock.lock();
+ try {
ret.inCache = false;
ret.closeServer();
+ } finally {
+ lock.unlock();
}
ret = null;
}
@@ -409,10 +421,11 @@
}
}
- protected synchronized boolean available() {
+ protected boolean available() {
boolean available = true;
int old = -1;
+ clientLock.lock();
try {
try {
old = serverSocket.getSoTimeout();
@@ -436,21 +449,33 @@
logFinest("HttpClient.available(): " +
"SocketException: not available");
available = false;
+ } finally {
+ clientLock.unlock();
}
return available;
}
- protected synchronized void putInKeepAliveCache() {
- if (inCache) {
- assert false : "Duplicate put to keep alive cache";
- return;
+ protected void putInKeepAliveCache() {
+ clientLock.lock();
+ try {
+ if (inCache) {
+ assert false : "Duplicate put to keep alive cache";
+ return;
+ }
+ inCache = true;
+ kac.put(url, null, this);
+ } finally {
+ clientLock.unlock();
}
- inCache = true;
- kac.put(url, null, this);
}
- protected synchronized boolean isInKeepAliveCache() {
- return inCache;
+ protected boolean isInKeepAliveCache() {
+ clientLock.lock();
+ try {
+ return inCache;
+ } finally {
+ clientLock.unlock();
+ }
}
/*
@@ -497,8 +522,13 @@
/*
* Returns true if this httpclient is from cache
*/
- public synchronized boolean isCachedConnection() {
- return cachedHttpClient;
+ public boolean isCachedConnection() {
+ clientLock.lock();
+ try {
+ return cachedHttpClient;
+ } finally {
+ clientLock.unlock();
+ }
}
/*
@@ -516,9 +546,10 @@
/*
* call openServer in a privileged block
*/
- private synchronized void privilegedOpenServer(final InetSocketAddress server)
+ private void privilegedOpenServer(final InetSocketAddress server)
throws IOException
{
+ assert clientLock.isHeldByCurrentThread();
try {
java.security.AccessController.doPrivileged(
new java.security.PrivilegedExceptionAction<>() {
@@ -544,48 +575,53 @@
/*
*/
- protected synchronized void openServer() throws IOException {
+ protected void openServer() throws IOException {
SecurityManager security = System.getSecurityManager();
- if (security != null) {
- security.checkConnect(host, port);
- }
-
- if (keepingAlive) { // already opened
- return;
- }
+ clientLock.lock();
+ try {
+ if (security != null) {
+ security.checkConnect(host, port);
+ }
- if (url.getProtocol().equals("http") ||
- url.getProtocol().equals("https") ) {
-
- if ((proxy != null) && (proxy.type() == Proxy.Type.HTTP)) {
- sun.net.www.URLConnection.setProxiedHost(host);
- privilegedOpenServer((InetSocketAddress) proxy.address());
- usingProxy = true;
- return;
- } else {
- // make direct connection
- openServer(host, port);
- usingProxy = false;
+ if (keepingAlive) { // already opened
return;
}
- } else {
- /* we're opening some other kind of url, most likely an
- * ftp url.
- */
- if ((proxy != null) && (proxy.type() == Proxy.Type.HTTP)) {
- sun.net.www.URLConnection.setProxiedHost(host);
- privilegedOpenServer((InetSocketAddress) proxy.address());
- usingProxy = true;
- return;
+ if (url.getProtocol().equals("http") ||
+ url.getProtocol().equals("https")) {
+
+ if ((proxy != null) && (proxy.type() == Proxy.Type.HTTP)) {
+ sun.net.www.URLConnection.setProxiedHost(host);
+ privilegedOpenServer((InetSocketAddress) proxy.address());
+ usingProxy = true;
+ return;
+ } else {
+ // make direct connection
+ openServer(host, port);
+ usingProxy = false;
+ return;
+ }
+
} else {
- // make direct connection
- super.openServer(host, port);
- usingProxy = false;
- return;
+ /* we're opening some other kind of url, most likely an
+ * ftp url.
+ */
+ if ((proxy != null) && (proxy.type() == Proxy.Type.HTTP)) {
+ sun.net.www.URLConnection.setProxiedHost(host);
+ privilegedOpenServer((InetSocketAddress) proxy.address());
+ usingProxy = true;
+ return;
+ } else {
+ // make direct connection
+ super.openServer(host, port);
+ usingProxy = false;
+ return;
+ }
}
+ } finally {
+ clientLock.unlock();
}
}
@@ -1006,8 +1042,13 @@
return ret;
}
- public synchronized InputStream getInputStream() {
- return serverInput;
+ public InputStream getInputStream() {
+ clientLock.lock();
+ try {
+ return serverInput;
+ } finally {
+ clientLock.unlock();
+ }
}
public OutputStream getOutputStream() {
--- a/src/java.base/share/classes/sun/net/www/http/KeepAliveCache.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/share/classes/sun/net/www/http/KeepAliveCache.java Fri Aug 30 15:42:27 2019 +0100
@@ -36,6 +36,8 @@
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
import jdk.internal.misc.InnocuousThread;
import sun.security.action.GetIntegerAction;
@@ -74,6 +76,7 @@
static final int LIFETIME = 5000;
+ private final ReentrantLock cacheLock = new ReentrantLock();
private Thread keepAliveTimer = null;
/**
@@ -86,76 +89,84 @@
* @param url The URL contains info about the host and port
* @param http The HttpClient to be cached
*/
- public synchronized void put(final URL url, Object obj, HttpClient http) {
- boolean startThread = (keepAliveTimer == null);
- if (!startThread) {
- if (!keepAliveTimer.isAlive()) {
- startThread = true;
+ public void put(final URL url, Object obj, HttpClient http) {
+ cacheLock.lock();
+ try {
+ boolean startThread = (keepAliveTimer == null);
+ if (!startThread) {
+ if (!keepAliveTimer.isAlive()) {
+ startThread = true;
+ }
}
- }
- if (startThread) {
- clear();
- /* Unfortunately, we can't always believe the keep-alive timeout we got
- * back from the server. If I'm connected through a Netscape proxy
- * to a server that sent me a keep-alive
- * time of 15 sec, the proxy unilaterally terminates my connection
- * The robustness to get around this is in HttpClient.parseHTTP()
- */
- final KeepAliveCache cache = this;
- AccessController.doPrivileged(new PrivilegedAction<>() {
- public Void run() {
- keepAliveTimer = InnocuousThread.newSystemThread("Keep-Alive-Timer", cache);
- keepAliveTimer.setDaemon(true);
- keepAliveTimer.setPriority(Thread.MAX_PRIORITY - 2);
- keepAliveTimer.start();
- return null;
- }
- });
- }
+ if (startThread) {
+ clear();
+ /* Unfortunately, we can't always believe the keep-alive timeout we got
+ * back from the server. If I'm connected through a Netscape proxy
+ * to a server that sent me a keep-alive
+ * time of 15 sec, the proxy unilaterally terminates my connection
+ * The robustness to get around this is in HttpClient.parseHTTP()
+ */
+ final KeepAliveCache cache = this;
+ AccessController.doPrivileged(new PrivilegedAction<>() {
+ public Void run() {
+ keepAliveTimer = InnocuousThread.newSystemThread("Keep-Alive-Timer", cache);
+ keepAliveTimer.setDaemon(true);
+ keepAliveTimer.setPriority(Thread.MAX_PRIORITY - 2);
+ keepAliveTimer.start();
+ return null;
+ }
+ });
+ }
- KeepAliveKey key = new KeepAliveKey(url, obj);
- ClientVector v = super.get(key);
+ KeepAliveKey key = new KeepAliveKey(url, obj);
+ ClientVector v = super.get(key);
- if (v == null) {
- int keepAliveTimeout = http.getKeepAliveTimeout();
- v = new ClientVector(keepAliveTimeout > 0 ?
- keepAliveTimeout * 1000 : LIFETIME);
- v.put(http);
- super.put(key, v);
- } else {
- v.put(http);
+ if (v == null) {
+ int keepAliveTimeout = http.getKeepAliveTimeout();
+ v = new ClientVector(keepAliveTimeout > 0 ?
+ keepAliveTimeout * 1000 : LIFETIME);
+ v.put(http);
+ super.put(key, v);
+ } else {
+ v.put(http);
+ }
+ } finally {
+ cacheLock.unlock();
}
}
/* remove an obsolete HttpClient from its VectorCache */
- public synchronized void remove(HttpClient h, Object obj) {
- KeepAliveKey key = new KeepAliveKey(h.url, obj);
- ClientVector v = super.get(key);
- if (v != null) {
- v.remove(h);
- if (v.isEmpty()) {
- removeVector(key);
+ public void remove(HttpClient h, Object obj) {
+ cacheLock.lock();
+ try {
+ KeepAliveKey key = new KeepAliveKey(h.url, obj);
+ ClientVector v = super.get(key);
+ if (v != null) {
+ v.remove(h);
+ if (v.isEmpty()) {
+ super.remove(key);
+ }
}
+ } finally {
+ cacheLock.unlock();
}
}
- /* called by a clientVector thread when all its connections have timed out
- * and that vector of connections should be removed.
- */
- synchronized void removeVector(KeepAliveKey k) {
- super.remove(k);
- }
-
/**
* Check to see if this URL has a cached HttpClient
*/
- public synchronized HttpClient get(URL url, Object obj) {
- KeepAliveKey key = new KeepAliveKey(url, obj);
- ClientVector v = super.get(key);
- if (v == null) { // nothing in cache yet
- return null;
+ public HttpClient get(URL url, Object obj) {
+ cacheLock.lock();
+ try {
+ KeepAliveKey key = new KeepAliveKey(url, obj);
+ ClientVector v = super.get(key);
+ if (v == null) { // nothing in cache yet
+ return null;
+ }
+ return v.get();
+ } finally {
+ cacheLock.unlock();
}
- return v.get();
}
/* Sleeps for an alloted timeout, then checks for timed out connections.
@@ -170,13 +181,16 @@
} catch (InterruptedException e) {}
// Remove all outdated HttpClients.
- synchronized (this) {
+ cacheLock.lock();
+ try {
long currentTime = System.currentTimeMillis();
List<KeepAliveKey> keysToRemove = new ArrayList<>();
for (KeepAliveKey key : keySet()) {
ClientVector v = get(key);
- synchronized (v) {
+ final Lock lock = v.lock;
+ lock.lock();
+ try {
KeepAliveEntry e = v.peek();
while (e != null) {
if ((currentTime - e.idleStartTime) > v.nap) {
@@ -191,12 +205,16 @@
if (v.isEmpty()) {
keysToRemove.add(key);
}
+ } finally {
+ lock.unlock();
}
}
for (KeepAliveKey key : keysToRemove) {
- removeVector(key);
+ super.remove(key);
}
+ } finally {
+ cacheLock.unlock();
}
} while (!isEmpty());
}
@@ -223,6 +241,7 @@
class ClientVector extends ArrayDeque<KeepAliveEntry> {
@java.io.Serial
private static final long serialVersionUID = -8680532108106489459L;
+ final ReentrantLock lock = new ReentrantLock();
// sleep time in milliseconds, before cache clear
int nap;
@@ -231,42 +250,57 @@
this.nap = nap;
}
- synchronized HttpClient get() {
- if (isEmpty()) {
- return null;
- }
+ HttpClient get() {
+ lock.lock();
+ try {
+ if (isEmpty()) {
+ return null;
+ }
- // Loop until we find a connection that has not timed out
- HttpClient hc = null;
- long currentTime = System.currentTimeMillis();
- do {
- KeepAliveEntry e = pop();
- if ((currentTime - e.idleStartTime) > nap) {
- e.hc.closeServer();
- } else {
- hc = e.hc;
- }
- } while ((hc == null) && (!isEmpty()));
- return hc;
+ // Loop until we find a connection that has not timed out
+ HttpClient hc = null;
+ long currentTime = System.currentTimeMillis();
+ do {
+ KeepAliveEntry e = pop();
+ if ((currentTime - e.idleStartTime) > nap) {
+ e.hc.closeServer();
+ } else {
+ hc = e.hc;
+ }
+ } while ((hc == null) && (!isEmpty()));
+ return hc;
+ } finally {
+ lock.unlock();
+ }
}
/* return a still valid, unused HttpClient */
- synchronized void put(HttpClient h) {
- if (size() >= KeepAliveCache.getMaxConnections()) {
- h.closeServer(); // otherwise the connection remains in limbo
- } else {
- push(new KeepAliveEntry(h, System.currentTimeMillis()));
+ void put(HttpClient h) {
+ lock.lock();
+ try {
+ if (size() >= KeepAliveCache.getMaxConnections()) {
+ h.closeServer(); // otherwise the connection remains in limbo
+ } else {
+ push(new KeepAliveEntry(h, System.currentTimeMillis()));
+ }
+ } finally {
+ lock.unlock();
}
}
/* remove an HttpClient */
- synchronized boolean remove(HttpClient h) {
- for (KeepAliveEntry curr : this) {
- if (curr.hc == h) {
- return super.remove(curr);
+ boolean remove(HttpClient h) {
+ lock.lock();
+ try {
+ for (KeepAliveEntry curr : this) {
+ if (curr.hc == h) {
+ return super.remove(curr);
+ }
}
+ return false;
+ } finally {
+ lock.unlock();
}
- return false;
}
/*
--- a/src/java.base/share/classes/sun/net/www/http/KeepAliveStream.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/share/classes/sun/net/www/http/KeepAliveStream.java Fri Aug 30 15:42:27 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1996, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1996, 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
@@ -26,7 +26,7 @@
package sun.net.www.http;
import java.io.*;
-
+import java.util.concurrent.locks.ReentrantLock;
import sun.net.ProgressSource;
import sun.net.www.MeteredStream;
import jdk.internal.misc.InnocuousThread;
@@ -47,7 +47,7 @@
boolean hurried;
// has this KeepAliveStream been put on the queue for asynchronous cleanup.
- protected boolean queuedForCleanup = false;
+ protected volatile boolean queuedForCleanup = false;
private static final KeepAliveStreamCleaner queue = new KeepAliveStreamCleaner();
private static Thread cleanerThread; // null
@@ -60,19 +60,17 @@
this.hc = hc;
}
+ final ReentrantLock readLock() {
+ return readLock;
+ }
+
/**
* Attempt to cache this connection
*/
public void close() throws IOException {
- // If the inputstream is closed already, just return.
- if (closed) {
- return;
- }
-
- // If this stream has already been queued for cleanup.
- if (queuedForCleanup) {
- return;
- }
+ // If the inputstream is closed already, or if this stream
+ // has already been queued for cleanup.just return.
+ if (closed || queuedForCleanup) return;
// Skip past the data that's left in the Inputstream because
// some sort of error may have occurred.
@@ -81,34 +79,43 @@
// to hang around for nothing. So if we can't skip without blocking
// we just close the socket and, therefore, terminate the keepAlive
// NOTE: Don't close super class
+ // For consistency, access to `expected` and `count` should be
+ // protected by readLock
+ readLock.lock();
try {
- if (expected > count) {
- long nskip = expected - count;
- if (nskip <= available()) {
- do {} while ((nskip = (expected - count)) > 0L
- && skip(Math.min(nskip, available())) > 0L);
- } else if (expected <= KeepAliveStreamCleaner.MAX_DATA_REMAINING && !hurried) {
- //put this KeepAliveStream on the queue so that the data remaining
- //on the socket can be cleanup asyncronously.
- queueForCleanup(new KeepAliveCleanerEntry(this, hc));
- } else {
- hc.closeServer();
+ if (closed || queuedForCleanup) return;
+ try {
+ if (expected > count) {
+ long nskip = expected - count;
+ if (nskip <= available()) {
+ do {
+ } while ((nskip = (expected - count)) > 0L
+ && skip(Math.min(nskip, available())) > 0L);
+ } else if (expected <= KeepAliveStreamCleaner.MAX_DATA_REMAINING && !hurried) {
+ //put this KeepAliveStream on the queue so that the data remaining
+ //on the socket can be cleanup asyncronously.
+ queueForCleanup(new KeepAliveCleanerEntry(this, hc));
+ } else {
+ hc.closeServer();
+ }
+ }
+ if (!closed && !hurried && !queuedForCleanup) {
+ hc.finished();
+ }
+ } finally {
+ if (pi != null)
+ pi.finishTracking();
+
+ if (!queuedForCleanup) {
+ // nulling out the underlying inputstream as well as
+ // httpClient to let gc collect the memories faster
+ in = null;
+ hc = null;
+ closed = true;
}
}
- if (!closed && !hurried && !queuedForCleanup) {
- hc.finished();
- }
} finally {
- if (pi != null)
- pi.finishTracking();
-
- if (!queuedForCleanup) {
- // nulling out the underlying inputstream as well as
- // httpClient to let gc collect the memories faster
- in = null;
- hc = null;
- closed = true;
- }
+ readLock.unlock();
}
}
@@ -124,7 +131,8 @@
throw new IOException("mark/reset not supported");
}
- public synchronized boolean hurry() {
+ public boolean hurry() {
+ readLock.lock();
try {
/* CASE 0: we're actually already done */
if (closed || count >= expected) {
@@ -147,11 +155,14 @@
} catch (IOException e) {
// e.printStackTrace();
return false;
+ } finally {
+ readLock.unlock();
}
}
private static void queueForCleanup(KeepAliveCleanerEntry kace) {
- synchronized(queue) {
+ queue.queueLock.lock();
+ try {
if(!kace.getQueuedForCleanup()) {
if (!queue.offer(kace)) {
kace.getHttpClient().closeServer();
@@ -159,7 +170,7 @@
}
kace.setQueuedForCleanup();
- queue.notifyAll();
+ queue.waiter.signalAll();
}
boolean startCleanupThread = (cleanerThread == null);
@@ -181,14 +192,18 @@
}
});
}
- } // queue
+ } finally {
+ queue.queueLock.unlock();
+ }
}
protected long remainingToRead() {
+ assert readLock.isHeldByCurrentThread();
return expected - count;
}
protected void setClosed() {
+ assert readLock.isHeldByCurrentThread();
in = null;
hc = null;
closed = true;
--- a/src/java.base/share/classes/sun/net/www/http/KeepAliveStreamCleaner.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/share/classes/sun/net/www/http/KeepAliveStreamCleaner.java Fri Aug 30 15:42:27 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2005, 2008, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 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
@@ -30,6 +30,8 @@
import sun.net.NetProperties;
import java.security.AccessController;
import java.security.PrivilegedAction;
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.ReentrantLock;
/**
* This class is used to cleanup any remaining data that may be on a KeepAliveStream
@@ -78,6 +80,8 @@
}
+ final ReentrantLock queueLock = new ReentrantLock();
+ final Condition waiter = queueLock.newCondition();
@Override
public boolean offer(KeepAliveCleanerEntry e) {
@@ -94,11 +98,12 @@
do {
try {
- synchronized(this) {
+ queueLock.lock();
+ try {
long before = System.currentTimeMillis();
long timeout = TIMEOUT;
while ((kace = poll()) == null) {
- this.wait(timeout);
+ waiter.wait(timeout);
long after = System.currentTimeMillis();
long elapsed = after - before;
@@ -110,6 +115,8 @@
before = after;
timeout -= elapsed;
}
+ } finally {
+ queueLock.unlock();
}
if(kace == null)
@@ -118,7 +125,9 @@
KeepAliveStream kas = kace.getKeepAliveStream();
if (kas != null) {
- synchronized(kas) {
+ final ReentrantLock readLock = kas.readLock();
+ readLock.lock();
+ try {
HttpClient hc = kace.getHttpClient();
try {
if (hc != null && !hc.isInKeepAliveCache()) {
@@ -147,6 +156,8 @@
} finally {
kas.setClosed();
}
+ } finally {
+ readLock.unlock();
}
}
} catch (InterruptedException ie) { }
--- a/src/java.base/share/classes/sun/net/www/protocol/http/AuthCacheImpl.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/share/classes/sun/net/www/protocol/http/AuthCacheImpl.java Fri Aug 30 15:42:27 2019 +0100
@@ -32,7 +32,7 @@
/**
* @author Michael McMahon
*/
-
+// Using synchronized in this class should be safe
public class AuthCacheImpl implements AuthCache {
HashMap<String,LinkedList<AuthCacheValue>> hashtable;
--- a/src/java.base/share/classes/sun/net/www/protocol/http/AuthenticationInfo.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/share/classes/sun/net/www/protocol/http/AuthenticationInfo.java Fri Aug 30 15:42:27 2019 +0100
@@ -31,6 +31,8 @@
import java.net.URL;
import java.util.HashMap;
import java.util.Objects;
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Function;
import sun.net.www.HeaderParser;
@@ -126,7 +128,8 @@
* the first completes its authentication.
*/
private static HashMap<String,Thread> requests = new HashMap<>();
-
+ private static final ReentrantLock requestLock = new ReentrantLock();
+ private static final Condition requestFinished = requestLock.newCondition();
/*
* check if AuthenticationInfo is available in the cache.
* If not, check if a request for this destination is in progress
@@ -142,8 +145,9 @@
// and we can revert to concurrent requests
return cached;
}
- synchronized (requests) {
- // check again after synchronizing, and if available
+ requestLock.lock();
+ try {
+ // check again after locking, and if available
// just return the cached value.
cached = cache.apply(key);
if (cached != null) return cached;
@@ -164,10 +168,10 @@
// Otherwise, an other thread is currently performing authentication:
// wait until it finishes.
while (requests.containsKey(key)) {
- try {
- requests.wait ();
- } catch (InterruptedException e) {}
+ requestFinished.awaitUninterruptibly();
}
+ } finally {
+ requestLock.unlock();
}
/* entry may be in cache now. */
return cache.apply(key);
@@ -177,13 +181,16 @@
* so that other threads can continue.
*/
private static void requestCompleted (String key) {
- synchronized (requests) {
+ requestLock.lock();
+ try {
Thread thread = requests.get(key);
if (thread != null && thread == Thread.currentThread()) {
boolean waspresent = requests.remove(key) != null;
assert waspresent;
}
- requests.notifyAll();
+ requestFinished.signalAll();
+ } finally {
+ requestLock.unlock();
}
}
@@ -414,9 +421,7 @@
if (!serializeAuth) {
return;
}
- synchronized (requests) {
- requestCompleted(key);
- }
+ requestCompleted(key);
}
/**
@@ -500,6 +505,7 @@
String s1, s2; /* used for serialization of pw */
@java.io.Serial
+ // should be safe to keep synchronized here
private synchronized void readObject(ObjectInputStream s)
throws IOException, ClassNotFoundException
{
@@ -512,6 +518,7 @@
}
@java.io.Serial
+ // should be safe to keep synchronized here
private synchronized void writeObject(java.io.ObjectOutputStream s)
throws IOException
{
--- a/src/java.base/share/classes/sun/net/www/protocol/http/DigestAuthentication.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/share/classes/sun/net/www/protocol/http/DigestAuthentication.java Fri Aug 30 15:42:27 2019 +0100
@@ -76,6 +76,7 @@
// One instance of these may be shared among several DigestAuthentication
// instances as a result of a single authorization (for multiple domains)
+ // synchronized should be safe here
static class Parameters implements java.io.Serializable {
private static final long serialVersionUID = -3584543755194526252L;
--- a/src/java.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java Fri Aug 30 15:42:27 2019 +0100
@@ -81,6 +81,8 @@
import java.nio.ByteBuffer;
import java.util.Objects;
import java.util.Properties;
+import java.util.concurrent.locks.ReentrantLock;
+
import static sun.net.www.protocol.http.AuthScheme.BASIC;
import static sun.net.www.protocol.http.AuthScheme.DIGEST;
import static sun.net.www.protocol.http.AuthScheme.NTLM;
@@ -97,7 +99,7 @@
public class HttpURLConnection extends java.net.HttpURLConnection {
- static String HTTP_CONNECT = "CONNECT";
+ static final String HTTP_CONNECT = "CONNECT";
static final String version;
public static final String userAgent;
@@ -352,7 +354,7 @@
* - getOutputStream()
* - getInputStream())
* - connect()
- * Access synchronized on this.
+ * Access synchronized on connectionLock.
*/
private boolean connecting = false;
@@ -384,6 +386,9 @@
/* Progress source */
protected ProgressSource pi;
+ /* Lock */
+ final ReentrantLock connectionLock = new ReentrantLock();
+
/* all the response headers we get back */
private MessageHeader responses;
/* the stream _from_ the server */
@@ -513,13 +518,18 @@
}
@Override
- public synchronized void setAuthenticator(Authenticator auth) {
- if (connecting || connected) {
- throw new IllegalStateException(
- "Authenticator must be set before connecting");
+ public void setAuthenticator(Authenticator auth) {
+ connectionLock.lock();
+ try {
+ if (connecting || connected) {
+ throw new IllegalStateException(
+ "Authenticator must be set before connecting");
+ }
+ authenticator = Objects.requireNonNull(auth);
+ authenticatorKey = AuthenticatorKeys.getKey(authenticator);
+ } finally {
+ connectionLock.unlock();
}
- authenticator = Objects.requireNonNull(auth);
- authenticatorKey = AuthenticatorKeys.getKey(authenticator);
}
public String getAuthenticatorKey() {
@@ -562,12 +572,17 @@
}
}
- public synchronized void setRequestMethod(String method)
+ public void setRequestMethod(String method)
throws ProtocolException {
- if (connecting) {
- throw new IllegalStateException("connect in progress");
+ connectionLock.lock();
+ try {
+ if (connecting) {
+ throw new IllegalStateException("connect in progress");
+ }
+ super.setRequestMethod(method);
+ } finally {
+ connectionLock.unlock();
}
- super.setRequestMethod(method);
}
/* adds the standard key/val pairs to reqests if necessary & write to
@@ -682,6 +697,8 @@
}
} else if (poster != null) {
/* add Content-Length & POST/PUT data */
+ // safe to synchronize on poster: this is
+ // a simple subclass of ByteArrayOutputStream
synchronized (poster) {
/* close it, so no more data can be added */
poster.close();
@@ -1009,8 +1026,11 @@
// overridden in HTTPS subclass
public void connect() throws IOException {
- synchronized (this) {
+ connectionLock.lock();
+ try {
connecting = true;
+ } finally {
+ connectionLock.unlock();
}
plainConnect();
}
@@ -1057,10 +1077,13 @@
}
protected void plainConnect() throws IOException {
- synchronized (this) {
+ connectionLock.lock();
+ try {
if (connected) {
return;
}
+ } finally {
+ connectionLock.unlock();
}
SocketPermission p = URLtoSocketPermission(this.url);
if (p != null) {
@@ -1323,28 +1346,34 @@
*/
@Override
- public synchronized OutputStream getOutputStream() throws IOException {
- connecting = true;
- SocketPermission p = URLtoSocketPermission(this.url);
-
- if (p != null) {
- try {
- return AccessController.doPrivilegedWithCombiner(
- new PrivilegedExceptionAction<>() {
- public OutputStream run() throws IOException {
- return getOutputStream0();
- }
- }, null, p
- );
- } catch (PrivilegedActionException e) {
- throw (IOException) e.getException();
+ public OutputStream getOutputStream() throws IOException {
+ connectionLock.lock();
+ try {
+ connecting = true;
+ SocketPermission p = URLtoSocketPermission(this.url);
+
+ if (p != null) {
+ try {
+ return AccessController.doPrivilegedWithCombiner(
+ new PrivilegedExceptionAction<>() {
+ public OutputStream run() throws IOException {
+ return getOutputStream0();
+ }
+ }, null, p
+ );
+ } catch (PrivilegedActionException e) {
+ throw (IOException) e.getException();
+ }
+ } else {
+ return getOutputStream0();
}
- } else {
- return getOutputStream0();
+ } finally {
+ connectionLock.unlock();
}
}
- private synchronized OutputStream getOutputStream0() throws IOException {
+ private OutputStream getOutputStream0() throws IOException {
+ assert connectionLock.isHeldByCurrentThread();
try {
if (!doOutput) {
throw new ProtocolException("cannot write to a URLConnection"
@@ -1434,17 +1463,19 @@
// we only want to capture the user defined Cookies once, as
// they cannot be changed by user code after we are connected,
// only internally.
- synchronized (this) {
- if (setUserCookies) {
- int k = requests.getKey("Cookie");
- if (k != -1)
- userCookies = requests.getValue(k);
- k = requests.getKey("Cookie2");
- if (k != -1)
- userCookies2 = requests.getValue(k);
- setUserCookies = false;
- }
- }
+ if (setUserCookies) {
+ // we should only reach here when called from
+ // writeRequest, which in turn is only called by
+ // getInputStream0
+ assert connectionLock.isHeldByCurrentThread();
+ int k = requests.getKey("Cookie");
+ if (k != -1)
+ userCookies = requests.getValue(k);
+ k = requests.getKey("Cookie2");
+ if (k != -1)
+ userCookies2 = requests.getValue(k);
+ setUserCookies = false;
+ }
// remove old Cookie header before setting new one.
requests.remove("Cookie");
@@ -1501,30 +1532,36 @@
}
@Override
- public synchronized InputStream getInputStream() throws IOException {
- connecting = true;
- SocketPermission p = URLtoSocketPermission(this.url);
-
- if (p != null) {
- try {
- return AccessController.doPrivilegedWithCombiner(
- new PrivilegedExceptionAction<>() {
- public InputStream run() throws IOException {
- return getInputStream0();
- }
- }, null, p
- );
- } catch (PrivilegedActionException e) {
- throw (IOException) e.getException();
+ public InputStream getInputStream() throws IOException {
+ connectionLock.lock();
+ try {
+ connecting = true;
+ SocketPermission p = URLtoSocketPermission(this.url);
+
+ if (p != null) {
+ try {
+ return AccessController.doPrivilegedWithCombiner(
+ new PrivilegedExceptionAction<>() {
+ public InputStream run() throws IOException {
+ return getInputStream0();
+ }
+ }, null, p
+ );
+ } catch (PrivilegedActionException e) {
+ throw (IOException) e.getException();
+ }
+ } else {
+ return getInputStream0();
}
- } else {
- return getInputStream0();
+ } finally {
+ connectionLock.unlock();
}
}
@SuppressWarnings("empty-statement")
- private synchronized InputStream getInputStream0() throws IOException {
-
+ private InputStream getInputStream0() throws IOException {
+
+ assert connectionLock.isHeldByCurrentThread();
if (!doInput) {
throw new ProtocolException("Cannot read from URLConnection"
+ " if doInput=false (call setDoInput(true))");
@@ -2053,7 +2090,16 @@
/**
* establish a tunnel through proxy server
*/
- public synchronized void doTunneling() throws IOException {
+ public void doTunneling() throws IOException {
+ connectionLock.lock();
+ try {
+ doTunneling0();
+ } finally{
+ connectionLock.unlock();
+ }
+ }
+
+ private void doTunneling0() throws IOException {
int retryTunnel = 0;
String statusLine = "";
int respCode = 0;
@@ -2423,7 +2469,7 @@
/**
* Gets the authentication for an HTTP server, and applies it to
* the connection.
- * @param authHdr the AuthenticationHeader which tells what auth scheme is
+ * @param authhdr the AuthenticationHeader which tells what auth scheme is
* preferred.
*/
@SuppressWarnings("fallthrough")
@@ -3166,17 +3212,22 @@
* @param value the value to be set
*/
@Override
- public synchronized void setRequestProperty(String key, String value) {
- if (connected || connecting)
- throw new IllegalStateException("Already connected");
- if (key == null)
- throw new NullPointerException ("key is null");
-
- if (isExternalMessageHeaderAllowed(key, value)) {
- requests.set(key, value);
- if (!key.equalsIgnoreCase("Content-Type")) {
- userHeaders.set(key, value);
+ public void setRequestProperty(String key, String value) {
+ connectionLock.lock();
+ try {
+ if (connected || connecting)
+ throw new IllegalStateException("Already connected");
+ if (key == null)
+ throw new NullPointerException("key is null");
+
+ if (isExternalMessageHeaderAllowed(key, value)) {
+ requests.set(key, value);
+ if (!key.equalsIgnoreCase("Content-Type")) {
+ userHeaders.set(key, value);
+ }
}
+ } finally {
+ connectionLock.unlock();
}
}
@@ -3192,21 +3243,26 @@
* @param key the keyword by which the request is known
* (e.g., "<code>accept</code>").
* @param value the value associated with it.
- * @see #getRequestProperties(java.lang.String)
+ * @see #getRequestProperty(java.lang.String)
* @since 1.4
*/
@Override
- public synchronized void addRequestProperty(String key, String value) {
- if (connected || connecting)
- throw new IllegalStateException("Already connected");
- if (key == null)
- throw new NullPointerException ("key is null");
-
- if (isExternalMessageHeaderAllowed(key, value)) {
- requests.add(key, value);
- if (!key.equalsIgnoreCase("Content-Type")) {
+ public void addRequestProperty(String key, String value) {
+ connectionLock.lock();
+ try {
+ if (connected || connecting)
+ throw new IllegalStateException("Already connected");
+ if (key == null)
+ throw new NullPointerException("key is null");
+
+ if (isExternalMessageHeaderAllowed(key, value)) {
+ requests.add(key, value);
+ if (!key.equalsIgnoreCase("Content-Type")) {
userHeaders.add(key, value);
+ }
}
+ } finally {
+ connectionLock.unlock();
}
}
@@ -3215,31 +3271,43 @@
// the connected test.
//
public void setAuthenticationProperty(String key, String value) {
- checkMessageHeader(key, value);
- requests.set(key, value);
+ // use lock here to avoid the need for external synchronization
+ // in AuthenticationInfo subclasses
+ connectionLock.lock();
+ try {
+ checkMessageHeader(key, value);
+ requests.set(key, value);
+ } finally {
+ connectionLock.unlock();
+ }
}
@Override
- public synchronized String getRequestProperty (String key) {
- if (key == null) {
- return null;
- }
-
- // don't return headers containing security sensitive information
- for (int i=0; i < EXCLUDE_HEADERS.length; i++) {
- if (key.equalsIgnoreCase(EXCLUDE_HEADERS[i])) {
+ public String getRequestProperty (String key) {
+ connectionLock.lock();
+ try {
+ if (key == null) {
return null;
}
- }
- if (!setUserCookies) {
- if (key.equalsIgnoreCase("Cookie")) {
- return userCookies;
+
+ // don't return headers containing security sensitive information
+ for (int i = 0; i < EXCLUDE_HEADERS.length; i++) {
+ if (key.equalsIgnoreCase(EXCLUDE_HEADERS[i])) {
+ return null;
+ }
}
- if (key.equalsIgnoreCase("Cookie2")) {
- return userCookies2;
+ if (!setUserCookies) {
+ if (key.equalsIgnoreCase("Cookie")) {
+ return userCookies;
+ }
+ if (key.equalsIgnoreCase("Cookie2")) {
+ return userCookies2;
+ }
}
+ return requests.findValue(key);
+ } finally {
+ connectionLock.unlock();
}
- return requests.findValue(key);
}
/**
@@ -3255,29 +3323,34 @@
* @since 1.4
*/
@Override
- public synchronized Map<String, List<String>> getRequestProperties() {
- if (connected)
- throw new IllegalStateException("Already connected");
-
- // exclude headers containing security-sensitive info
- if (setUserCookies) {
- return requests.getHeaders(EXCLUDE_HEADERS);
+ public Map<String, List<String>> getRequestProperties() {
+ connectionLock.lock();
+ try {
+ if (connected)
+ throw new IllegalStateException("Already connected");
+
+ // exclude headers containing security-sensitive info
+ if (setUserCookies) {
+ return requests.getHeaders(EXCLUDE_HEADERS);
+ }
+ /*
+ * The cookies in the requests message headers may have
+ * been modified. Use the saved user cookies instead.
+ */
+ Map<String, List<String>> userCookiesMap = null;
+ if (userCookies != null || userCookies2 != null) {
+ userCookiesMap = new HashMap<>();
+ if (userCookies != null) {
+ userCookiesMap.put("Cookie", Arrays.asList(userCookies));
+ }
+ if (userCookies2 != null) {
+ userCookiesMap.put("Cookie2", Arrays.asList(userCookies2));
+ }
+ }
+ return requests.filterAndAddHeaders(EXCLUDE_HEADERS2, userCookiesMap);
+ } finally {
+ connectionLock.unlock();
}
- /*
- * The cookies in the requests message headers may have
- * been modified. Use the saved user cookies instead.
- */
- Map<String, List<String>> userCookiesMap = null;
- if (userCookies != null || userCookies2 != null) {
- userCookiesMap = new HashMap<>();
- if (userCookies != null) {
- userCookiesMap.put("Cookie", Arrays.asList(userCookies));
- }
- if (userCookies2 != null) {
- userCookiesMap.put("Cookie2", Arrays.asList(userCookies2));
- }
- }
- return requests.filterAndAddHeaders(EXCLUDE_HEADERS2, userCookiesMap);
}
@Override
@@ -3321,7 +3394,7 @@
* value to be used in milliseconds
* @throws IllegalArgumentException if the timeout parameter is negative
*
- * @see java.net.URLConnectiongetReadTimeout()
+ * @see java.net.URLConnection#getReadTimeout()
* @see java.io.InputStream#read()
* @since 1.5
*/
@@ -3440,6 +3513,7 @@
* @see java.io.FilterInputStream#in
* @see java.io.FilterInputStream#reset()
*/
+ // safe to use synchronized here: super method is synchronized too
@Override
public synchronized void mark(int readlimit) {
super.mark(readlimit);
@@ -3470,6 +3544,7 @@
* @see java.io.FilterInputStream#in
* @see java.io.FilterInputStream#mark(int)
*/
+ // safe to use synchronized here: super method is synchronized too
@Override
public synchronized void reset() throws IOException {
super.reset();
--- a/src/java.base/share/classes/sun/net/www/protocol/http/NegotiateAuthentication.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/share/classes/sun/net/www/protocol/http/NegotiateAuthentication.java Fri Aug 30 15:42:27 2019 +0100
@@ -30,6 +30,8 @@
import java.net.Authenticator.RequestorType;
import java.util.Base64;
import java.util.HashMap;
+import java.util.concurrent.locks.ReentrantLock;
+
import sun.net.www.HeaderParser;
import sun.util.logging.PlatformLogger;
import static sun.net.www.protocol.http.AuthScheme.NEGOTIATE;
@@ -58,6 +60,8 @@
// the cache can be used only once, so after the first use, it's cleaned.
static HashMap <String, Boolean> supported = null;
static ThreadLocal <HashMap <String, Negotiator>> cache = null;
+ private static final ReentrantLock negotiateLock = new ReentrantLock();
+
/* Whether cache is enabled for Negotiate/Kerberos */
private static final boolean cacheSPNEGO;
static {
@@ -126,40 +130,50 @@
*
* @return true if supported
*/
- private static synchronized boolean isSupportedImpl(HttpCallerInfo hci) {
- if (supported == null) {
- supported = new HashMap<>();
- }
- String hostname = hci.host;
- hostname = hostname.toLowerCase();
- if (supported.containsKey(hostname)) {
- return supported.get(hostname);
- }
+ private static boolean isSupportedImpl(HttpCallerInfo hci) {
+ negotiateLock.lock();
+ try {
+ if (supported == null) {
+ supported = new HashMap<>();
+ }
+ String hostname = hci.host;
+ hostname = hostname.toLowerCase();
+ if (supported.containsKey(hostname)) {
+ return supported.get(hostname);
+ }
- Negotiator neg = Negotiator.getNegotiator(hci);
- if (neg != null) {
- supported.put(hostname, true);
- // the only place cache.put is called. here we can make sure
- // the object is valid and the oneToken inside is not null
- if (cache == null) {
- cache = new ThreadLocal<>() {
- @Override
- protected HashMap<String, Negotiator> initialValue() {
- return new HashMap<>();
- }
- };
+ Negotiator neg = Negotiator.getNegotiator(hci);
+ if (neg != null) {
+ supported.put(hostname, true);
+ // the only place cache.put is called. here we can make sure
+ // the object is valid and the oneToken inside is not null
+ if (cache == null) {
+ cache = new ThreadLocal<>() {
+ @Override
+ protected HashMap<String, Negotiator> initialValue() {
+ return new HashMap<>();
+ }
+ };
+ }
+ cache.get().put(hostname, neg);
+ return true;
+ } else {
+ supported.put(hostname, false);
+ return false;
}
- cache.get().put(hostname, neg);
- return true;
- } else {
- supported.put(hostname, false);
- return false;
+ } finally {
+ negotiateLock.unlock();
}
}
- private static synchronized HashMap<String, Negotiator> getCache() {
- if (cache == null) return null;
- return cache.get();
+ private static HashMap<String, Negotiator> getCache() {
+ negotiateLock.lock();
+ try {
+ if (cache == null) return null;
+ return cache.get();
+ } finally {
+ negotiateLock.unlock();
+ }
}
@Override
@@ -197,7 +211,7 @@
* @return true if all goes well, false if no headers were set.
*/
@Override
- public synchronized boolean setHeaders(HttpURLConnection conn, HeaderParser p, String raw) {
+ public boolean setHeaders(HttpURLConnection conn, HeaderParser p, String raw) {
try {
String response;
--- a/src/java.base/share/classes/sun/net/www/protocol/https/HttpsClient.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/share/classes/sun/net/www/protocol/https/HttpsClient.java Fri Aug 30 15:42:27 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2001, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 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
@@ -42,18 +42,14 @@
import java.util.Objects;
import java.util.StringTokenizer;
import java.util.Vector;
-
-import javax.security.auth.x500.X500Principal;
-
+import java.util.concurrent.locks.Lock;
import javax.net.ssl.*;
import sun.net.www.http.HttpClient;
import sun.net.www.protocol.http.AuthenticatorKeys;
import sun.net.www.protocol.http.HttpURLConnection;
import sun.security.action.*;
-
import sun.security.util.HostnameChecker;
import sun.security.ssl.SSLSocketImpl;
-
import sun.util.logging.PlatformLogger;
import static sun.net.www.protocol.http.HttpURLConnection.TunnelState.*;
@@ -208,7 +204,7 @@
* Use New to get new HttpsClient. This constructor is meant to be
* used only by New method. New properly checks for URL spoofing.
*
- * @param URL https URL with which a connection must be established
+ * @param url https URL with which a connection must be established
*/
private HttpsClient(SSLSocketFactory sf, URL url)
throws IOException
@@ -341,8 +337,10 @@
boolean compatible = ((ret.proxy != null && ret.proxy.equals(p)) ||
(ret.proxy == null && p == Proxy.NO_PROXY))
&& Objects.equals(ret.getAuthenticatorKey(), ak);
+ Lock lock = ret.clientLock;
if (compatible) {
- synchronized (ret) {
+ lock.lock();
+ try {
ret.cachedHttpClient = true;
assert ret.inCache;
ret.inCache = false;
@@ -351,18 +349,23 @@
if (logger.isLoggable(PlatformLogger.Level.FINEST)) {
logger.finest("KeepAlive stream retrieved from the cache, " + ret);
}
+ } finally {
+ lock.unlock();
}
} else {
// We cannot return this connection to the cache as it's
// KeepAliveTimeout will get reset. We simply close the connection.
// This should be fine as it is very rare that a connection
// to the same host will not use the same proxy.
- synchronized(ret) {
+ lock.lock();
+ try {
if (logger.isLoggable(PlatformLogger.Level.FINEST)) {
logger.finest("Not returning this connection to cache: " + ret);
}
ret.inCache = false;
ret.closeServer();
+ } finally {
+ lock.unlock();
}
ret = null;
}
--- a/src/java.base/unix/classes/sun/net/www/protocol/http/ntlm/NTLMAuthentication.java Fri Aug 30 13:11:16 2019 +0100
+++ b/src/java.base/unix/classes/sun/net/www/protocol/http/ntlm/NTLMAuthentication.java Fri Aug 30 15:42:27 2019 +0100
@@ -225,7 +225,7 @@
* @return true if all goes well, false if no headers were set.
*/
@Override
- public synchronized boolean setHeaders(HttpURLConnection conn, HeaderParser p, String raw) {
+ public boolean setHeaders(HttpURLConnection conn, HeaderParser p, String raw) {
try {
String response;