8229867: Re-examine synchronization usages in http and https protocol handlers

Reviewed-by: chegar, alanb, michaelm

Author: dfuch

src/java.base/share/classes/sun/net/www/MessageHeader.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1995, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1995, 2020, 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
@@ -294,7 +294,7 @@ public synchronized Map<String, List<String>> filterAndAddHeaders(
      * @param line the line to check.
      * @return true if the line might be a request line.
      */
-    private boolean isRequestline(String line) {
+    private static boolean isRequestline(String line) {
         String k = line.trim();
         int i = k.lastIndexOf(' ');
         if (i <= 0) return false;
@@ -311,12 +311,23 @@ private boolean isRequestline(String line) {
         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  static 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]);

src/java.base/share/classes/sun/net/www/MeteredStream.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1994, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1994, 2020, 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 @@ public class MeteredStream extends FilterInputStream {
     protected long markedCount = 0;
     protected int markLimit = -1;
     protected ProgressSource pi;
+    private final ReentrantLock readLock = new ReentrantLock();
 
     public MeteredStream(InputStream is, ProgressSource pi, long expected)
     {
@@ -57,7 +57,9 @@ public MeteredStream(InputStream is, ProgressSource pi, long expected)
         }
     }
 
-    private final void justRead(long n) throws IOException   {
+    private final void justRead(long n) throws IOException {
+        assert isLockHeldByCurrentThread();
+
         if (n == -1) {
 
             /*
@@ -99,6 +101,7 @@ private final void justRead(long n) throws IOException   {
      * Returns true if the mark is valid, false otherwise
      */
     private boolean isMarked() {
+        assert isLockHeldByCurrentThread();
 
         if (markLimit < 0) {
             return false;
@@ -113,94 +116,130 @@ private boolean isMarked() {
         return true;
     }
 
-    public synchronized int read() throws java.io.IOException {
-        if (closed) {
-            return -1;
-        }
-        int c = in.read();
-        if (c != -1) {
-            justRead(1);
-        } else {
-            justRead(c);
+    public int read() throws java.io.IOException {
+        lock();
+        try {
+            if (closed) return -1;
+            int c = in.read();
+            if (c != -1) {
+                justRead(1);
+            } else {
+                justRead(c);
+            }
+            return c;
+        } finally {
+            unlock();
         }
-        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;
-        }
-        int n = in.read(b, off, len);
-        justRead(n);
-        return n;
-    }
-
-    public synchronized long skip(long n) throws IOException {
+        lock();
+        try {
+            if (closed) return -1;
 
-        // REMIND: what does skip do on EOF????
-        if (closed) {
-            return 0;
+            int n = in.read(b, off, len);
+            justRead(n);
+            return n;
+        } finally {
+            unlock();
         }
+    }
 
-        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);
+    public long skip(long n) throws IOException {
+        lock();
+        try {
+            // REMIND: what does skip do on EOF????
+            if (closed) return 0;
+
+            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 {
+            unlock();
         }
-        justRead(n);
-        return n;
     }
 
     public void close() throws IOException {
-        if (closed) {
-            return;
-        }
-        if (pi != null)
-            pi.finishTracking();
+        lock();
+        try {
+            if (closed) return;
+            if (pi != null)
+                pi.finishTracking();
 
-        closed = true;
-        in.close();
+            closed = true;
+            in.close();
+        } finally {
+            unlock();
+        }
     }
 
-    public synchronized int available() throws IOException {
-        return closed ? 0: in.available();
+    public int available() throws IOException {
+        lock();
+        try {
+            return closed ? 0 : in.available();
+        } finally {
+            unlock();
+        }
     }
 
-    public synchronized void mark(int readLimit) {
-        if (closed) {
-            return;
-        }
-        super.mark(readLimit);
+    public void mark(int readLimit) {
+        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 {
+            unlock();
+        }
     }
 
-    public synchronized void reset() throws IOException {
-        if (closed) {
-            return;
-        }
+    public void reset() throws IOException {
+        lock();
+        try {
+            if (closed) return;
+            if (!isMarked()) {
+                throw new IOException("Resetting to an invalid mark");
+            }
 
-        if (!isMarked()) {
-            throw new IOException ("Resetting to an invalid mark");
+            count = markedCount;
+            super.reset();
+        } finally {
+            unlock();
         }
-
-        count = markedCount;
-        super.reset();
     }
 
     public boolean markSupported() {
-        if (closed) {
-            return false;
+        lock();
+        try {
+            if (closed) return false;
+            return super.markSupported();
+        } finally {
+            unlock();
         }
-        return super.markSupported();
+    }
+
+    public final void lock() {
+        readLock.lock();
+    }
+
+    public final void unlock() {
+        readLock.unlock();
+    }
+
+    public final boolean isLockHeldByCurrentThread() {
+        return readLock.isHeldByCurrentThread();
     }
 
     @SuppressWarnings("deprecation")