# HG changeset patch # User dfuchs # Date 1567176147 -3600 # Node ID 8595871a54467cd547360a9cf0689aa8a218388b # Parent e89c7aaf29069b40eb2fdc2cae41a4ff1bbe0baf JDK-8229867: first prototype diff -r e89c7aaf2906 -r 8595871a5446 src/java.base/share/classes/sun/net/www/MessageHeader.java --- 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]); diff -r e89c7aaf2906 -r 8595871a5446 src/java.base/share/classes/sun/net/www/MeteredStream.java --- 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") diff -r e89c7aaf2906 -r 8595871a5446 src/java.base/share/classes/sun/net/www/http/ChunkedInputStream.java --- 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); } } diff -r e89c7aaf2906 -r 8595871a5446 src/java.base/share/classes/sun/net/www/http/ChunkedOutputStream.java --- 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 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(); } } } diff -r e89c7aaf2906 -r 8595871a5446 src/java.base/share/classes/sun/net/www/http/HttpCapture.java --- 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; diff -r e89c7aaf2906 -r 8595871a5446 src/java.base/share/classes/sun/net/www/http/HttpClient.java --- 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() { diff -r e89c7aaf2906 -r 8595871a5446 src/java.base/share/classes/sun/net/www/http/KeepAliveCache.java --- 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 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 { @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; } /* diff -r e89c7aaf2906 -r 8595871a5446 src/java.base/share/classes/sun/net/www/http/KeepAliveStream.java --- 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; diff -r e89c7aaf2906 -r 8595871a5446 src/java.base/share/classes/sun/net/www/http/KeepAliveStreamCleaner.java --- 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) { } diff -r e89c7aaf2906 -r 8595871a5446 src/java.base/share/classes/sun/net/www/protocol/http/AuthCacheImpl.java --- 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> hashtable; diff -r e89c7aaf2906 -r 8595871a5446 src/java.base/share/classes/sun/net/www/protocol/http/AuthenticationInfo.java --- 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 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 { diff -r e89c7aaf2906 -r 8595871a5446 src/java.base/share/classes/sun/net/www/protocol/http/DigestAuthentication.java --- 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; diff -r e89c7aaf2906 -r 8595871a5446 src/java.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java --- 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., "accept"). * @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> getRequestProperties() { - if (connected) - throw new IllegalStateException("Already connected"); - - // exclude headers containing security-sensitive info - if (setUserCookies) { - return requests.getHeaders(EXCLUDE_HEADERS); + public Map> 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> 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> 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(); diff -r e89c7aaf2906 -r 8595871a5446 src/java.base/share/classes/sun/net/www/protocol/http/NegotiateAuthentication.java --- 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 supported = null; static ThreadLocal > 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 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 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 getCache() { - if (cache == null) return null; - return cache.get(); + private static HashMap 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; diff -r e89c7aaf2906 -r 8595871a5446 src/java.base/share/classes/sun/net/www/protocol/https/HttpsClient.java --- 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; } diff -r e89c7aaf2906 -r 8595871a5446 src/java.base/unix/classes/sun/net/www/protocol/http/ntlm/NTLMAuthentication.java --- 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;