[foreign-memaccess] RFR 8234581: Cleanup use of exceptions in the memory access API

[Paul Sandoz]

Good cleanup.


You might be able to use Objects.checkFromIndexSize in MemorySegmentImpl instead of your own checkBounds method (it’s possible to customize the exception message if you use the internal Preconditions.checkFromIndexSize).


I generally try to avoid the use of < etc because I find it makes it harder to read the doc in source (I tend to read doc more in source than in the generated HTML).

* @throws IllegalArgumentException if the start index is < 0, or if the step is = 0.
->
* @throws IllegalArgumentException if {@code start < 0} or {@code step == 0}.

 174      * @throws IndexOutOfBoundsException if the new segment bounds are illegal; this can happen because:
 175      * <ul>
 176      * <li>{@code offset} is negative or greater than {@code byteSize()}</li>
 177      * <li>{@code newSize} is negative or {@code newSize > byteSize() - offset}</li>
 178      * </ul>
->

@throws IndexOutOfBoundsException if {@code offset < 0}, {@code offset > byteSize()}, {@code newSize < 0}, or  {@code newSize > byteSize() - offset}

(Since we are using IndexOutOfBoundsException we can be a little more terse.)

If you are ok with that I am happy to make such a change after your patch has settled down and pushed.

Paul.

> On Nov 21, 2019, at 4:53 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
> 
> New day, new cleanup :-)
> 
> This time, following some of the indications from Paul, the goal is to cleanup usages of exceptions in the memory access API. I found 4 broad categories of issues:
> 
> - We sometimes throw IAE where an IOOB would be preferred (namely, MemorySegment::slice and MemoryAddress::copy)
> 
> - We sometimes use RuntimeException to document unforeseen failures of Unsafe::allocateMemory (ByteBuffer::allocateDirect does not do so)
> 
> - The handling of exceptions when forming layout paths is plain inconsistent - and IAE should be used throughout
> 
> - MemorySegment::close should always throw ISE if the state is not valid
> 
> 
> The changes in LayoutPath handling required some deeper changes in the implementation - which now triggers an uniform IAE with a message of the kind: Bad layout path: <cause>.
> 
> 
> Since I was there, I also added the logic to zero the native memory allocated with MemorySegment::allocateNative. This behavior can be disabled with a runtime flag: -Djdk.internal.foreign.skipZeroMemory.
> 
> 
> I also made extra sure that the exceptions thrown in the code are reflected in the throws clause of the API methods, and that there are the right @throw taglets for them.
> 
> Webrev:
> 
> http://cr.openjdk.java.net/~mcimadamore/panama/8234581/
> 
> javadoc:
> 
> cr.openjdk.java.net/~mcimadamore/panama/8234581_javadoc
> 
> 
> Cheers
> Maurizio
>