Repository: cassandra Updated Branches: refs/heads/cassandra-2.1 9d421f354 -> 5da4c4b16 refs/heads/trunk 4b16c1116 -> 65617aa0a
Improve assertions in Memory patch by benedict; reviewed by jbellis for CASSANDRA-8792 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/5da4c4b1 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/5da4c4b1 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/5da4c4b1 Branch: refs/heads/cassandra-2.1 Commit: 5da4c4b16d059f160730c2debe823b4caae082e8 Parents: 9d421f3 Author: Benedict Elliott Smith <[email protected]> Authored: Thu Feb 19 12:05:08 2015 +0000 Committer: Benedict Elliott Smith <[email protected]> Committed: Thu Feb 19 12:05:08 2015 +0000 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../org/apache/cassandra/io/util/Memory.java | 40 ++++++++++---------- 2 files changed, 22 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/5da4c4b1/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 1238173..7980855 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 2.1.4 + * Improve assertions in Memory (CASSANDRA-8792) * Fix SSTableRewriter cleanup (CASSANDRA-8802) * Introduce SafeMemory for CompressionMetadata.Writer (CASSANDRA-8758) * 'nodetool info' prints exception against older node (CASSANDRA-8796) http://git-wip-us.apache.org/repos/asf/cassandra/blob/5da4c4b1/src/java/org/apache/cassandra/io/util/Memory.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/io/util/Memory.java b/src/java/org/apache/cassandra/io/util/Memory.java index 2e7f28f..ea78840 100644 --- a/src/java/org/apache/cassandra/io/util/Memory.java +++ b/src/java/org/apache/cassandra/io/util/Memory.java @@ -54,9 +54,13 @@ public class Memory implements AutoCloseable protected Memory(long bytes) { + if (bytes <= 0) + throw new AssertionError(); size = bytes; peer = allocator.allocate(size); - if (size != 0 && peer == 0) + // we permit a 0 peer iff size is zero, since such an allocation makes no sense, and an allocator would be + // justified in returning a null pointer (and permitted to do so: http://www.cplusplus.com/reference/cstdlib/malloc) + if (peer == 0) throw new OutOfMemoryError(); } @@ -78,20 +82,20 @@ public class Memory implements AutoCloseable public void setByte(long offset, byte b) { - checkPosition(offset); + checkBounds(offset, offset + 1); unsafe.putByte(peer + offset, b); } public void setMemory(long offset, long bytes, byte b) { + checkBounds(offset, offset + bytes); // check if the last element will fit into the memory - checkPosition(offset + bytes - 1); unsafe.setMemory(peer + offset, bytes, b); } public void setLong(long offset, long l) { - checkPosition(offset); + checkBounds(offset, offset + 8); if (unaligned) { unsafe.putLong(peer + offset, l); @@ -130,7 +134,7 @@ public class Memory implements AutoCloseable public void setInt(long offset, int l) { - checkPosition(offset); + checkBounds(offset, offset + 4); if (unaligned) { unsafe.putInt(peer + offset, l); @@ -165,7 +169,8 @@ public class Memory implements AutoCloseable throw new NullPointerException(); else if (buffer.remaining() == 0) return; - checkPosition(memoryOffset + buffer.remaining()); + + checkBounds(memoryOffset, memoryOffset + buffer.remaining()); if (buffer.hasArray()) { setBytes(memoryOffset, buffer.array(), buffer.arrayOffset() + buffer.position(), buffer.remaining()); @@ -196,22 +201,21 @@ public class Memory implements AutoCloseable else if (count == 0) return; - checkPosition(memoryOffset); long end = memoryOffset + count; - checkPosition(end - 1); + checkBounds(memoryOffset, end); unsafe.copyMemory(buffer, BYTE_ARRAY_BASE_OFFSET + bufferOffset, null, peer + memoryOffset, count); } public byte getByte(long offset) { - checkPosition(offset); + checkBounds(offset, offset + 1); return unsafe.getByte(peer + offset); } public long getLong(long offset) { - checkPosition(offset); + checkBounds(offset, offset + 8); if (unaligned) { return unsafe.getLong(peer + offset); } else { @@ -243,7 +247,7 @@ public class Memory implements AutoCloseable public int getInt(long offset) { - checkPosition(offset); + checkBounds(offset, offset + 4); if (unaligned) { return unsafe.getInt(peer + offset); } else { @@ -282,18 +286,15 @@ public class Memory implements AutoCloseable else if (count == 0) return; - checkPosition(memoryOffset); - long end = memoryOffset + count; - checkPosition(end - 1); - + checkBounds(memoryOffset, memoryOffset + count); FastByteOperations.UnsafeOperations.copy(null, peer + memoryOffset, buffer, bufferOffset, count); } @Inline - protected void checkPosition(long offset) + protected void checkBounds(long start, long end) { assert peer != 0 : "Memory was freed"; - assert offset >= 0 && offset < size : "Illegal offset: " + offset + ", size: " + size; + assert start >= 0 && end <= size && start <= end : "Illegal bounds [" + start + ".." + end + "); size: " + size; } public void put(long trgOffset, Memory memory, long srcOffset, long size) @@ -310,8 +311,8 @@ public class Memory implements AutoCloseable public void free() { - assert peer != 0; - allocator.free(peer); + if (peer != 0) allocator.free(peer); + else assert size == 0; peer = 0; } @@ -365,4 +366,5 @@ public class Memory implements AutoCloseable { return String.format("Memory@[%x..%x)", peer, peer + size); } + }
