Skip to content
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.

Sign up for GitHub

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

6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available #14981

Closed
wants to merge 8 commits into from
Closed

6478546: FileInputStream.read() throws OutOfMemoryError when there is plenty available #14981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8d7b347
6478546: FileInputStream.read() throws OutOfMemoryError when there is…
bplb Jul 20, 2023
973e848
6478546: Prevent short read
bplb Jul 25, 2023
1cb8684
6478546: Minor refactoring
bplb Jul 25, 2023
69941de
6478546: Decrease malloc limit to 1.5 MB
bplb Jul 25, 2023
9264975
6478546: Move buffer clamping up to Java layer; correct read behavior…
bplb Jul 27, 2023
cdb8455
6478546: do-while -> while-do in writeBytes; remove NULL and bounds c…
bplb Jul 31, 2023
3a016a7
Merge
bplb Oct 9, 2023
408e2df
Merge
bplb Nov 6, 2023
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
54 changes: 41 additions & 13 deletions src/java.base/share/classes/java/io/FileInputStream.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

import java.nio.channels.FileChannel;
import java.util.Arrays;
import java.util.Objects;
import jdk.internal.misc.Blocker;
import jdk.internal.util.ArraysSupport;
import sun.nio.ch.FileChannelImpl;
Expand Down Expand Up @@ -236,14 +237,51 @@ public int read() throws IOException {

private native int read0() throws IOException;

private native int readBytes0(byte[] b, int off, int len) throws IOException;

/**
* Reads a subarray as a sequence of bytes.
* @param b the data to be written
* @param off the start offset in the data
* @param len the number of bytes that are written
* @throws IOException If an I/O error has occurred.
*/
private native int readBytes(byte[] b, int off, int len) throws IOException;
private int readBytes(byte[] b, int off, int len) throws IOException {
Objects.checkFromIndexSize(off, len, b.length);
int nread = 0;
int pos = off;
int remaining = len;
long comp = Blocker.begin();
try {
do {
int size = Math.min(remaining, 1572864);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.5Mb seems high, I think we really need micro that do real file I/O to help tune this.

try {
int n = readBytes0(b, pos, size);
if (n < 0) {
// EOF
if (nread == 0)
nread = -1;
break;
}
nread += n;
if (n < size) {
// buffer not filled
break;
}
pos += n;
remaining -= n;
} catch (IOException ioe) {
if (nread > 0) {
break;
}
throw ioe;
}
} while (remaining > 0);
} finally {
Blocker.end(comp);
}
return nread;
}

/**
* Reads up to {@code b.length} bytes of data from this input
Expand All @@ -258,12 +296,7 @@ public int read() throws IOException {
*/
@Override
public int read(byte[] b) throws IOException {
long comp = Blocker.begin();
try {
return readBytes(b, 0, b.length);
} finally {
Blocker.end(comp);
}
return readBytes(b, 0, b.length);
}

/**
Expand All @@ -282,12 +315,7 @@ public int read(byte[] b) throws IOException {
*/
@Override
public int read(byte[] b, int off, int len) throws IOException {
long comp = Blocker.begin();
try {
return readBytes(b, off, len);
} finally {
Blocker.end(comp);
}
return readBytes(b, off, len);
}

@Override
Expand Down
47 changes: 27 additions & 20 deletions src/java.base/share/classes/java/io/FileOutputStream.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
package java.io;

import java.nio.channels.FileChannel;
import java.util.Objects;
import jdk.internal.access.SharedSecrets;
import jdk.internal.access.JavaIOFileDescriptorAccess;
import jdk.internal.misc.Blocker;
Expand Down Expand Up @@ -299,7 +300,7 @@ private void open(String name, boolean append) throws FileNotFoundException {
* @param append {@code true} if the write operation first
* advances the position to the end of file
*/
private native void write(int b, boolean append) throws IOException;
private native void write0(int b, boolean append) throws IOException;

/**
* Writes the specified byte to this file output stream. Implements
Expand All @@ -313,23 +314,41 @@ public void write(int b) throws IOException {
boolean append = FD_ACCESS.getAppend(fd);
long comp = Blocker.begin();
try {
write(b, append);
write0(b, append);
} finally {
Blocker.end(comp);
}
}

private native int writeBytes0(byte[] b, int off, int len, boolean append)
throws IOException;

/**
* Writes a sub array as a sequence of bytes.
* @param b the data to be written
* @param off the start offset in the data
* @param len the number of bytes that are written
* @param append {@code true} to first advance the position to the
* end of file
* @throws IOException If an I/O error has occurred.
*/
private native void writeBytes(byte[] b, int off, int len, boolean append)
throws IOException;
private void writeBytes(byte[] b, int off, int len)
throws IOException
{
Objects.checkFromIndexSize(off, len, b.length);
boolean append = FD_ACCESS.getAppend(fd);
int pos = off;
int remaining = len;
long comp = Blocker.begin();
try {
while (remaining > 0) {
int size = Math.min(remaining, 1572864);
int n = writeBytes0(b, pos, size, append);
pos += n;
remaining -= n;
}
} finally {
Blocker.end(comp);
}
}

/**
* Writes {@code b.length} bytes from the specified byte array
Expand All @@ -340,13 +359,7 @@ private native void writeBytes(byte[] b, int off, int len, boolean append)
*/
@Override
public void write(byte[] b) throws IOException {
boolean append = FD_ACCESS.getAppend(fd);
long comp = Blocker.begin();
try {
writeBytes(b, 0, b.length, append);
} finally {
Blocker.end(comp);
}
writeBytes(b, 0, b.length);
}

/**
Expand All @@ -361,13 +374,7 @@ public void write(byte[] b) throws IOException {
*/
@Override
public void write(byte[] b, int off, int len) throws IOException {
boolean append = FD_ACCESS.getAppend(fd);
long comp = Blocker.begin();
try {
writeBytes(b, off, len, append);
} finally {
Blocker.end(comp);
}
writeBytes(b, off, len);
}

/**
Expand Down
43 changes: 40 additions & 3 deletions src/java.base/share/classes/java/io/RandomAccessFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
package java.io;

import java.nio.channels.FileChannel;
import java.util.Objects;

import jdk.internal.access.JavaIORandomAccessFileAccess;
import jdk.internal.access.SharedSecrets;
Expand Down Expand Up @@ -393,12 +394,40 @@ public int read() throws IOException {
* @throws IOException If an I/O error has occurred.
*/
private int readBytes(byte[] b, int off, int len) throws IOException {
Objects.checkFromIndexSize(off, len, b.length);
int nread = 0;
int pos = off;
int remaining = len;
long comp = Blocker.begin();
try {
return readBytes0(b, off, len);
do {
int size = Math.min(remaining, 1572864);
try {
int n = readBytes0(b, pos, size);
if (n < 0) {
// EOF
if (nread == 0)
nread = -1;
break;
}
nread += n;
if (n < size) {
// buffer not filled
break;
}
pos += n;
remaining -= n;
} catch (IOException ioe) {
if (nread > 0) {
break;
}
throw ioe;
}
} while (remaining > 0);
} finally {
Blocker.end(comp);
}
return nread;
}

private native int readBytes0(byte[] b, int off, int len) throws IOException;
Expand Down Expand Up @@ -565,15 +594,23 @@ public void write(int b) throws IOException {
* @throws IOException If an I/O error has occurred.
*/
private void writeBytes(byte[] b, int off, int len) throws IOException {
Objects.checkFromIndexSize(off, len, b.length);
int pos = off;
int remaining = len;
long comp = Blocker.begin();
try {
writeBytes0(b, off, len);
while (remaining > 0) {
int size = Math.min(remaining, 1572864);
int n = writeBytes0(b, pos, size);
pos += n;
remaining -= n;
}
} finally {
Blocker.end(comp);
}
}

private native void writeBytes0(byte[] b, int off, int len) throws IOException;
private native int writeBytes0(byte[] b, int off, int len) throws IOException;

/**
* Writes {@code b.length} bytes from the specified byte array
Expand Down
2 changes: 1 addition & 1 deletion src/java.base/share/native/libjava/FileInputStream.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ Java_java_io_FileInputStream_read0(JNIEnv *env, jobject this) {
}

JNIEXPORT jint JNICALL
Java_java_io_FileInputStream_readBytes(JNIEnv *env, jobject this,
Java_java_io_FileInputStream_readBytes0(JNIEnv *env, jobject this,
jbyteArray bytes, jint off, jint len) {
return readBytes(env, this, bytes, off, len, fis_fd);
}
Expand Down
8 changes: 4 additions & 4 deletions src/java.base/share/native/libjava/FileOutputStream.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,13 @@ Java_java_io_FileOutputStream_open0(JNIEnv *env, jobject this,
}

JNIEXPORT void JNICALL
Java_java_io_FileOutputStream_write(JNIEnv *env, jobject this, jint byte, jboolean append) {
Java_java_io_FileOutputStream_write0(JNIEnv *env, jobject this, jint byte, jboolean append) {
writeSingle(env, this, byte, append, fos_fd);
}

JNIEXPORT void JNICALL
Java_java_io_FileOutputStream_writeBytes(JNIEnv *env,
JNIEXPORT int JNICALL
Java_java_io_FileOutputStream_writeBytes0(JNIEnv *env,
jobject this, jbyteArray bytes, jint off, jint len, jboolean append) {
writeBytes(env, this, bytes, off, len, append, fos_fd);
return writeBytes(env, this, bytes, off, len, append, fos_fd);
}

4 changes: 2 additions & 2 deletions src/java.base/share/native/libjava/RandomAccessFile.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ Java_java_io_RandomAccessFile_write0(JNIEnv *env, jobject this, jint byte) {
writeSingle(env, this, byte, JNI_FALSE, raf_fd);
}

JNIEXPORT void JNICALL
JNIEXPORT int JNICALL
Java_java_io_RandomAccessFile_writeBytes0(JNIEnv *env,
jobject this, jbyteArray bytes, jint off, jint len) {
writeBytes(env, this, bytes, off, len, JNI_FALSE, raf_fd);
return writeBytes(env, this, bytes, off, len, JNI_FALSE, raf_fd);
}

JNIEXPORT jlong JNICALL
Expand Down
Loading