From d9c771bedcd99311ff3f5049163cc2181a71b6b2 Mon Sep 17 00:00:00 2001 From: leventov Date: Mon, 21 Sep 2015 19:08:45 +0300 Subject: [PATCH] Make key/value size discovery more defensive, inspired by #43 --- .../hash/impl/stage/hash/KeyBytesInterop.java | 2 +- .../internal/CopyingMetaBytesInterop.java | 11 ++++++----- .../internal/DataValueMetaBytesInterop.java | 9 ++++++--- .../internal/DelegatingMetaBytesInteropProvider.java | 5 +++-- .../serialization/internal/DirectBytesBuffer.java | 12 +++++++----- .../hash/serialization/internal/MetaProvider.java | 2 +- .../serialization/internal/SerializationBuilder.java | 5 +++-- .../openhft/chronicle/map/ChronicleMapBuilder.java | 2 +- .../net/openhft/chronicle/map/WriterWithSize.java | 6 ++++-- .../map/impl/stage/map/ValueBytesInterop.java | 2 +- .../java/net/openhft/chronicle/map/Issue43Test.java | 4 ++++ 11 files changed, 37 insertions(+), 23 deletions(-) diff --git a/src/main/java/net/openhft/chronicle/hash/impl/stage/hash/KeyBytesInterop.java b/src/main/java/net/openhft/chronicle/hash/impl/stage/hash/KeyBytesInterop.java index b8e55e91e..ea3f2fd6b 100644 --- a/src/main/java/net/openhft/chronicle/hash/impl/stage/hash/KeyBytesInterop.java +++ b/src/main/java/net/openhft/chronicle/hash/impl/stage/hash/KeyBytesInterop.java @@ -34,6 +34,6 @@ public class KeyBytesInterop> public MKI keyMetaInterop(K key) { return hh.h().metaKeyInteropProvider.get( - ch.copies, hh.h().originalMetaKeyInterop, keyInterop, key); + ch.copies, hh.h().originalMetaKeyInterop, keyInterop, key, false); } } diff --git a/src/main/java/net/openhft/chronicle/hash/serialization/internal/CopyingMetaBytesInterop.java b/src/main/java/net/openhft/chronicle/hash/serialization/internal/CopyingMetaBytesInterop.java index 7f7220328..94a2b8004 100644 --- a/src/main/java/net/openhft/chronicle/hash/serialization/internal/CopyingMetaBytesInterop.java +++ b/src/main/java/net/openhft/chronicle/hash/serialization/internal/CopyingMetaBytesInterop.java @@ -92,10 +92,10 @@ public abstract class CopyingMetaBytesInterop extends BasicCopyingMetaByte @Override public CopyingMetaBytesInterop get( ThreadLocalCopies copies, - CopyingMetaBytesInterop originalMetaWriter, M writer, E e) { + CopyingMetaBytesInterop originalMetaWriter, M writer, E e, boolean checked) { DirectBytesBuffer.ForBytesMarshaller forBytesMarshaller = provider.get(copies, originalMetaWriter.buffer()).forBytesMarshaller; - forBytesMarshaller.init(writer, e, mutable, maxSize); + forBytesMarshaller.init(writer, e, mutable, maxSize, checked); return forBytesMarshaller; } } @@ -115,11 +115,12 @@ public abstract class CopyingMetaBytesInterop extends BasicCopyingMetaByte } @Override - public CopyingMetaBytesInterop get(ThreadLocalCopies copies, - CopyingMetaBytesInterop originalMetaWriter, W writer, E e) { + public CopyingMetaBytesInterop get( + ThreadLocalCopies copies, CopyingMetaBytesInterop originalMetaWriter, + W writer, E e, boolean checked) { DirectBytesBuffer.ForBytesWriter forBytesWriter = provider.get(copies, originalMetaWriter.buffer()).forBytesWriter; - forBytesWriter.init(writer, e, mutable); + forBytesWriter.init(writer, e, mutable, checked); return forBytesWriter; } } diff --git a/src/main/java/net/openhft/chronicle/hash/serialization/internal/DataValueMetaBytesInterop.java b/src/main/java/net/openhft/chronicle/hash/serialization/internal/DataValueMetaBytesInterop.java index d19f11ef7..eaa235fa0 100644 --- a/src/main/java/net/openhft/chronicle/hash/serialization/internal/DataValueMetaBytesInterop.java +++ b/src/main/java/net/openhft/chronicle/hash/serialization/internal/DataValueMetaBytesInterop.java @@ -17,6 +17,7 @@ package net.openhft.chronicle.hash.serialization.internal; import net.openhft.chronicle.hash.serialization.BytesWriter; +import net.openhft.lang.io.BoundsCheckingDirectBytes; import net.openhft.lang.io.Bytes; import net.openhft.lang.model.*; import net.openhft.lang.threadlocal.ThreadLocalCopies; @@ -40,8 +41,10 @@ public abstract class DataValueMetaBytesInterop return e.equals(other); } - void init(BytesWriter writer, E e, long size) { + void init(BytesWriter writer, E e, long size, boolean checked) { Bytes buffer = this.buffer.obtain(size, true); + if (checked) + buffer = new BoundsCheckingDirectBytes(buffer, null); writer.write(buffer, e); buffer.flip(); assert buffer.remaining() == size; @@ -87,13 +90,13 @@ public abstract class DataValueMetaBytesInterop @Override public MetaBytesInterop> get( ThreadLocalCopies copies, MetaBytesInterop> originalMetaWriter, - BytesWriter writer, E e) { + BytesWriter writer, E e, boolean checked) { if (e instanceof Byteable) return metaByteableInterop; DirectBytesBuffer.ForDataValueWriter forDataValueWriter = provider.get(copies, ((DataValueMetaBytesInterop)originalMetaWriter).buffer) .forDataValueWriter; - forDataValueWriter.init(writer, e, size); + forDataValueWriter.init(writer, e, size, checked); return forDataValueWriter; } } diff --git a/src/main/java/net/openhft/chronicle/hash/serialization/internal/DelegatingMetaBytesInteropProvider.java b/src/main/java/net/openhft/chronicle/hash/serialization/internal/DelegatingMetaBytesInteropProvider.java index 7023de886..3374a0526 100644 --- a/src/main/java/net/openhft/chronicle/hash/serialization/internal/DelegatingMetaBytesInteropProvider.java +++ b/src/main/java/net/openhft/chronicle/hash/serialization/internal/DelegatingMetaBytesInteropProvider.java @@ -35,8 +35,9 @@ public final class DelegatingMetaBytesInteropProvider get(ThreadLocalCopies copies, - MetaBytesInterop originalMetaWriter, I writer, E e) { + public MetaBytesInterop get( + ThreadLocalCopies copies, MetaBytesInterop originalMetaWriter, I writer, E e, + boolean checked) { return originalMetaWriter; } diff --git a/src/main/java/net/openhft/chronicle/hash/serialization/internal/DirectBytesBuffer.java b/src/main/java/net/openhft/chronicle/hash/serialization/internal/DirectBytesBuffer.java index 8fabcd4d4..cfc930514 100644 --- a/src/main/java/net/openhft/chronicle/hash/serialization/internal/DirectBytesBuffer.java +++ b/src/main/java/net/openhft/chronicle/hash/serialization/internal/DirectBytesBuffer.java @@ -17,9 +17,7 @@ package net.openhft.chronicle.hash.serialization.internal; import net.openhft.chronicle.hash.serialization.BytesWriter; -import net.openhft.lang.io.Bytes; -import net.openhft.lang.io.DirectBytes; -import net.openhft.lang.io.DirectStore; +import net.openhft.lang.io.*; import net.openhft.lang.io.serialization.BytesMarshallableSerializer; import net.openhft.lang.io.serialization.BytesMarshaller; import net.openhft.lang.io.serialization.ObjectSerializer; @@ -91,13 +89,15 @@ class DirectBytesBuffer super(DirectBytesBuffer.this); } - void init(M writer, E e, boolean mutable, long maxSize) { + void init(M writer, E e, boolean mutable, long maxSize, boolean checked) { if (mutable || writer != this.writer || e != cur) { this.writer = writer; cur = e; while (true) { try { Bytes buffer = this.buffer.obtain(maxSize, true); + if (checked) + buffer = new BoundsCheckingDirectBytes(buffer, null); writer.write(buffer, e); buffer.flip(); long size = this.size = buffer.remaining(); @@ -122,12 +122,14 @@ class DirectBytesBuffer super(DirectBytesBuffer.this); } - void init(W writer, E e, boolean mutable) { + void init(W writer, E e, boolean mutable, boolean checked) { if (mutable || writer != this.writer || e != cur) { this.writer = writer; cur = e; long size = writer.size(e); Bytes buffer = this.buffer.obtain(size, true); + if (checked) + buffer = new BoundsCheckingDirectBytes(buffer, null); writer.write(buffer, e); buffer.flip(); this.size = size; diff --git a/src/main/java/net/openhft/chronicle/hash/serialization/internal/MetaProvider.java b/src/main/java/net/openhft/chronicle/hash/serialization/internal/MetaProvider.java index aeceac3ac..71b2c7469 100644 --- a/src/main/java/net/openhft/chronicle/hash/serialization/internal/MetaProvider.java +++ b/src/main/java/net/openhft/chronicle/hash/serialization/internal/MetaProvider.java @@ -22,7 +22,7 @@ import java.io.Serializable; public interface MetaProvider> extends Serializable { - MW get(ThreadLocalCopies copies, MW originalMetaWriter, W writer, E e); + MW get(ThreadLocalCopies copies, MW originalMetaWriter, W writer, E e, boolean checked); ThreadLocalCopies getCopies(ThreadLocalCopies copies); } diff --git a/src/main/java/net/openhft/chronicle/hash/serialization/internal/SerializationBuilder.java b/src/main/java/net/openhft/chronicle/hash/serialization/internal/SerializationBuilder.java index 8899bbbd1..f239f560c 100755 --- a/src/main/java/net/openhft/chronicle/hash/serialization/internal/SerializationBuilder.java +++ b/src/main/java/net/openhft/chronicle/hash/serialization/internal/SerializationBuilder.java @@ -278,7 +278,7 @@ public final class SerializationBuilder implements Cloneable, Serializable { MetaProvider metaInteropProvider = this.metaInteropProvider; try { metaInterop = metaInteropProvider.get( - copies, this.metaInterop, interop, sampleObject); + copies, this.metaInterop, interop, sampleObject, true); break findSufficientSerializationSize; } catch (Exception e) { CopyingMetaBytesInterop.checkMaxSizeStillReasonable(maxSize, e); @@ -287,7 +287,8 @@ public final class SerializationBuilder implements Cloneable, Serializable { } } else { MetaProvider metaInteropProvider = this.metaInteropProvider; - metaInterop = metaInteropProvider.get(copies, this.metaInterop, interop, sampleObject); + metaInterop = metaInteropProvider.get( + copies, this.metaInterop, interop, sampleObject, true); } return metaInterop.size(interop, sampleObject); } diff --git a/src/main/java/net/openhft/chronicle/map/ChronicleMapBuilder.java b/src/main/java/net/openhft/chronicle/map/ChronicleMapBuilder.java index c27296116..fe048964b 100755 --- a/src/main/java/net/openhft/chronicle/map/ChronicleMapBuilder.java +++ b/src/main/java/net/openhft/chronicle/map/ChronicleMapBuilder.java @@ -1329,7 +1329,7 @@ public final class ChronicleMapBuilder implements MetaProvider metaWriterProvider = valueBuilder.metaInteropProvider(); copies = metaWriterProvider.getCopies(copies); MetaBytesWriter metaValueWriter = metaWriterProvider.get(copies, - valueBuilder.metaInterop(), valueWriter, defaultValue); + valueBuilder.metaInterop(), valueWriter, defaultValue, true); return new ConstantValueProvider<>(defaultValue, metaValueWriter, valueWriter); } diff --git a/src/main/java/net/openhft/chronicle/map/WriterWithSize.java b/src/main/java/net/openhft/chronicle/map/WriterWithSize.java index 6b4283b4d..1d2b115ff 100644 --- a/src/main/java/net/openhft/chronicle/map/WriterWithSize.java +++ b/src/main/java/net/openhft/chronicle/map/WriterWithSize.java @@ -55,7 +55,8 @@ final class WriterWithSize { copies = writerProvider.getCopies(copies); Object writer = writerProvider.get(copies, originalWriter); copies = metaWriterProvider.getCopies(copies); - MetaBytesWriter metaWriter = metaWriterProvider.get(copies, originalMetaWriter, writer, t); + MetaBytesWriter metaWriter = metaWriterProvider.get( + copies, originalMetaWriter, writer, t, false); long size = metaWriter.size(writer, t); if (bufferResizer != null) out = resizeIfRequired(out, size); @@ -90,7 +91,8 @@ final class WriterWithSize { public ThreadLocalCopies writeInLoop(Bytes out, T t, Object writer, @Nullable ThreadLocalCopies copies) { copies = metaWriterProvider.getCopies(copies); - MetaBytesWriter metaWriter = metaWriterProvider.get(copies, originalMetaWriter, writer, t); + MetaBytesWriter metaWriter = metaWriterProvider.get( + copies, originalMetaWriter, writer, t, false); long size = metaWriter.size(writer, t); if (bufferResizer != null) out = resizeIfRequired(out, size); diff --git a/src/main/java/net/openhft/chronicle/map/impl/stage/map/ValueBytesInterop.java b/src/main/java/net/openhft/chronicle/map/impl/stage/map/ValueBytesInterop.java index 183c8dff6..251cb6ed2 100644 --- a/src/main/java/net/openhft/chronicle/map/impl/stage/map/ValueBytesInterop.java +++ b/src/main/java/net/openhft/chronicle/map/impl/stage/map/ValueBytesInterop.java @@ -35,6 +35,6 @@ public class ValueBytesInterop