Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8321513

SegmentAllocator:allocateFrom(AddressLayout, MemorySegment) does not throw stated UnsupportedOperationException

XMLWordPrintable

    • Icon: CSR CSR
    • Resolution: Approved
    • Icon: P3 P3
    • 22, 23
    • core-libs
    • None
    • behavioral
    • minimal
    • Hide
      Code may exclusively catch UOE (and not IAE) from VarHandle and MemorySegment operations. In such cases, the exception type has to be changed.

      It is unlikely, read-only segments are provided to the SegmentAllocator factories. In such rare cases, we will now fail faster.

      As the FFM API was in preview in 21, these changes should not affect production code.
      Show
      Code may exclusively catch UOE (and not IAE) from VarHandle and MemorySegment operations. In such cases, the exception type has to be changed. It is unlikely, read-only segments are provided to the SegmentAllocator factories. In such rare cases, we will now fail faster. As the FFM API was in preview in 21, these changes should not affect production code.
    • Java API
    • SE

      Summary

      Providing or using read-only segments for write operations should throw IllegalArgumentException rather than UnsupportedOperationException. This is also true for a few cases where a native segment is required.

      Problem

      VarHandle objects that take a MemorySegment as an argument should throw IllegalArgumentException if a provided segment argument is read-only and the operation is a write operation. MemorySegment operations (like get() and set()) are in reality convenience methods for underlying VarHandle objects and should mimic the underlying behavior. For consistency, all methods in the FFM API that are similar in type should throw the same exception type.

      Solution

      Change the JavaDocs to reflect the actual and intended exception type.

      Change some methods, like MemorySegment::fill and MemorySegment::copy, so they will throw IllegalArgumentException rather than UnsupportedOperationException.

      Prevent factories in SegmentAllocator to use read-only segments.

      It was discussed to catch IAEs for some MemorySegment operations where the target is this and turn them into an UnsupportedOperationException. However, doing so might result in unwanted performance impacts, and would also limit refactoring options (e.g. working with var handles directly throws different set of exception compared when working with memory segment getter/setters).

      Specification

      diff --git a/src/java.base/share/classes/java/lang/foreign/MemoryLayout.java b/src/java.base/share/classes/java/lang/foreign/MemoryLayout.java
      index 0933c637acbc..321d9b09bad3 100644
      --- a/src/java.base/share/classes/java/lang/foreign/MemoryLayout.java
      +++ b/src/java.base/share/classes/java/lang/foreign/MemoryLayout.java
      @@ -631,6 +631,9 @@ public sealed interface MemoryLayout
            *     <li>The accessed memory segment must be
            *     {@link MemorySegment#isAccessibleBy(Thread) accessible} from the thread
            *     performing the access operation, or a {@link WrongThreadException} is thrown.</li>
      +     *     <li>For write operations, the accessed memory segment must not be
      +     *     {@link MemorySegment#isReadOnly() read only}, or an
      +     *     {@link IllegalArgumentException} is thrown.</li>
            *     <li>The {@linkplain MemorySegment#scope() scope} associated with the accessed
            *     segment must be {@linkplain MemorySegment.Scope#isAlive() alive}, or an
            *     {@link IllegalStateException} is thrown.</li>
      diff --git a/src/java.base/share/classes/java/lang/foreign/MemorySegment.java b/src/java.base/share/classes/java/lang/foreign/MemorySegment.java
      index 0b0a2e136cb5..da255b9912b2 100644
      --- a/src/java.base/share/classes/java/lang/foreign/MemorySegment.java
      +++ b/src/java.base/share/classes/java/lang/foreign/MemorySegment.java
      @@ -869,7 +869,7 @@ MemorySegment reinterpret(long newSize,
            *         this segment is not {@linkplain Scope#isAlive() alive}
            * @throws WrongThreadException if this method is called from a thread {@code T},
            *         such that {@code isAccessibleBy(T) == false}
      -     * @throws UnsupportedOperationException if this segment is
      +     * @throws IllegalArgumentException if this segment is
            *         {@linkplain #isReadOnly() read-only}
            */
           MemorySegment fill(byte value);
      @@ -894,7 +894,7 @@ MemorySegment reinterpret(long newSize,
            *         {@code src} is not {@linkplain Scope#isAlive() alive}
            * @throws WrongThreadException if this method is called from a thread {@code T},
            *         such that {@code src.isAccessibleBy(T) == false}
      -     * @throws UnsupportedOperationException if this segment is
      +     * @throws IllegalArgumentException if this segment is
            *         {@linkplain #isReadOnly() read-only}
            * @return this segment
            */
      @@ -1269,6 +1269,8 @@ MemorySegment reinterpret(long newSize,
            *         this segment is not {@linkplain Scope#isAlive() alive}
            * @throws WrongThreadException if this method is called from a thread {@code T},
            *         such that {@code isAccessibleBy(T) == false}
      +     * @throws IllegalArgumentException if this segment is
      +     *         {@linkplain #isReadOnly() read-only}
            */
           void setString(long offset, String str);
      
      @@ -1306,6 +1308,8 @@ MemorySegment reinterpret(long newSize,
            *         such that {@code isAccessibleBy(T) == false}
            * @throws IllegalArgumentException if {@code charset} is not a
            *         {@linkplain StandardCharsets standard charset}
      +     * @throws IllegalArgumentException if this segment is
      +     *         {@linkplain #isReadOnly() read-only}
            */
           void setString(long offset, String str, Charset charset);
      
      @@ -1493,7 +1497,7 @@ static MemorySegment ofAddress(long address) {
            * @throws IndexOutOfBoundsException if {@code dstOffset > dstSegment.byteSize() - bytes}
            * @throws IndexOutOfBoundsException if either {@code srcOffset},
            *         {@code dstOffset} or {@code bytes} are {@code < 0}
      -     * @throws UnsupportedOperationException if {@code dstSegment} is
      +     * @throws IllegalArgumentException if {@code dstSegment} is
            *         {@linkplain #isReadOnly() read-only}
            */
           @ForceInline
      @@ -1552,7 +1556,7 @@ static void copy(MemorySegment srcSegment, long srcOffset,
            *         {@code dstSegment} is not {@linkplain Scope#isAlive() alive}
            * @throws WrongThreadException if this method is called from a thread {@code T},
            *         such that {@code dstSegment.isAccessibleBy(T) == false}
      -     * @throws UnsupportedOperationException if {@code dstSegment} is {@linkplain #isReadOnly() read-only}
      +     * @throws IllegalArgumentException if {@code dstSegment} is {@linkplain #isReadOnly() read-only}
            * @throws IndexOutOfBoundsException if {@code elementCount * srcLayout.byteSize()} overflows
            * @throws IndexOutOfBoundsException if {@code elementCount * dtsLayout.byteSize()} overflows
            * @throws IndexOutOfBoundsException if {@code srcOffset > srcSegment.byteSize() - (elementCount * srcLayout.byteSize())}
      @@ -1605,7 +1609,7 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            *         <a href="MemorySegment.html#segment-alignment">incompatible with the alignment constraint</a>
            *         in the provided layout
            * @throws IndexOutOfBoundsException if {@code offset > byteSize() - layout.byteSize()}
      -     * @throws UnsupportedOperationException if this segment is
      +     * @throws IllegalArgumentException if this segment is
            *         {@linkplain #isReadOnly() read-only}
            */
           void set(ValueLayout.OfByte layout, long offset, byte value);
      @@ -1643,7 +1647,7 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            *         <a href="MemorySegment.html#segment-alignment">incompatible with the alignment constraint</a>
            *         in the provided layout
            * @throws IndexOutOfBoundsException if {@code offset > byteSize() - layout.byteSize()}
      -     * @throws UnsupportedOperationException if this segment is
      +     * @throws IllegalArgumentException if this segment is
            *         {@linkplain #isReadOnly() read-only}
            */
           void set(ValueLayout.OfBoolean layout, long offset, boolean value);
      @@ -1681,7 +1685,7 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            *         <a href="MemorySegment.html#segment-alignment">incompatible with the alignment constraint</a>
            *         in the provided layout
            * @throws IndexOutOfBoundsException if {@code offset > byteSize() - layout.byteSize()}
      -     * @throws UnsupportedOperationException if this segment is
      +     * @throws IllegalArgumentException if this segment is
            *         {@linkplain #isReadOnly() read-only}
            */
           void set(ValueLayout.OfChar layout, long offset, char value);
      @@ -1719,7 +1723,7 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            *         <a href="MemorySegment.html#segment-alignment">incompatible with the alignment constraint</a>
            *         in the provided layout
            * @throws IndexOutOfBoundsException if {@code offset > byteSize() - layout.byteSize()}
      -     * @throws UnsupportedOperationException if this segment is
      +     * @throws IllegalArgumentException if this segment is
            *         {@linkplain #isReadOnly() read-only}
            */
           void set(ValueLayout.OfShort layout, long offset, short value);
      @@ -1757,7 +1761,7 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            *         <a href="MemorySegment.html#segment-alignment">incompatible with the alignment constraint</a>
            *         in the provided layout
            * @throws IndexOutOfBoundsException if {@code offset > byteSize() - layout.byteSize()}
      -     * @throws UnsupportedOperationException if this segment is
      +     * @throws IllegalArgumentException if this segment is
            *         {@linkplain #isReadOnly() read-only}
            */
           void set(ValueLayout.OfInt layout, long offset, int value);
      @@ -1795,7 +1799,7 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            *         <a href="MemorySegment.html#segment-alignment">incompatible with the alignment constraint</a>
            *         in the provided layout
            * @throws IndexOutOfBoundsException if {@code offset > byteSize() - layout.byteSize()}
      -     * @throws UnsupportedOperationException if this segment is
      +     * @throws IllegalArgumentException if this segment is
            *         {@linkplain #isReadOnly() read-only}
            */
           void set(ValueLayout.OfFloat layout, long offset, float value);
      @@ -1833,7 +1837,7 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            *         <a href="MemorySegment.html#segment-alignment">incompatible with the alignment constraint</a>
            *         in the provided layout
            * @throws IndexOutOfBoundsException if {@code offset > byteSize() - layout.byteSize()}
      -     * @throws UnsupportedOperationException if this segment is
      +     * @throws IllegalArgumentException if this segment is
            *         {@linkplain #isReadOnly() read-only}
            */
           void set(ValueLayout.OfLong layout, long offset, long value);
      @@ -1871,7 +1875,7 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            *         <a href="MemorySegment.html#segment-alignment">incompatible with the alignment constraint</a>
            *         in the provided layout
            * @throws IndexOutOfBoundsException if {@code offset > byteSize() - layout.byteSize()}
      -     * @throws UnsupportedOperationException if this segment is
      +     * @throws IllegalArgumentException if this segment is
            *         {@linkplain #isReadOnly() read-only}
            */
           void set(ValueLayout.OfDouble layout, long offset, double value);
      @@ -1921,8 +1925,10 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            * @throws IndexOutOfBoundsException if {@code offset > byteSize() - layout.byteSize()}
            * @throws UnsupportedOperationException if this segment is
            *         {@linkplain #isReadOnly() read-only}
      -     * @throws UnsupportedOperationException if {@code value} is not a
      +     * @throws IllegalArgumentException if {@code value} is not a
            *         {@linkplain #isNative() native} segment
      +     * @throws IllegalArgumentException if this segment is
      +     *         {@linkplain #isReadOnly() read-only}
            */
           void set(AddressLayout layout, long offset, MemorySegment value);
      
      @@ -2055,7 +2061,7 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            * @throws IllegalArgumentException if {@code layout.byteAlignment() > layout.byteSize()}
            * @throws IndexOutOfBoundsException if {@code index * layout.byteSize()} overflows
            * @throws IndexOutOfBoundsException if {@code index * layout.byteSize() > byteSize() - layout.byteSize()}
      -     * @throws UnsupportedOperationException if this segment is {@linkplain #isReadOnly() read-only}
      +     * @throws IllegalArgumentException if this segment is {@linkplain #isReadOnly() read-only}
            */
           void setAtIndex(ValueLayout.OfByte layout, long index, byte value);
      
      @@ -2078,7 +2084,7 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            * @throws IllegalArgumentException if {@code layout.byteAlignment() > layout.byteSize()}
            * @throws IndexOutOfBoundsException if {@code index * layout.byteSize()} overflows
            * @throws IndexOutOfBoundsException if {@code index * layout.byteSize() > byteSize() - layout.byteSize()}
      -     * @throws UnsupportedOperationException if this segment is {@linkplain #isReadOnly() read-only}
      +     * @throws IllegalArgumentException if this segment is {@linkplain #isReadOnly() read-only}
            */
           void setAtIndex(ValueLayout.OfBoolean layout, long index, boolean value);
      
      @@ -2101,7 +2107,7 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            * @throws IllegalArgumentException if {@code layout.byteAlignment() > layout.byteSize()}
            * @throws IndexOutOfBoundsException if {@code index * layout.byteSize()} overflows
            * @throws IndexOutOfBoundsException if {@code index * layout.byteSize() > byteSize() - layout.byteSize()}
      -     * @throws UnsupportedOperationException if this segment is {@linkplain #isReadOnly() read-only}
      +     * @throws IllegalArgumentException if this segment is {@linkplain #isReadOnly() read-only}
            */
           void setAtIndex(ValueLayout.OfShort layout, long index, short value);
      
      @@ -2146,7 +2152,7 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            * @throws IllegalArgumentException if {@code layout.byteAlignment() > layout.byteSize()}
            * @throws IndexOutOfBoundsException if {@code index * layout.byteSize()} overflows
            * @throws IndexOutOfBoundsException if {@code index * layout.byteSize() > byteSize() - layout.byteSize()}
      -     * @throws UnsupportedOperationException if this segment is {@linkplain #isReadOnly() read-only}
      +     * @throws IllegalArgumentException if this segment is {@linkplain #isReadOnly() read-only}
            */
           void setAtIndex(ValueLayout.OfInt layout, long index, int value);
      
      @@ -2191,7 +2197,7 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            * @throws IllegalArgumentException if {@code layout.byteAlignment() > layout.byteSize()}
            * @throws IndexOutOfBoundsException if {@code index * layout.byteSize()} overflows
            * @throws IndexOutOfBoundsException if {@code index * layout.byteSize() > byteSize() - layout.byteSize()}
      -     * @throws UnsupportedOperationException if this segment is {@linkplain #isReadOnly() read-only}
      +     * @throws IllegalArgumentException if this segment is {@linkplain #isReadOnly() read-only}
            */
           void setAtIndex(ValueLayout.OfFloat layout, long index, float value);
      
      @@ -2236,7 +2242,7 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            * @throws IllegalArgumentException if {@code layout.byteAlignment() > layout.byteSize()}
            * @throws IndexOutOfBoundsException if {@code index * layout.byteSize()} overflows
            * @throws IndexOutOfBoundsException if {@code index * layout.byteSize() > byteSize() - layout.byteSize()}
      -     * @throws UnsupportedOperationException if this segment is {@linkplain #isReadOnly() read-only}
      +     * @throws IllegalArgumentException if this segment is {@linkplain #isReadOnly() read-only}
            */
           void setAtIndex(ValueLayout.OfLong layout, long index, long value);
      
      @@ -2281,7 +2287,7 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            * @throws IllegalArgumentException if {@code layout.byteAlignment() > layout.byteSize()}
            * @throws IndexOutOfBoundsException if {@code index * layout.byteSize()} overflows
            * @throws IndexOutOfBoundsException if {@code index * layout.byteSize() > byteSize() - layout.byteSize()}
      -     * @throws UnsupportedOperationException if this segment is {@linkplain #isReadOnly() read-only}
      +     * @throws IllegalArgumentException if this segment is {@linkplain #isReadOnly() read-only}
            */
           void setAtIndex(ValueLayout.OfDouble layout, long index, double value);
      
      @@ -2336,7 +2342,9 @@ static void copy(MemorySegment srcSegment, ValueLayout srcElementLayout, long sr
            * @throws IndexOutOfBoundsException if {@code index * layout.byteSize()} overflows
            * @throws IndexOutOfBoundsException if {@code index * layout.byteSize() > byteSize() - layout.byteSize()}
            * @throws UnsupportedOperationException if this segment is {@linkplain #isReadOnly() read-only}
      -     * @throws UnsupportedOperationException if {@code value} is not a {@linkplain #isNative() native} segment
      +     * @throws IllegalArgumentException if {@code value} is not a {@linkplain #isNative() native} segment
      +     * @throws IllegalArgumentException if this segment is
      +     *         {@linkplain #isReadOnly() read-only}
            */
           void setAtIndex(AddressLayout layout, long index, MemorySegment value);
      
      @@ -2460,7 +2468,7 @@ static void copy(MemorySegment srcSegment, ValueLayout srcLayout, long srcOffset
            *         <a href="MemorySegment.html#segment-alignment">incompatible with the alignment constraint</a>
            *         in the source element layout
            * @throws IllegalArgumentException if {@code dstLayout.byteAlignment() > dstLayout.byteSize()}
      -     * @throws UnsupportedOperationException if {@code dstSegment} is {@linkplain #isReadOnly() read-only}
      +     * @throws IllegalArgumentException if {@code dstSegment} is {@linkplain #isReadOnly() read-only}
            * @throws IndexOutOfBoundsException if {@code elementCount * dstLayout.byteSize()} overflows
            * @throws IndexOutOfBoundsException if {@code dstOffset > dstSegment.byteSize() - (elementCount * dstLayout.byteSize())}
            * @throws IndexOutOfBoundsException if {@code srcIndex > srcArray.length - elementCount}
      diff --git a/src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java b/src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java
      index a4977d62d007..6be9d949ea65 100644
      --- a/src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java
      +++ b/src/java.base/share/classes/java/lang/foreign/SegmentAllocator.java
      @@ -350,7 +350,7 @@ default MemorySegment allocateFrom(ValueLayout.OfDouble layout, double value) {
            *
            * @param layout the layout of the block of memory to be allocated
            * @param value  the value to be set in the newly allocated memory segment
      -     * @throws UnsupportedOperationException if {@code value} is not
      +     * @throws IllegalArgumentException if {@code value} is not
            *         a {@linkplain MemorySegment#isNative() native} segment
            */
           default MemorySegment allocateFrom(AddressLayout layout, MemorySegment value) {
      @@ -670,9 +670,11 @@ default MemorySegment allocate(long byteSize) {
            *
            * @param segment the segment from which the returned allocator should slice from
            * @return a new slicing allocator
      +     * @throws IllegalArgumentException if the {@code segment} is
      +     *         {@linkplain MemorySegment#isReadOnly() read-only}
            */
      
      
      @@ -700,9 +702,19 @@ static SegmentAllocator slicingAllocator(MemorySegment segment) {
            * @param segment the memory segment to be recycled by the returned allocator
            * @return an allocator that recycles an existing segment upon each new
            *         allocation request
      +     * @throws IllegalArgumentException if the {@code segment} is
      +     *         {@linkplain MemorySegment#isReadOnly() read-only}
            */

            pminborg Per-Ake Minborg
            kganapureddy Krushnareddy Ganapureddy
            Maurizio Cimadamore
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated:
              Resolved: