GH-43479: [Java] Change visibility of MemoryUtil.UNSAFE (#43480)

### Rationale for this change `MemoryUtil.UNSAFE` field is a public field which provides unrestricted access to `sun.misc.Unsafe` instance which may cause misusage and possibly JVM crashes. ### What changes are included in this PR? Make the field (and other related fields) private and only allow indirect use of Unsafe through `MemoryUtil` methods ### Are these changes tested? Yes. ### Are there any user-facing changes? No **This PR includes breaking changes to public APIs.** Code using `MemoryUtil.UNSAFE` would break as the visibility of the field was changed to private * GitHub Issue: #43479 Authored-by: Laurent Goujon <[email protected]> Signed-off-by: Dane Pitkin <[email protected]>
apache · Jul 30, 2024 · 6bba203 · 6bba203
1 parent 48782e7
commit 6bba203
Show file tree

Hide file tree

Showing 20 changed files with 218 additions and 201 deletions.
diff --git a/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/consumer/ClobConsumer.java b/java/adapter/jdbc/src/main/java/org/apache/arrow/adapter/jdbc/consumer/ClobConsumer.java
@@ -86,12 +86,8 @@ public void consume(ResultSet resultSet) throws SQLException {
             while ((dataBuffer.writerIndex() + bytes.length) > dataBuffer.capacity()) {
               vector.reallocDataBuffer();
             }
-            MemoryUtil.UNSAFE.copyMemory(
-                bytes,
-                MemoryUtil.BYTE_ARRAY_BASE_OFFSET,
-                null,
-                dataBuffer.memoryAddress() + startIndex + totalBytes,
-                bytes.length);
+            MemoryUtil.copyToMemory(
+                bytes, 0, dataBuffer.memoryAddress() + startIndex + totalBytes, bytes.length);
 
             totalBytes += bytes.length;
             read += readSize;
@@ -133,12 +129,8 @@ public void consume(ResultSet resultSet) throws SQLException {
           while ((dataBuffer.writerIndex() + bytes.length) > dataBuffer.capacity()) {
             vector.reallocDataBuffer();
           }
-          MemoryUtil.UNSAFE.copyMemory(
-              bytes,
-              MemoryUtil.BYTE_ARRAY_BASE_OFFSET,
-              null,
-              dataBuffer.memoryAddress() + startIndex + totalBytes,
-              bytes.length);
+          MemoryUtil.copyToMemory(
+              bytes, 0, dataBuffer.memoryAddress() + startIndex + totalBytes, bytes.length);
 
           totalBytes += bytes.length;
           read += readSize;

diff --git a/...rithm/src/main/java/org/apache/arrow/algorithm/sort/FixedWidthOutOfPlaceVectorSorter.java b/...rithm/src/main/java/org/apache/arrow/algorithm/sort/FixedWidthOutOfPlaceVectorSorter.java
@@ -77,7 +77,7 @@ public void sortOutOfPlace(V srcVector, V dstVector, VectorValueComparator<V> co
           BitVectorHelper.unsetBit(dstValidityBuffer, dstIndex);
         } else {
           BitVectorHelper.setBit(dstValidityBuffer, dstIndex);
-          MemoryUtil.UNSAFE.copyMemory(
+          MemoryUtil.copyMemory(
               srcValueBuffer.memoryAddress() + srcIndex * ((long) valueWidth),
               dstValueBuffer.memoryAddress() + dstIndex * ((long) valueWidth),
               valueWidth);

diff --git a/...hm/src/main/java/org/apache/arrow/algorithm/sort/VariableWidthOutOfPlaceVectorSorter.java b/...hm/src/main/java/org/apache/arrow/algorithm/sort/VariableWidthOutOfPlaceVectorSorter.java
@@ -91,7 +91,7 @@ public void sortOutOfPlace(V srcVector, V dstVector, VectorValueComparator<V> co
           int valueLength =
               srcOffsetBuffer.getInt((srcIndex + 1) * ((long) BaseVariableWidthVector.OFFSET_WIDTH))
                   - srcOffset;
-          MemoryUtil.UNSAFE.copyMemory(
+          MemoryUtil.copyMemory(
               srcValueBuffer.memoryAddress() + srcOffset,
               dstValueBuffer.memoryAddress() + dstOffset,
               valueLength);

diff --git a/java/c/src/test/java/org/apache/arrow/c/ArrowArrayUtilityTest.java b/java/c/src/test/java/org/apache/arrow/c/ArrowArrayUtilityTest.java
@@ -103,14 +103,14 @@ allocator, dummyHandle, new ArrowFieldNode(/* length= */ 0, 0), new long[] {0}))
   @Test
   void cleanupAfterFailure() throws Exception {
     // Note values are all dummy values here
-    long address = MemoryUtil.UNSAFE.allocateMemory(16);
+    long address = MemoryUtil.allocateMemory(16);
     try (BufferImportTypeVisitor visitor =
         new BufferImportTypeVisitor(
             allocator, dummyHandle, new ArrowFieldNode(0, 0), new long[] {address})) {
       // This fails, but only after we've already imported a buffer.
       assertThrows(IllegalStateException.class, () -> visitor.visit(new ArrowType.Int(32, true)));
     } finally {
-      MemoryUtil.UNSAFE.freeMemory(address);
+      MemoryUtil.freeMemory(address);
     }
   }
 
@@ -119,7 +119,7 @@ void bufferAssociatedWithAllocator() throws Exception {
     // Note values are all dummy values here
     final long bufferSize = 16;
     final long fieldLength = bufferSize / IntVector.TYPE_WIDTH;
-    long address = MemoryUtil.UNSAFE.allocateMemory(bufferSize);
+    long address = MemoryUtil.allocateMemory(bufferSize);
     long baseline = allocator.getAllocatedMemory();
     ArrowFieldNode fieldNode = new ArrowFieldNode(fieldLength, 0);
     try (BufferImportTypeVisitor visitor =
@@ -134,7 +134,7 @@ void bufferAssociatedWithAllocator() throws Exception {
           .isEqualTo(allocator);
       assertThat(allocator.getAllocatedMemory()).isEqualTo(baseline + bufferSize);
     } finally {
-      MemoryUtil.UNSAFE.freeMemory(address);
+      MemoryUtil.freeMemory(address);
     }
     assertThat(allocator.getAllocatedMemory()).isEqualTo(baseline);
   }
@@ -161,7 +161,7 @@ void releaseRetain() {
   @Test
   void associate() {
     final long bufferSize = 16;
-    final long address = MemoryUtil.UNSAFE.allocateMemory(bufferSize);
+    final long address = MemoryUtil.allocateMemory(bufferSize);
     try {
       ArrowArray array = ArrowArray.allocateNew(allocator);
       ReferenceCountedArrowArray handle = new ReferenceCountedArrowArray(array);
@@ -173,7 +173,7 @@ void associate() {
       handle.release();
       assertThat(array.isClosed()).isTrue();
     } finally {
-      MemoryUtil.UNSAFE.freeMemory(address);
+      MemoryUtil.freeMemory(address);
     }
   }
 }