Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions src/java.base/share/classes/sun/net/www/MessageHeader.java
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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]);
Expand Down
173 changes: 106 additions & 67 deletions src/java.base/share/classes/sun/net/www/MeteredStream.java
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;

Expand All @@ -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)
{
Expand All @@ -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) {

/*
Expand Down Expand Up @@ -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;
Expand All @@ -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")
Expand Down
Loading