RFR: 8299684: (bf) JNI direct buffer functions with large capacity behave unexpectedly [v4]

[Alan Bateman]

On Fri, 6 Jan 2023 23:56:39 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> Remove cast in `JNI::NewDirectByteBuffer`of `long` capacity to `int`, modify the constructor in question to accept a `long` capacity, and verify in the constructor that the  capacity does not overflow `int` range, throwing IAE If it does.
>
> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8299684: Add test for out of range capacity passed to NewDirectBuffer

I agree with David that it would be clearer and simpler to check if capacity is negative or greater than Integer.MAX_VALUE. There other helper methods called from this code and they are named checkXXX so it would be good to keep that consistent if you can.

We don't have other tests for this JNI function in the test tree so having the new tests be a complete unit test would be good. This means checking the position, limit, and capacity of the ByteBuffer and testing that it's a direct buffer. If you make it a JUnit test then you'll be able to paramatrized test with value sources, and use the assertions API when checking each of the buffer properties.

For the native code used by the test then it would be clearer if allocBigBuffer were renamed to newDirectByteBuffer. Also I think you'll need to check the value from malloc in case it fails. You'll need another native function to free the memory and have the tests invoke the free in a finally block so that memory doesn't accumulate as the test cycles through the different buffer sizes. Right now, I assume the tests don't run on 32-bit as it mallocs two 2Gb areas. Once the test is re-worded then you can get a sense as to how much memory it needs and whether you need a @requires in the test description to ensue that the test system has sufficient memory.

-------------

PR: https://git.openjdk.org/jdk/pull/11873