RFR: 8318966: Some methods make promises about Java array element alignment that are too strong

Jorn Vernee jvernee at openjdk.org
Thu Nov 16 19:23:44 UTC 2023


See the JBS issue for an extended problem description.

This patch changes the specification and implementation of `MethodHandles::byteArrayViewVarHandle`, `MethodHandles::byteBufferViewVarHandle`, `ByteBuffer::alignedSlice`, and `ByteBuffer::alignmentOffset` to weaken the guarantees they make about the alignment of Java array elements, in order to bring them in line with the guarantees made by an arbitrary JVM implementation (which makes no guarantees about array element alignment beyond them being aligned to their natural alignment).

- `MethodHandles::byteArrayViewVarHandle`: we can not guarantee any alignment for the accesses. Which means we can only reliably support plain get and set access modes. The javadoc text explaining which other access modes are supported, or how to compute aligned offsets into the array is dropped, as it is not guaranteed to be correct on all JVM implementations. The implementation of the returned VarHandle is changed to throw an `UnsupportedOperationException` for the unsupported access modes, as mandated by the spec of `VarHandle` [1].

- `MethodHandles::byteBufferViewVarHandle`: the implementation/spec is incorrect when accessing a heap buffer (wrapping a byte[]), for the same reasons as `byteArrayViewVarHandle`. The spec is changed to specify that when accessing a _heap buffer_, only plain get and set access modes are supported. The implementation of the returned var handle is changed to throw an `IllegalStateException` when an access is attempted on a heap buffer using an access mode other than plain get or set. Note that we don't throw an outright `UnsupportedOperationException` for this case, since whether the access modes are supported depends on the byte buffer instance being used.

- `ByteBuffer::alignedSlice` and `ByteBuffer::alignmentOffset`: The former method depends directly on the latter for all its alignment computations. We change the implementation of the latter method to throw an `UnsupportedOperationException` for all unit sizes greater than 1, when the buffer is non-direct. This change is largely covered by the existing specification:


     * @throws UnsupportedOperationException
     *         If the native platform does not guarantee stable alignment offset
     *         values for the given unit size when managing the memory regions
     *         of buffers of the same kind as this buffer (direct or
     *         non-direct).  For example, if garbage collection would result
     *         in the moving of a memory region covered by a non-direct buffer
     *         from one location to another and both locations have different
     *         alignment characteristics.


However, the `@implNote` mentions that an `UnsupportedOperationException` will be thrown for unit sizes greater than 8. This is updated to say unit sizes greater than 1.

Note that the testing code for these APIs is tightly coupled, so it is practically convenient to address all these issues together.

Testing: local `java/lang/invoke/VarHandles`, more TODO

[1]: https://github.com/openjdk/jdk/blob/ffa35d8cf181cfbcb54497e997dbd18a9b62b97e/src/java.base/share/classes/java/lang/invoke/VarHandle.java#L189-L191

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

Commit messages:
 - Add api note pointing to alternatives for client that require non-plain access
 - simplify spec for alignmentOffset and alignedSlice
 - Merge note about misaligned access in byteBufferViewVarHandle
 - updated alignedSlice implNote as well
 - updated alignedOffset implNote
 - Use ISE for bbvvh instead of UOE
 - restore some tests for direct buffers
 - fix BAVV and BBVV impl and tests
 - regen test files
 - fix BasicByte test

Changes: https://git.openjdk.org/jdk/pull/16681/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16681&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8318966
  Stats: 5133 lines in 19 files changed: 612 ins; 3440 del; 1081 mod
  Patch: https://git.openjdk.org/jdk/pull/16681.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16681/head:pull/16681

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


More information about the nio-dev mailing list