You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by rx...@apache.org on 2016/07/06 23:30:28 UTC
spark git commit: [SPARK-16021] Fill freed memory in test to help
catch correctness bugs
Repository: spark
Updated Branches:
refs/heads/master b8ebf63c1 -> 44c7c62bc
[SPARK-16021] Fill freed memory in test to help catch correctness bugs
## What changes were proposed in this pull request?
This patches `MemoryAllocator` to fill clean and freed memory with known byte values, similar to https://github.com/jemalloc/jemalloc/wiki/Use-Case:-Find-a-memory-corruption-bug . Memory filling is flag-enabled in test only by default.
## How was this patch tested?
Unit test that it's on in test.
cc sameeragarwal
Author: Eric Liang <ek...@databricks.com>
Closes #13983 from ericl/spark-16021.
Project: http://git-wip-us.apache.org/repos/asf/spark/repo
Commit: http://git-wip-us.apache.org/repos/asf/spark/commit/44c7c62b
Tree: http://git-wip-us.apache.org/repos/asf/spark/tree/44c7c62b
Diff: http://git-wip-us.apache.org/repos/asf/spark/diff/44c7c62b
Branch: refs/heads/master
Commit: 44c7c62bcfca74c82ffc4e3c53997fff47bfacac
Parents: b8ebf63
Author: Eric Liang <ek...@databricks.com>
Authored: Wed Jul 6 16:30:25 2016 -0700
Committer: Reynold Xin <rx...@databricks.com>
Committed: Wed Jul 6 16:30:25 2016 -0700
----------------------------------------------------------------------
.../main/java/org/apache/spark/unsafe/Platform.java | 4 ++++
.../spark/unsafe/memory/HeapMemoryAllocator.java | 10 +++++++++-
.../apache/spark/unsafe/memory/MemoryAllocator.java | 13 ++++++++++++-
.../org/apache/spark/unsafe/memory/MemoryBlock.java | 7 +++++++
.../spark/unsafe/memory/UnsafeMemoryAllocator.java | 9 ++++++++-
.../org/apache/spark/unsafe/PlatformUtilSuite.java | 16 ++++++++++++++++
project/SparkBuild.scala | 1 +
7 files changed, 57 insertions(+), 3 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/spark/blob/44c7c62b/common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java
----------------------------------------------------------------------
diff --git a/common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java b/common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java
index 77c8c39..a2ee45c 100644
--- a/common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java
+++ b/common/unsafe/src/main/java/org/apache/spark/unsafe/Platform.java
@@ -176,6 +176,10 @@ public final class Platform {
throw new IllegalStateException("unreachable");
}
+ public static void setMemory(Object object, long offset, long size, byte value) {
+ _UNSAFE.setMemory(object, offset, size, value);
+ }
+
public static void setMemory(long address, byte value, long size) {
_UNSAFE.setMemory(address, size, value);
}
http://git-wip-us.apache.org/repos/asf/spark/blob/44c7c62b/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/HeapMemoryAllocator.java
----------------------------------------------------------------------
diff --git a/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/HeapMemoryAllocator.java b/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/HeapMemoryAllocator.java
index 09847ce..3cd4264 100644
--- a/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/HeapMemoryAllocator.java
+++ b/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/HeapMemoryAllocator.java
@@ -24,6 +24,7 @@ import java.util.LinkedList;
import java.util.Map;
import org.apache.spark.unsafe.Platform;
+import org.apache.spark.unsafe.memory.MemoryAllocator;
/**
* A simple {@link MemoryAllocator} that can allocate up to 16GB using a JVM long primitive array.
@@ -64,12 +65,19 @@ public class HeapMemoryAllocator implements MemoryAllocator {
}
}
long[] array = new long[(int) ((size + 7) / 8)];
- return new MemoryBlock(array, Platform.LONG_ARRAY_OFFSET, size);
+ MemoryBlock memory = new MemoryBlock(array, Platform.LONG_ARRAY_OFFSET, size);
+ if (MemoryAllocator.MEMORY_DEBUG_FILL_ENABLED) {
+ memory.fill(MemoryAllocator.MEMORY_DEBUG_FILL_CLEAN_VALUE);
+ }
+ return memory;
}
@Override
public void free(MemoryBlock memory) {
final long size = memory.size();
+ if (MemoryAllocator.MEMORY_DEBUG_FILL_ENABLED) {
+ memory.fill(MemoryAllocator.MEMORY_DEBUG_FILL_FREED_VALUE);
+ }
if (shouldPool(size)) {
synchronized (this) {
LinkedList<WeakReference<MemoryBlock>> pool = bufferPoolsBySize.get(size);
http://git-wip-us.apache.org/repos/asf/spark/blob/44c7c62b/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/MemoryAllocator.java
----------------------------------------------------------------------
diff --git a/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/MemoryAllocator.java b/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/MemoryAllocator.java
index 5192f68..8bd2b06 100644
--- a/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/MemoryAllocator.java
+++ b/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/MemoryAllocator.java
@@ -20,8 +20,19 @@ package org.apache.spark.unsafe.memory;
public interface MemoryAllocator {
/**
+ * Whether to fill newly allocated and deallocated memory with 0xa5 and 0x5a bytes respectively.
+ * This helps catch misuse of uninitialized or freed memory, but imposes some overhead.
+ */
+ public static final boolean MEMORY_DEBUG_FILL_ENABLED = Boolean.parseBoolean(
+ System.getProperty("spark.memory.debugFill", "false"));
+
+ // Same as jemalloc's debug fill values.
+ public static final byte MEMORY_DEBUG_FILL_CLEAN_VALUE = (byte)0xa5;
+ public static final byte MEMORY_DEBUG_FILL_FREED_VALUE = (byte)0x5a;
+
+ /**
* Allocates a contiguous block of memory. Note that the allocated memory is not guaranteed
- * to be zeroed out (call `zero()` on the result if this is necessary).
+ * to be zeroed out (call `fill(0)` on the result if this is necessary).
*/
MemoryBlock allocate(long size) throws OutOfMemoryError;
http://git-wip-us.apache.org/repos/asf/spark/blob/44c7c62b/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/MemoryBlock.java
----------------------------------------------------------------------
diff --git a/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/MemoryBlock.java b/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/MemoryBlock.java
index 1bc924d..cd1d378 100644
--- a/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/MemoryBlock.java
+++ b/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/MemoryBlock.java
@@ -53,4 +53,11 @@ public class MemoryBlock extends MemoryLocation {
public static MemoryBlock fromLongArray(final long[] array) {
return new MemoryBlock(array, Platform.LONG_ARRAY_OFFSET, array.length * 8L);
}
+
+ /**
+ * Fills the memory block with the specified byte value.
+ */
+ public void fill(byte value) {
+ Platform.setMemory(obj, offset, length, value);
+ }
}
http://git-wip-us.apache.org/repos/asf/spark/blob/44c7c62b/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/UnsafeMemoryAllocator.java
----------------------------------------------------------------------
diff --git a/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/UnsafeMemoryAllocator.java b/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/UnsafeMemoryAllocator.java
index 98ce711..55bcdf1 100644
--- a/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/UnsafeMemoryAllocator.java
+++ b/common/unsafe/src/main/java/org/apache/spark/unsafe/memory/UnsafeMemoryAllocator.java
@@ -27,13 +27,20 @@ public class UnsafeMemoryAllocator implements MemoryAllocator {
@Override
public MemoryBlock allocate(long size) throws OutOfMemoryError {
long address = Platform.allocateMemory(size);
- return new MemoryBlock(null, address, size);
+ MemoryBlock memory = new MemoryBlock(null, address, size);
+ if (MemoryAllocator.MEMORY_DEBUG_FILL_ENABLED) {
+ memory.fill(MemoryAllocator.MEMORY_DEBUG_FILL_CLEAN_VALUE);
+ }
+ return memory;
}
@Override
public void free(MemoryBlock memory) {
assert (memory.obj == null) :
"baseObject not null; are you trying to use the off-heap allocator to free on-heap memory?";
+ if (MemoryAllocator.MEMORY_DEBUG_FILL_ENABLED) {
+ memory.fill(MemoryAllocator.MEMORY_DEBUG_FILL_FREED_VALUE);
+ }
Platform.freeMemory(memory.offset);
}
}
http://git-wip-us.apache.org/repos/asf/spark/blob/44c7c62b/common/unsafe/src/test/java/org/apache/spark/unsafe/PlatformUtilSuite.java
----------------------------------------------------------------------
diff --git a/common/unsafe/src/test/java/org/apache/spark/unsafe/PlatformUtilSuite.java b/common/unsafe/src/test/java/org/apache/spark/unsafe/PlatformUtilSuite.java
index 693ec6e..a77ba82 100644
--- a/common/unsafe/src/test/java/org/apache/spark/unsafe/PlatformUtilSuite.java
+++ b/common/unsafe/src/test/java/org/apache/spark/unsafe/PlatformUtilSuite.java
@@ -17,6 +17,9 @@
package org.apache.spark.unsafe;
+import org.apache.spark.unsafe.memory.MemoryAllocator;
+import org.apache.spark.unsafe.memory.MemoryBlock;
+
import org.junit.Assert;
import org.junit.Test;
@@ -58,4 +61,17 @@ public class PlatformUtilSuite {
Assert.assertEquals((byte)i, data[i + 1]);
}
}
+
+ @Test
+ public void memoryDebugFillEnabledInTest() {
+ Assert.assertTrue(MemoryAllocator.MEMORY_DEBUG_FILL_ENABLED);
+ MemoryBlock onheap = MemoryAllocator.HEAP.allocate(1);
+ MemoryBlock offheap = MemoryAllocator.UNSAFE.allocate(1);
+ Assert.assertEquals(
+ Platform.getByte(onheap.getBaseObject(), onheap.getBaseOffset()),
+ MemoryAllocator.MEMORY_DEBUG_FILL_CLEAN_VALUE);
+ Assert.assertEquals(
+ Platform.getByte(offheap.getBaseObject(), offheap.getBaseOffset()),
+ MemoryAllocator.MEMORY_DEBUG_FILL_CLEAN_VALUE);
+ }
}
http://git-wip-us.apache.org/repos/asf/spark/blob/44c7c62b/project/SparkBuild.scala
----------------------------------------------------------------------
diff --git a/project/SparkBuild.scala b/project/SparkBuild.scala
index b1a9f39..c769ba3 100644
--- a/project/SparkBuild.scala
+++ b/project/SparkBuild.scala
@@ -825,6 +825,7 @@ object TestSettings {
javaOptions in Test += "-Dspark.testing=1",
javaOptions in Test += "-Dspark.port.maxRetries=100",
javaOptions in Test += "-Dspark.master.rest.enabled=false",
+ javaOptions in Test += "-Dspark.memory.debugFill=true",
javaOptions in Test += "-Dspark.ui.enabled=false",
javaOptions in Test += "-Dspark.ui.showConsoleProgress=false",
javaOptions in Test += "-Dspark.unsafe.exceptionOnMemoryLeak=true",
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org