8246282: [REDO] JDK-8245121 (bf) XBuffer.put(Xbuffer src) can give unexpected result when storage overlaps

Reviewed-by: psandoz, alanb
This commit is contained in:
Brian Burkhalter 2020-06-04 11:39:39 -07:00
parent dd016c34dd
commit 9cadf1a004
5 changed files with 467 additions and 71 deletions

@ -1,5 +1,5 @@
/*
* Copyright (c) 2000, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2000, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -409,44 +409,7 @@ class Direct$Type$Buffer$RW$$BO$
public $Type$Buffer put($Type$Buffer src) {
#if[rw]
checkSegment();
if (src instanceof Direct$Type$Buffer$BO$) {
if (src == this)
throw createSameBufferException();
Direct$Type$Buffer$RW$$BO$ sb = (Direct$Type$Buffer$RW$$BO$)src;
int spos = sb.position();
int slim = sb.limit();
assert (spos <= slim);
int srem = (spos <= slim ? slim - spos : 0);
int pos = position();
int lim = limit();
assert (pos <= lim);
int rem = (pos <= lim ? lim - pos : 0);
if (srem > rem)
throw new BufferOverflowException();
try {
UNSAFE.copyMemory(sb.ix(spos), ix(pos), (long)srem << $LG_BYTES_PER_VALUE$);
} finally {
Reference.reachabilityFence(sb);
Reference.reachabilityFence(this);
}
sb.position(spos + srem);
position(pos + srem);
} else if (src.hb != null) {
int spos = src.position();
int slim = src.limit();
assert (spos <= slim);
int srem = (spos <= slim ? slim - spos : 0);
put(src.hb, src.offset + spos, srem);
src.position(spos + srem);
} else {
super.put(src);
}
super.put(src);
return this;
#else[rw]
throw new ReadOnlyBufferException();

@ -1,5 +1,5 @@
/*
* Copyright (c) 2000, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2000, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -47,7 +47,7 @@ class Heap$Type$Buffer$RW$
// Cached array base offset
private static final long ARRAY_BASE_OFFSET = UNSAFE.arrayBaseOffset($type$[].class);
// Cached array base offset
// Cached array index scale
private static final long ARRAY_INDEX_SCALE = UNSAFE.arrayIndexScale($type$[].class);
// For speed these fields are actually declared in X-Buffer;
@ -244,29 +244,7 @@ class Heap$Type$Buffer$RW$
public $Type$Buffer put($Type$Buffer src) {
#if[rw]
checkSegment();
if (src instanceof Heap$Type$Buffer) {
if (src == this)
throw createSameBufferException();
Heap$Type$Buffer sb = (Heap$Type$Buffer)src;
int pos = position();
int sbpos = sb.position();
int n = sb.limit() - sbpos;
if (n > limit() - pos)
throw new BufferOverflowException();
System.arraycopy(sb.hb, sb.ix(sbpos),
hb, ix(pos), n);
sb.position(sbpos + n);
position(pos + n);
} else if (src.isDirect()) {
int n = src.remaining();
int pos = position();
if (n > limit() - pos)
throw new BufferOverflowException();
src.get(hb, ix(pos), n);
position(pos + n);
} else {
super.put(src);
}
super.put(src);
return this;
#else[rw]
throw new ReadOnlyBufferException();

@ -1,5 +1,5 @@
/*
* Copyright (c) 2000, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2000, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@ -145,6 +145,10 @@ class StringCharBuffer // package-private
return null;
}
boolean isAddressable() {
return false;
}
public boolean equals(Object ob) {
if (this == ob)
return true;

@ -30,6 +30,7 @@ package java.nio;
#if[char]
import java.io.IOException;
#end[char]
import java.lang.ref.Reference;
#if[streamableType]
import java.util.Spliterator;
import java.util.stream.StreamSupport;
@ -116,7 +117,7 @@ import jdk.internal.util.ArraysSupport;
* obvious. It is therefore recommended that direct buffers be allocated
* primarily for large, long-lived buffers that are subject to the underlying
* system's native I/O operations. In general it is best to allocate direct
* buffers only when they yield a measureable gain in program performance.
* buffers only when they yield a measurable gain in program performance.
*
* <p> A direct byte buffer may also be created by {@link
* java.nio.channels.FileChannel#map mapping} a region of a file
@ -922,7 +923,10 @@ public abstract class $Type$Buffer
* dst.put(src.get()); </pre>
*
* except that it first checks that there is sufficient space in this
* buffer and it is potentially much more efficient.
* buffer and it is potentially much more efficient. If this buffer and
* the source buffer share the same backing array or memory, then the
* result will be as if the source elements were first copied to an
* intermediate location before being written into this buffer.
*
* @param src
* The source buffer from which $type$s are to be read;
@ -945,11 +949,67 @@ public abstract class $Type$Buffer
throw createSameBufferException();
if (isReadOnly())
throw new ReadOnlyBufferException();
int n = src.remaining();
if (n > remaining())
int srcPos = src.position();
int n = src.limit() - srcPos;
int pos = position();
if (n > limit() - pos)
throw new BufferOverflowException();
for (int i = 0; i < n; i++)
put(src.get());
Object srcBase = src.base();
#if[char]
if (src.isAddressable()) {
#else[char]
assert srcBase != null || src.isDirect();
#end[char]
Object base = base();
assert base != null || isDirect();
long srcAddr = src.address + ((long)srcPos << $LG_BYTES_PER_VALUE$);
long addr = address + ((long)pos << $LG_BYTES_PER_VALUE$);
long len = (long)n << $LG_BYTES_PER_VALUE$;
#if[!byte]
if (this.order() == src.order()) {
#end[!byte]
try {
UNSAFE.copyMemory(srcBase,
srcAddr,
base,
addr,
len);
} finally {
Reference.reachabilityFence(src);
Reference.reachabilityFence(this);
}
#if[!byte]
} else {
try {
UNSAFE.copySwapMemory(srcBase,
srcAddr,
base,
addr,
len,
(long)1 << $LG_BYTES_PER_VALUE$);
} finally {
Reference.reachabilityFence(src);
Reference.reachabilityFence(this);
}
}
#end[!byte]
position(pos + n);
src.position(srcPos + n);
#if[char]
} else { // src.isAddressable() == false
assert StringCharBuffer.class.isInstance(src);
for (int i = 0; i < n; i++)
put(src.get());
}
#end[char]
return this;
}
@ -1437,6 +1497,20 @@ public abstract class $Type$Buffer
*/
public abstract boolean isDirect();
#if[char]
/**
* Tells whether this buffer has addressable memory, e.g., a Java array or
* a native address. This method returns {@code true}. Subclasses such as
* {@code StringCharBuffer}, which wraps a {@code CharSequence}, should
* override this method to return {@code false}.
*
* @return {@code true} if, and only, this buffer has addressable memory
*/
boolean isAddressable() {
return true;
}
#end[char]
#if[!char]
/**

@ -0,0 +1,377 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.nio.Buffer;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
import java.nio.CharBuffer;
import java.nio.DoubleBuffer;
import java.nio.FloatBuffer;
import java.nio.IntBuffer;
import java.nio.LongBuffer;
import java.nio.ReadOnlyBufferException;
import java.nio.ShortBuffer;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Random;
import org.testng.Assert;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
/*
* @test
* @bug 8245121
* @summary Ensure that a bulk put of a buffer into another is correct.
* @compile --enable-preview -source ${jdk.version} BulkPutBuffer.java
* @run testng/othervm --enable-preview BulkPutBuffer
*/
public class BulkPutBuffer {
static final long SEED = System.nanoTime();
static final MyRandom RND = new MyRandom(SEED);
static final int ITERATIONS = 100;
static final int MAX_CAPACITY = 1024;
static class MyRandom extends Random {
MyRandom(long seed) {
super(seed);
}
public byte nextByte() {
return (byte)next(8);
}
public char nextChar() {
return (char)next(16);
}
public short nextShort() {
return (short)next(16);
}
}
enum BufferKind {
HEAP,
HEAP_VIEW,
DIRECT,
STRING;
}
static final Map<Class<?>,TypeAttr> typeToAttr;
static record TypeAttr(Class<?> type, int bytes, String name) {}
static {
typeToAttr = Map.of(
byte.class, new TypeAttr(ByteBuffer.class, Byte.BYTES, "Byte"),
char.class, new TypeAttr(CharBuffer.class, Character.BYTES, "Char"),
short.class, new TypeAttr(ShortBuffer.class, Short.BYTES, "Short"),
int.class, new TypeAttr(IntBuffer.class, Integer.BYTES, "Int"),
float.class, new TypeAttr(FloatBuffer.class, Float.BYTES, "Float"),
long.class, new TypeAttr(LongBuffer.class, Long.BYTES, "Long"),
double.class, new TypeAttr(DoubleBuffer.class, Double.BYTES, "Double")
);
}
static BufferKind[] getKinds(Class<?> elementType) {
BufferKind[] kinds;
if (elementType == byte.class)
kinds = new BufferKind[] {
BufferKind.DIRECT,
BufferKind.HEAP
};
else if (elementType == char.class)
kinds = BufferKind.values();
else
kinds = new BufferKind[] {
BufferKind.DIRECT,
BufferKind.HEAP,
BufferKind.HEAP_VIEW
};
return kinds;
}
static ByteOrder[] getOrders(BufferKind kind, Class<?> elementType) {
switch (kind) {
case HEAP:
return new ByteOrder[] { ByteOrder.nativeOrder() };
default:
if (elementType == byte.class)
return new ByteOrder[] { ByteOrder.nativeOrder() };
else
return new ByteOrder[] { ByteOrder.BIG_ENDIAN,
ByteOrder.LITTLE_ENDIAN };
}
}
public static class BufferProxy {
final Class<?> elementType;
final BufferKind kind;
final ByteOrder order;
// Buffer methods
MethodHandle alloc;
MethodHandle allocBB;
MethodHandle allocDirect;
MethodHandle asTypeBuffer;
MethodHandle putAbs;
MethodHandle getAbs;
MethodHandle putBufRel;
MethodHandle equals;
// MyRandom method
MethodHandle nextType;
BufferProxy(Class<?> elementType, BufferKind kind, ByteOrder order) {
this.elementType = elementType;
this.kind = kind;
this.order = order;
Class<?> bufferType = typeToAttr.get(elementType).type;
var lookup = MethodHandles.lookup();
try {
String name = typeToAttr.get(elementType).name;
alloc = lookup.findStatic(bufferType, "allocate",
MethodType.methodType(bufferType, int.class));
allocBB = lookup.findStatic(ByteBuffer.class, "allocate",
MethodType.methodType(ByteBuffer.class, int.class));
allocDirect = lookup.findStatic(ByteBuffer.class, "allocateDirect",
MethodType.methodType(ByteBuffer.class, int.class));
if (elementType != byte.class) {
asTypeBuffer = lookup.findVirtual(ByteBuffer.class,
"as" + name + "Buffer", MethodType.methodType(bufferType));
}
putAbs = lookup.findVirtual(bufferType, "put",
MethodType.methodType(bufferType, int.class, elementType));
getAbs = lookup.findVirtual(bufferType, "get",
MethodType.methodType(elementType, int.class));
putBufRel = lookup.findVirtual(bufferType, "put",
MethodType.methodType(bufferType, bufferType));
equals = lookup.findVirtual(bufferType, "equals",
MethodType.methodType(boolean.class, Object.class));
nextType = lookup.findVirtual(MyRandom.class,
"next" + name, MethodType.methodType(elementType));
} catch (IllegalAccessException | NoSuchMethodException e) {
throw new AssertionError(e);
}
}
Buffer create(int capacity) throws Throwable {
Class<?> bufferType = typeToAttr.get(elementType).type;
try {
if (bufferType == ByteBuffer.class ||
kind == BufferKind.DIRECT || kind == BufferKind.HEAP_VIEW) {
int len = capacity*typeToAttr.get(elementType).bytes;
ByteBuffer bb = (ByteBuffer)allocBB.invoke(len);
byte[] bytes = new byte[len];
RND.nextBytes(bytes);
bb.put(0, bytes);
if (bufferType == ByteBuffer.class) {
return (Buffer)bb;
} else {
bb.order(order);
return (Buffer)asTypeBuffer.invoke(bb);
}
} else if (bufferType == CharBuffer.class &&
kind == BufferKind.STRING) {
char[] array = new char[capacity];
for (int i = 0; i < capacity; i++) {
array[i] = RND.nextChar();
}
return CharBuffer.wrap(new String(array));
} else {
Buffer buf = (Buffer)alloc.invoke(capacity);
for (int i = 0; i < capacity; i++) {
putAbs.invoke(buf, i, nextType.invoke(RND));
}
return buf;
}
} catch (Exception e) {
throw new AssertionError(e);
}
}
void copy(Buffer src, int srcOff, Buffer dst, int dstOff, int length)
throws Throwable {
try {
for (int i = 0; i < length; i++) {
putAbs.invoke(dst, dstOff + i, getAbs.invoke(src, srcOff + i));
}
} catch (ReadOnlyBufferException ro) {
throw ro;
} catch (Exception e) {
throw new AssertionError(e);
}
}
void put(Buffer src, Buffer dst) throws Throwable {
try {
putBufRel.invoke(dst, src);
} catch (ReadOnlyBufferException ro) {
throw ro;
} catch (Exception e) {
throw new AssertionError(e);
}
}
boolean equals(Buffer src, Buffer dst) throws Throwable {
try {
return Boolean.class.cast(equals.invoke(dst, src));
} catch (Exception e) {
throw new AssertionError(e);
}
}
}
static List<BufferProxy> getProxies(Class<?> type) {
List proxies = new ArrayList();
for (BufferKind kind : getKinds(type)) {
for (ByteOrder order : getOrders(kind, type)) {
proxies.add(new BufferProxy(type, kind, order));
}
}
return proxies;
}
@DataProvider
static Object[][] proxies() {
ArrayList<Object[]> args = new ArrayList<>();
for (Class<?> type : typeToAttr.keySet()) {
List<BufferProxy> proxies = getProxies(type);
for (BufferProxy proxy : proxies) {
args.add(new Object[] {proxy});
}
}
return args.toArray(Object[][]::new);
}
@DataProvider
static Object[][] proxyPairs() {
List<Object[]> args = new ArrayList<>();
for (Class<?> type : typeToAttr.keySet()) {
List<BufferProxy> proxies = getProxies(type);
for (BufferProxy proxy1 : proxies) {
for (BufferProxy proxy2 : proxies) {
args.add(new Object[] {proxy1, proxy2});
}
}
}
return args.toArray(Object[][]::new);
}
@Test(dataProvider = "proxies")
public static void testSelf(BufferProxy bp) throws Throwable {
for (int i = 0; i < ITERATIONS; i++) {
int cap = RND.nextInt(MAX_CAPACITY);
Buffer buf = bp.create(cap);
int lowerOffset = RND.nextInt(1 + cap/10);
int lowerLength = RND.nextInt(1 + cap/2);
if (lowerLength < 2)
continue;
Buffer lower = buf.slice(lowerOffset, lowerLength);
Buffer lowerCopy = bp.create(lowerLength);
if (lowerCopy.isReadOnly()) {
Assert.expectThrows(ReadOnlyBufferException.class,
() -> bp.copy(lower, 0, lowerCopy, 0, lowerLength));
break;
} else {
bp.copy(lower, 0, lowerCopy, 0, lowerLength);
}
int middleOffset = RND.nextInt(1 + cap/2);
Buffer middle = buf.slice(middleOffset, lowerLength);
if (middle.isReadOnly()) {
Assert.expectThrows(ReadOnlyBufferException.class,
() -> bp.put(lower, middle));
break;
} else {
bp.put(lower, middle);
}
middle.flip();
Assert.assertTrue(bp.equals(lowerCopy, middle),
String.format("%d %s %d %d %d %d%n", SEED,
buf.getClass().getName(), cap,
lowerOffset, lowerLength, middleOffset));
}
}
@Test(dataProvider = "proxyPairs")
public static void testPairs(BufferProxy bp, BufferProxy sbp) throws Throwable {
for (int i = 0; i < ITERATIONS; i++) {
int cap = Math.max(4, RND.nextInt(MAX_CAPACITY));
int cap2 = cap/2;
Buffer buf = bp.create(cap);
int pos = RND.nextInt(Math.max(1, cap2));
buf.position(pos);
buf.mark();
int lim = pos + Math.max(1, cap - pos);
buf.limit(lim);
int scap = Math.max(buf.remaining(), RND.nextInt(1024));
Buffer src = sbp.create(scap);
int diff = scap - buf.remaining();
int spos = diff > 0 ? RND.nextInt(diff) : 0;
src.position(spos);
src.mark();
int slim = spos + buf.remaining();
src.limit(slim);
if (buf.isReadOnly()) {
Assert.expectThrows(ReadOnlyBufferException.class,
() -> bp.put(src, buf));
break;
} else {
bp.put(src, buf);
}
buf.reset();
src.reset();
Assert.assertTrue(bp.equals(src, buf),
String.format("%d %s %d %d %d %s %d %d %d%n", SEED,
buf.getClass().getName(), cap, pos, lim,
src.getClass().getName(), scap, spos, slim));
}
}
}