Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent illegal access warnings on Java 9 #110

Merged
merged 9 commits into from Jan 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .travis.yml
Expand Up @@ -5,3 +5,6 @@ dist: trusty
jdk:
- oraclejdk8
- oraclejdk9
- openjdk10
- openjdk11
- openjdk12
19 changes: 18 additions & 1 deletion pom.xml
Expand Up @@ -13,7 +13,7 @@
<parent>
<groupId>io.airlift</groupId>
<artifactId>airbase</artifactId>
<version>83</version>
<version>88</version>
</parent>

<inceptionYear>2012</inceptionYear>
Expand Down Expand Up @@ -78,6 +78,23 @@
</dependencies>

<build>
<pluginManagement>
<plugins>
<!-- TODO: update these in Airbase -->
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<version>0.8.2</version>
</plugin>

<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
<version>3.1.10</version>
</plugin>
</plugins>
</pluginManagement>

<plugins>
<plugin>
<groupId>com.mycila</groupId>
Expand Down
32 changes: 8 additions & 24 deletions src/main/java/io/airlift/slice/JvmUtils.java
Expand Up @@ -15,14 +15,10 @@

import sun.misc.Unsafe;

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.nio.Buffer;
import java.nio.ByteBuffer;

import static io.airlift.slice.Preconditions.checkArgument;
import static sun.misc.Unsafe.ARRAY_BOOLEAN_INDEX_SCALE;
import static sun.misc.Unsafe.ARRAY_BYTE_INDEX_SCALE;
import static sun.misc.Unsafe.ARRAY_DOUBLE_INDEX_SCALE;
Expand All @@ -34,9 +30,8 @@
final class JvmUtils
{
static final Unsafe unsafe;
static final MethodHandle newByteBuffer;

private static final Field ADDRESS_ACCESSOR;
private static final long ADDRESS_OFFSET;

static {
try {
Expand All @@ -57,17 +52,10 @@ final class JvmUtils
assertArrayIndexScale("Float", ARRAY_FLOAT_INDEX_SCALE, 4);
assertArrayIndexScale("Double", ARRAY_DOUBLE_INDEX_SCALE, 8);

// fetch a method handle for the hidden constructor for DirectByteBuffer
Class<?> directByteBufferClass = ClassLoader.getSystemClassLoader().loadClass("java.nio.DirectByteBuffer");
Constructor<?> constructor = directByteBufferClass.getDeclaredConstructor(long.class, int.class, Object.class);
constructor.setAccessible(true);
newByteBuffer = MethodHandles.lookup().unreflectConstructor(constructor)
.asType(MethodType.methodType(ByteBuffer.class, long.class, int.class, Object.class));

ADDRESS_ACCESSOR = Buffer.class.getDeclaredField("address");
ADDRESS_ACCESSOR.setAccessible(true);
// fetch the address field for direct buffers
ADDRESS_OFFSET = unsafe.objectFieldOffset(Buffer.class.getDeclaredField("address"));
}
catch (Exception e) {
catch (ReflectiveOperationException e) {
throw new RuntimeException(e);
}
}
Expand All @@ -79,14 +67,10 @@ private static void assertArrayIndexScale(final String name, int actualIndexScal
}
}

public static long getAddress(Buffer buffer)
static long bufferAddress(Buffer buffer)
{
try {
return (long) ADDRESS_ACCESSOR.get(buffer);
}
catch (IllegalAccessException e) {
throw new RuntimeException(e);
}
checkArgument(buffer.isDirect(), "buffer is not direct");
return unsafe.getLong(buffer, ADDRESS_OFFSET);
}

private JvmUtils() {}
Expand Down
32 changes: 13 additions & 19 deletions src/main/java/io/airlift/slice/Slice.java
Expand Up @@ -24,7 +24,7 @@
import java.nio.ByteBuffer;
import java.nio.charset.Charset;

import static io.airlift.slice.JvmUtils.newByteBuffer;
import static io.airlift.slice.JvmUtils.bufferAddress;
import static io.airlift.slice.JvmUtils.unsafe;
import static io.airlift.slice.Preconditions.checkArgument;
import static io.airlift.slice.Preconditions.checkPositionIndexes;
Expand All @@ -41,9 +41,9 @@
import static io.airlift.slice.SizeOf.sizeOfIntArray;
import static io.airlift.slice.SizeOf.sizeOfLongArray;
import static io.airlift.slice.SizeOf.sizeOfShortArray;
import static io.airlift.slice.StringDecoder.decodeString;
import static java.lang.Math.min;
import static java.lang.Math.multiplyExact;
import static java.lang.Math.toIntExact;
import static java.lang.String.format;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Objects.requireNonNull;
Expand Down Expand Up @@ -1194,8 +1194,7 @@ public String toString(int index, int length, Charset charset)
if (base instanceof byte[]) {
return new String((byte[]) base, (int) ((address - ARRAY_BYTE_BASE_OFFSET) + index), length, charset);
}
// direct memory can only be converted to a string using a ByteBuffer
return decodeString(toByteBuffer(index, length), charset);
return new String(getBytes(index, length), charset);
electrum marked this conversation as resolved.
Show resolved Hide resolved
}

public ByteBuffer toByteBuffer()
Expand All @@ -1208,24 +1207,19 @@ public ByteBuffer toByteBuffer(int index, int length)
checkIndexLength(index, length);

if (base instanceof byte[]) {
return ByteBuffer.wrap((byte[]) base, (int) ((address - ARRAY_BYTE_BASE_OFFSET) + index), length);
return ByteBuffer.wrap((byte[]) base, (int) ((address - ARRAY_BYTE_BASE_OFFSET) + index), length).slice();
electrum marked this conversation as resolved.
Show resolved Hide resolved
}

try {
return (ByteBuffer) newByteBuffer.invokeExact(address + index, length, (Object) reference);
}
catch (Throwable throwable) {
if (throwable instanceof Error) {
throw (Error) throwable;
}
if (throwable instanceof RuntimeException) {
throw (RuntimeException) throwable;
}
if (throwable instanceof InterruptedException) {
Thread.currentThread().interrupt();
}
throw new RuntimeException(throwable);
if ((reference instanceof ByteBuffer) && ((ByteBuffer) reference).isDirect()) {
ByteBuffer buffer = (ByteBuffer) reference;
int position = toIntExact(address - bufferAddress(buffer)) + index;
buffer = buffer.duplicate();
buffer.position(position);
buffer.limit(position + length);
return buffer.slice();
}

throw new UnsupportedOperationException("Conversion to ByteBuffer not supported for this Slice");
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/io/airlift/slice/Slices.java
Expand Up @@ -23,7 +23,7 @@
import java.nio.channels.FileChannel.MapMode;
import java.nio.charset.Charset;

import static io.airlift.slice.JvmUtils.getAddress;
import static io.airlift.slice.JvmUtils.bufferAddress;
import static io.airlift.slice.Preconditions.checkPositionIndexes;
import static java.lang.String.format;
import static java.nio.charset.StandardCharsets.UTF_8;
Expand Down Expand Up @@ -118,7 +118,7 @@ public static Slice copyOf(Slice slice, int offset, int length)
public static Slice wrappedBuffer(ByteBuffer buffer)
{
if (buffer.isDirect()) {
long address = getAddress(buffer);
long address = bufferAddress(buffer);
return new Slice(null, address + buffer.position(), buffer.remaining(), buffer.capacity(), buffer);
}

Expand Down
84 changes: 0 additions & 84 deletions src/main/java/io/airlift/slice/StringDecoder.java

This file was deleted.

54 changes: 54 additions & 0 deletions src/test/java/io/airlift/slice/TestSlice.java
Expand Up @@ -19,6 +19,7 @@
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
Expand Down Expand Up @@ -823,6 +824,59 @@ public static void assertIndexOf(Slice data, Slice pattern)
assertEquals(bruteForce, indexOf);
}

@Test
public void testToByteBuffer()
{
byte[] original = "hello world".getBytes(UTF_8);

Slice slice = allocate(original.length);
slice.setBytes(0, original);
assertEquals(slice.getBytes(), original);

assertEquals(getBytes(slice.toByteBuffer()), original);

assertToByteBuffer(slice, original);
}

@Test
public void testToByteBufferDirect()
{
byte[] original = "hello world".getBytes(UTF_8);

ByteBuffer buffer = ByteBuffer.allocateDirect(original.length + 5);
buffer.putShort((short) 0xABCD);
buffer.put(original);
buffer.flip().position(2);

Slice slice = Slices.wrappedBuffer(buffer);
assertEquals(slice.getBytes(), original);

assertToByteBuffer(slice, original);

// conversion does not depend on wrapped buffer position or limit
buffer.position(8).limit(12);
assertToByteBuffer(slice, original);
}

private static void assertToByteBuffer(Slice slice, byte[] original)
{
for (int index = 0; index < original.length; index++) {
for (int length = 0; length < (original.length - index); length++) {
byte[] actual = getBytes(slice.toByteBuffer(index, length));
byte[] expected = Arrays.copyOfRange(original, index, index + length);
assertEquals(actual, expected);
}
}
}

private static byte[] getBytes(ByteBuffer buffer)
{
assertEquals(buffer.position(), 0);
byte[] data = new byte[buffer.remaining()];
buffer.get(data);
return data;
}

private static List<Long> createRandomLongs(int count)
{
Random random = new Random();
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/io/airlift/slice/TestUnsafeSliceFactory.java
Expand Up @@ -19,7 +19,7 @@
import java.nio.ByteBuffer;
import java.security.Permission;

import static io.airlift.slice.JvmUtils.getAddress;
import static io.airlift.slice.JvmUtils.bufferAddress;
import static io.airlift.slice.JvmUtils.unsafe;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertTrue;
Expand Down Expand Up @@ -55,7 +55,7 @@ public void testRawAddressWithReference()
{
ByteBuffer buffer = ByteBuffer.allocateDirect(100);
assertTrue(buffer.isDirect());
long address = getAddress(buffer);
long address = bufferAddress(buffer);

UnsafeSliceFactory factory = UnsafeSliceFactory.getInstance();

Expand Down