Files.copy(InputStream, Path, CopyOption...) can fail with OOM if direct memory is limited

[github at paul.kungfoocoder.org]

Recently we came across a very surprising situation.

In our code we were doing something similar to:

echo 'new java.io.ByteArrayInputStream(new byte[256*1024*1024]).transferTo(new java.io.FileOutputStream("/tmp/aaa"));' | jshell -R-XX:MaxDirectMemorySize=25M

Which works perfectly well. For various reasons some of the code was adapted to use Paths, instead of Files, and this was changed to (effectively):

echo 'java.nio.file.Files.copy(new java.io.ByteArrayInputStream(new byte[256*1024*1024]), java.nio.file.Path.of("/tmp/aaa"), java.nio.file.StandardCopyOption.REPLACE_EXISTING);' | jshell -R-XX:MaxDirectMemorySize=25M

Whereupon it started to fail with a message similar to:
Exception java.lang.OutOfMemoryError: Cannot reserve 268435456 bytes of direct buffer memory (allocated: 127, limit: 26214400)
Note that this isn't strictly a problem of Files.copy, but rather of the output stream returned byFiles.newOutputStream, as evidenced by echo 'new java.io.ByteArrayInputStream(new byte[256*1024*1024]).transferTo(java.nio.file.Files.newOutputStream(java.nio.file.Path.of("/tmp/aaa")));' | jshell -R-XX:MaxDirectMemorySize=25M 

This makes it much harder to use a Path, since we cannot know whether or not it will be possible to write to the file or not.

This fails on all of Java 11 (11.0.18), Java 17 (17.0.8), and Java 20 (openjdk 20.0.2 2023-07-18), and has been tested on both Mac and Linux.

In this particular code path, the easiest fix is to replace https://github.com/adoptium/jdk/blob/d0be73a78038faf9509623bc4ba71eb4385cd645/src/java.base/share/classes/sun/nio/ch/IOUtil.java#L94 with something like:

            bb = Util.getTemporaryDirectBuffer(Math.min(rem, VM.maxDirectMemory()/2));
Since the code outside of this method already loops until the ByteBuffer is full, at least on this code path. Other code paths should also retry, since Channel.write() doesn't guarantee to write all bytes anyway. A better fix would be to look at how much memory is available, and allocate that much, or to fall back to a streaming copy, similar to what the default InputStream.transferTo already does.

You might wonder _why_ we limit the MaxDirectMemorySize, and we do so since we found that if we don’t it can lead to the OOMKiller killing our containers, due to memory being allocated outside of the heap. Nonetheless, whether it should be limited or not, then I still find it very surprising that I can do inputStream.transferTo(fileOutputStream) and expect that it will work, regardless of the InputStream, but I cannot do the same with inputStream.transferTo(channelOutputStream), as that might fail depending on how much data is available in the input stream, and the amount of available Direct Memory.

Discussion also raised with Adoptium, where they suggested to post it here.
https://github.com/adoptium/adoptium-support/issues/894

For our product, I have changed Files.copy() everywhere to use InputStream.transferTo(fileOutputStream), but this doesn’t seem like it is the intended result. I look forward to your responses on how we can best deal with this within Java.

Cheers,
Paul
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/nio-dev/attachments/20230912/a4764a66/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: apple-touch-icon-180x180.png
Type: image/png
Size: 21128 bytes
Desc: not available
URL: <https://mail.openjdk.org/pipermail/nio-dev/attachments/20230912/a4764a66/apple-touch-icon-180x180-0001.png>