You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by dg...@apache.org on 2018/11/19 09:41:24 UTC
ignite git commit: IGNITE-10192 Fixed
OptimizedMarshallerTest#testAllocationOverflow throws OOME instead of
expected IOE - Fixes #5400.
Repository: ignite
Updated Branches:
refs/heads/master 355ce6fe8 -> 96e2fddc9
IGNITE-10192 Fixed OptimizedMarshallerTest#testAllocationOverflow throws OOME instead of expected IOE - Fixes #5400.
Signed-off-by: Dmitriy Govorukhin <dm...@gmail.com>
Project: http://git-wip-us.apache.org/repos/asf/ignite/repo
Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/96e2fddc
Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/96e2fddc
Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/96e2fddc
Branch: refs/heads/master
Commit: 96e2fddc9b7c18ec11ab181ad45633f4041a9aa3
Parents: 355ce6f
Author: ibessonov <be...@gmail.com>
Authored: Mon Nov 19 12:41:05 2018 +0300
Committer: Dmitriy Govorukhin <dm...@gmail.com>
Committed: Mon Nov 19 12:41:05 2018 +0300
----------------------------------------------------------------------
.../internal/util/io/GridUnsafeDataOutput.java | 76 ++++++++++++--------
.../optimized/OptimizedMarshallerTest.java | 56 ++++++---------
2 files changed, 66 insertions(+), 66 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/ignite/blob/96e2fddc/modules/core/src/main/java/org/apache/ignite/internal/util/io/GridUnsafeDataOutput.java
----------------------------------------------------------------------
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/util/io/GridUnsafeDataOutput.java b/modules/core/src/main/java/org/apache/ignite/internal/util/io/GridUnsafeDataOutput.java
index ad94889..8b664ab 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/util/io/GridUnsafeDataOutput.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/util/io/GridUnsafeDataOutput.java
@@ -17,8 +17,10 @@
package org.apache.ignite.internal.util.io;
+import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.OutputStream;
+import java.util.Arrays;
import org.apache.ignite.internal.util.GridUnsafe;
import org.apache.ignite.internal.util.typedef.internal.S;
import org.apache.ignite.internal.util.typedef.internal.U;
@@ -37,8 +39,15 @@ import static org.apache.ignite.internal.util.GridUnsafe.SHORT_ARR_OFF;
* Data output based on {@code Unsafe} operations.
*/
public class GridUnsafeDataOutput extends OutputStream implements GridDataOutput {
+ /**
+ * Based on {@link ByteArrayOutputStream#MAX_ARRAY_SIZE} or many other similar constants in other classes.
+ * It's not safe to allocate more then this number of elements in byte array, because it can throw
+ * java.lang.OutOfMemoryError: Requested array size exceeds VM limit
+ */
+ private static final int MAX_BYTE_ARRAY_SIZE = Integer.MAX_VALUE - 8;
+
/** */
- private static final Long CHECK_FREQ = Long.getLong(IGNITE_MARSHAL_BUFFERS_RECHECK, 10000);
+ private static final long CHECK_FREQ = Long.getLong(IGNITE_MARSHAL_BUFFERS_RECHECK, 10000);
/** Length of char buffer (for writing strings). */
private static final int CHAR_BUF_SIZE = 256;
@@ -121,9 +130,8 @@ public class GridUnsafeDataOutput extends OutputStream implements GridDataOutput
* @param size Size.
*/
private void requestFreeSize(int size) throws IOException {
- // If arithmetic overflow occurs, off + size should be less than size.
- if (off + size < size)
- throw new IOException("Failed to allocate required memory (arithmetic overflow detected) " +
+ if (!canBeAllocated(off + size))
+ throw new IOException("Failed to allocate required memory (byte array size overflow detected) " +
"[length=" + size + ", offset=" + off + ']');
size = off + size;
@@ -133,22 +141,18 @@ public class GridUnsafeDataOutput extends OutputStream implements GridDataOutput
long now = U.currentTimeMillis();
if (size > bytes.length) {
- byte[] newBytes = new byte[Math.max(size << 1, size)]; // Grow.
+ int newSize = size << 1;
- System.arraycopy(bytes, 0, newBytes, 0, off);
+ if (!canBeAllocated(newSize))
+ newSize = MAX_BYTE_ARRAY_SIZE;
- bytes = newBytes;
+ bytes = Arrays.copyOf(bytes, newSize); // Grow.
}
else if (now - lastCheck > CHECK_FREQ) {
int halfSize = bytes.length >> 1;
- if (maxOff < halfSize) {
- byte[] newBytes = new byte[halfSize]; // Shrink.
-
- System.arraycopy(bytes, 0, newBytes, 0, off);
-
- bytes = newBytes;
- }
+ if (maxOff < halfSize)
+ bytes = Arrays.copyOf(bytes, halfSize); // Shrink.
maxOff = 0;
lastCheck = now;
@@ -156,6 +160,15 @@ public class GridUnsafeDataOutput extends OutputStream implements GridDataOutput
}
/**
+ * @param size Size of potential byte array to check.
+ * @return true if {@code new byte[size]} won't throw {@link OutOfMemoryError} given enough heap space.
+ * @see GridUnsafeDataOutput#MAX_BYTE_ARRAY_SIZE
+ */
+ private boolean canBeAllocated(long size) {
+ return 0 <= size && size <= MAX_BYTE_ARRAY_SIZE;
+ }
+
+ /**
* @param size Size.
* @throws IOException In case of error.
*/
@@ -188,9 +201,9 @@ public class GridUnsafeDataOutput extends OutputStream implements GridDataOutput
@Override public void writeDoubleArray(double[] arr) throws IOException {
writeInt(arr.length);
- int bytesToCp = arr.length << 3;
+ checkArrayAllocationOverflow(8, arr.length, "double");
- checkArrayAllocationOverflow(bytesToCp, arr.length, "double");
+ int bytesToCp = arr.length << 3;
requestFreeSize(bytesToCp);
@@ -222,9 +235,9 @@ public class GridUnsafeDataOutput extends OutputStream implements GridDataOutput
@Override public void writeCharArray(char[] arr) throws IOException {
writeInt(arr.length);
- int bytesToCp = arr.length << 1;
+ checkArrayAllocationOverflow(2, arr.length, "char");
- checkArrayAllocationOverflow(bytesToCp, arr.length, "char");
+ int bytesToCp = arr.length << 1;
requestFreeSize(bytesToCp);
@@ -247,9 +260,9 @@ public class GridUnsafeDataOutput extends OutputStream implements GridDataOutput
@Override public void writeLongArray(long[] arr) throws IOException {
writeInt(arr.length);
- int bytesToCp = arr.length << 3;
+ checkArrayAllocationOverflow(8, arr.length, "long");
- checkArrayAllocationOverflow(bytesToCp, arr.length, "long");
+ int bytesToCp = arr.length << 3;
requestFreeSize(bytesToCp);
@@ -272,9 +285,9 @@ public class GridUnsafeDataOutput extends OutputStream implements GridDataOutput
@Override public void writeFloatArray(float[] arr) throws IOException {
writeInt(arr.length);
- int bytesToCp = arr.length << 2;
+ checkArrayAllocationOverflow(4, arr.length, "float");
- checkArrayAllocationOverflow(bytesToCp, arr.length, "float");
+ int bytesToCp = arr.length << 2;
requestFreeSize(bytesToCp);
@@ -315,9 +328,9 @@ public class GridUnsafeDataOutput extends OutputStream implements GridDataOutput
@Override public void writeShortArray(short[] arr) throws IOException {
writeInt(arr.length);
- int bytesToCp = arr.length << 1;
+ checkArrayAllocationOverflow(2, arr.length, "short");
- checkArrayAllocationOverflow(bytesToCp, arr.length, "short");
+ int bytesToCp = arr.length << 1;
requestFreeSize(bytesToCp);
@@ -340,9 +353,9 @@ public class GridUnsafeDataOutput extends OutputStream implements GridDataOutput
@Override public void writeIntArray(int[] arr) throws IOException {
writeInt(arr.length);
- int bytesToCp = arr.length << 2;
+ checkArrayAllocationOverflow(4, arr.length, "int");
- checkArrayAllocationOverflow(bytesToCp, arr.length, "int");
+ int bytesToCp = arr.length << 2;
requestFreeSize(bytesToCp);
@@ -491,16 +504,17 @@ public class GridUnsafeDataOutput extends OutputStream implements GridDataOutput
/**
* Check for possible arithmetic overflow when trying to serialize a humongous array.
*
- * @param bytesToAlloc Bytes to allocate.
+ * @param bytes Number of bytes in a single array element.
* @param arrLen Array length.
* @param type Type of an array.
* @throws IOException If oveflow presents and data corruption can occur.
*/
- private void checkArrayAllocationOverflow(int bytesToAlloc, int arrLen, String type) throws IOException {
- // If arithmetic overflow occurs, bytesToAlloc should be less than arrLen.
- if (bytesToAlloc < arrLen)
+ private void checkArrayAllocationOverflow(int bytes, int arrLen, String type) throws IOException {
+ long bytesToAlloc = (long)arrLen * bytes;
+
+ if (!canBeAllocated(bytesToAlloc))
throw new IOException("Failed to allocate required memory for " + type + " array " +
- "(arithmetic overflow detected) [length=" + arrLen + ']');
+ "(byte array size overflow detected) [length=" + arrLen + ']');
}
/**
http://git-wip-us.apache.org/repos/asf/ignite/blob/96e2fddc/modules/core/src/test/java/org/apache/ignite/internal/marshaller/optimized/OptimizedMarshallerTest.java
----------------------------------------------------------------------
diff --git a/modules/core/src/test/java/org/apache/ignite/internal/marshaller/optimized/OptimizedMarshallerTest.java b/modules/core/src/test/java/org/apache/ignite/internal/marshaller/optimized/OptimizedMarshallerTest.java
index 7b1d221..94c854f 100644
--- a/modules/core/src/test/java/org/apache/ignite/internal/marshaller/optimized/OptimizedMarshallerTest.java
+++ b/modules/core/src/test/java/org/apache/ignite/internal/marshaller/optimized/OptimizedMarshallerTest.java
@@ -391,45 +391,28 @@ public class OptimizedMarshallerTest extends GridCommonAbstractTest {
/**
* Tests checks for arithmetic overflow when trying to serialize huge object.
* WARNING! Requires a lot of heap space. Should not be run on CI.
+ * Minimal memory requirement is about 6-7 gigabytes of heap.
*/
public void _testAllocationOverflow() {
allocationOverflowCheck(() -> marshaller().marshal(new HugeObject()));
- allocationOverflowCheck(() -> {
- marshaller().marshal(new short[1<<30]);
- marshaller().marshal(new short[1<<30]);
- return null;
- });
+ allocationOverflowCheck(() -> marshaller().marshal(new short[1 << 30]));
- allocationOverflowCheck(() -> {
- marshaller().marshal(new char[1<<30]);
- marshaller().marshal(new char[1<<30]);
- return null;
- });
+ allocationOverflowCheck(() -> marshaller().marshal(new char[1 << 30]));
- allocationOverflowCheck(() -> {
- marshaller().marshal(new int[1<<30]);
- marshaller().marshal(new int[1<<30]);
- return null;
- });
+ allocationOverflowCheck(() -> marshaller().marshal(new int[1 << 29]));
- allocationOverflowCheck(() -> {
- marshaller().marshal(new float[1<<29]);
- marshaller().marshal(new float[1<<29]);
- return null;
- });
+ allocationOverflowCheck(() -> marshaller().marshal(new float[1 << 29]));
- allocationOverflowCheck(() -> {
- marshaller().marshal(new long[1<<29]);
- marshaller().marshal(new long[1<<29]);
- return null;
- });
+ allocationOverflowCheck(() -> marshaller().marshal(new long[1 << 28]));
- allocationOverflowCheck(() -> {
- marshaller().marshal(new double[1<<29]);
- marshaller().marshal(new double[1<<29]);
- return null;
- });
+ allocationOverflowCheck(() -> marshaller().marshal(new double[1 << 28]));
+
+ // This particular case requires about 13G of heap space.
+ // It failed because of bug in previous implementation of GridUnsafeDataOutput, mainly line
+ // "if (bytesToAlloc < arrLen)" in method "checkArrayAllocationOverflow". That check doesn't
+ // work as desired on the length in the example below.
+ allocationOverflowCheck(() -> marshaller().marshal(new long[0x2800_0000]));
}
/**
@@ -437,8 +420,9 @@ public class OptimizedMarshallerTest extends GridCommonAbstractTest {
*
* @param call Callable that cause allocation overflow.
*/
+ @SuppressWarnings("ThrowableNotThrown")
private void allocationOverflowCheck(Callable<?> call) {
- GridTestUtils.assertThrowsAnyCause(log, call, IOException.class, "Impossible to allocate required memory");
+ GridTestUtils.assertThrowsAnyCause(log, call, IOException.class, "Failed to allocate required memory");
}
/**
@@ -448,10 +432,12 @@ public class OptimizedMarshallerTest extends GridCommonAbstractTest {
/** {@inheritDoc} */
@Override public void writeExternal(ObjectOutput out) throws IOException {
- out.write(new byte[1 << 31 - 2]);
- out.write(new byte[1 << 31 - 2]);
- out.write(new byte[1 << 31 - 2]);
- out.write(new byte[1 << 31 - 2]);
+ byte[] bytes = new byte[1 << 31 - 2];
+
+ out.write(bytes);
+ out.write(bytes);
+ out.write(bytes);
+ out.write(bytes);
}
/** {@inheritDoc} */