On Mon, 24 Jul 2023 09:16:23 GMT, Vyom Tewari <vtew...@openjdk.org> wrote:
>> Limit native memory allocation and move write loop from the native layer
>> into Java. This change should make the OOME reported in the issue much less
>> likely.
>
> src/java.base/share/native/libjava/io_util.c line 99:
>
>> 97: return 0;
>> 98: } else if (len > BUF_SIZE) {
>> 99: if (len > MAX_MALLOC_SIZE)
>
> Hi Brian if I am reading code correctly then with the current code change
> FIS.read(byte[] b, int off, int len) will always read (MAX_MALLOC_SIZE
> 2097152) bytes if len > MAX_MALLOC_SIZE.
>
> The java doc of read as below
>
> public int read(byte[] b,
> int off,
> int len)
> throws IOException
> Reads up to len bytes of data from the input stream into an array of bytes.
> An attempt is made to read as many as len bytes, but a smaller number may be
> read. The number of bytes actually read is returned as an integer.
>
> I think if you are limiting the internal dynamic buffer to 2097152 byte then
> you have to at least attempt to read as many as len bytes if possible before
> returning.
>
> If I simply the run the following code
>
> int size = 501 * 501 * 501 * 3;
>
> FileInputStream fis = new
> FileInputStream("/home/vyom1/test.img"); // Any file with size >=
> 501*501*501*2
>
> System.out.println("size: " + size);
>
> byte buf[] = new byte[size];
>
> System.out.println("buf ok");
>
> int bytesRead = fis.read(buf, 0, size);
> System.out.println("Bytes read " + bytesRead);
>
> It will always print “Bytes read 2097152” which is not as per Java
> specification of InputStream.read(byte[]b, int off, int len).
A short read is okay but changing long standing behavior could potentially
break already broken code that calls the method with a large byte array and
assumes it will read to EOF. So I think it's a forced move to read until there
is no space remaining or EOF is reached. This is logic for the Java level, not
the native code of course. If an I/O exception is on the second/subsequent
reads then it will have to return the bytes that are read, not throw with bytes
in the buffer.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14981#discussion_r1272236807
