This is an automated email from the ASF dual-hosted git repository.
jorgebg pushed a commit to branch TINKERPOP-2154
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit c66c497880c0dae6df5e5450bee6a2376f10a149
Author: Jorge Bay Gondra <jorgebaygon...@gmail.com>
AuthorDate: Fri Feb 8 12:31:53 2019 +0100
TINKERPOP-2154 GraphBinary: Release working buffers on failure
---
.../driver/ser/binary/GraphBinaryWriter.java | 21 +++--
.../driver/ser/binary/types/BindingSerializer.java | 9 +-
.../driver/ser/binary/types/BulkSetSerializer.java | 12 ++-
.../ser/binary/types/ByteCodeSerializer.java | 25 ++++--
.../ser/binary/types/CollectionSerializer.java | 10 ++-
.../ser/binary/types/CustomTypeSerializer.java | 3 -
.../driver/ser/binary/types/EdgeSerializer.java | 33 +++++---
.../driver/ser/binary/types/GraphSerializer.java | 79 ++++++++++++------
.../driver/ser/binary/types/LambdaSerializer.java | 14 +++-
.../driver/ser/binary/types/MapSerializer.java | 14 ++--
.../driver/ser/binary/types/MetricsSerializer.java | 27 ++++--
.../driver/ser/binary/types/PathSerializer.java | 11 ++-
.../binary/types/TraversalMetricsSerializer.java | 16 +++-
.../binary/types/TraversalStrategySerializer.java | 11 ++-
.../ser/binary/types/TraverserSerializer.java | 11 ++-
.../driver/ser/binary/types/TreeSerializer.java | 16 ++--
.../ser/binary/types/VertexPropertySerializer.java | 23 +++--
.../driver/ser/binary/types/VertexSerializer.java | 19 +++--
.../ser/binary/TypeSerializerFailureTests.java | 97 ++++++++++++++++++++++
19 files changed, 341 insertions(+), 110 deletions(-)
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/GraphBinaryWriter.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/GraphBinaryWriter.java
index b3c2c0f..1703658 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/GraphBinaryWriter.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/GraphBinaryWriter.java
@@ -71,14 +71,16 @@ public class GraphBinaryWriter {
final TypeSerializer<T> serializer = (TypeSerializer<T>)
registry.getSerializer(objectClass);
if (serializer instanceof CustomTypeSerializer) {
+ // It's a custom type
CustomTypeSerializer customTypeSerializer = (CustomTypeSerializer)
serializer;
- // Is a custom type
- // Write type code, custom type name and let the serializer write
- // the rest of {custom_type_info} followed by {value_flag} and
{value}
- return allocator.compositeBuffer(3).addComponents(true,
- Unpooled.wrappedBuffer(customTypeCodeBytes),
- writeValue(customTypeSerializer.getTypeName(), allocator,
false),
- customTypeSerializer.write(value, allocator, this));
+
+ // Try to serialize the custom value before allocating a composite
buffer
+ ByteBuf customTypeValueBuffer = customTypeSerializer.write(value,
allocator, this);
+
+ return allocator.compositeBuffer(3)
+ .addComponent(true,
Unpooled.wrappedBuffer(customTypeCodeBytes))
+ .addComponent(true,
writeValue(customTypeSerializer.getTypeName(), allocator, false))
+ .addComponent(true, customTypeValueBuffer);
}
if (serializer instanceof TransformSerializer) {
@@ -88,11 +90,14 @@ public class GraphBinaryWriter {
return write(transformSerializer.transform(value), allocator);
}
+ // Try to serialize the value before creating a new composite buffer
+ ByteBuf typeInfoAndValueBuffer = serializer.write(value, allocator,
this);
+
return allocator.compositeBuffer(2).addComponents(true,
// {type_code}
Unpooled.wrappedBuffer(serializer.getDataType().getDataTypeBuffer()),
// {type_info}{value_flag}{value}
- serializer.write(value, allocator, this));
+ typeInfoAndValueBuffer);
}
/**
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/BindingSerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/BindingSerializer.java
index de73fb2..3d9f61a 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/BindingSerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/BindingSerializer.java
@@ -45,8 +45,13 @@ public class BindingSerializer extends
SimpleTypeSerializer<Bytecode.Binding> {
@Override
protected ByteBuf writeValue(final Bytecode.Binding value, final
ByteBufAllocator allocator, final GraphBinaryWriter context) throws
SerializationException {
final CompositeByteBuf result = allocator.compositeBuffer(2);
- result.addComponent(true, context.writeValue(value.variable(),
allocator, false));
- result.addComponent(true, context.write(value.value(), allocator));
+ try {
+ result.addComponent(true, context.writeValue(value.variable(),
allocator, false));
+ result.addComponent(true, context.write(value.value(), allocator));
+ } catch (Exception ex) {
+ result.release();
+ throw ex;
+ }
return result;
}
}
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/BulkSetSerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/BulkSetSerializer.java
index 99880de..04f3d63 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/BulkSetSerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/BulkSetSerializer.java
@@ -55,9 +55,15 @@ public class BulkSetSerializer extends
SimpleTypeSerializer<BulkSet> {
final CompositeByteBuf result = allocator.compositeBuffer(1 +
raw.size() * 2);
result.addComponent(true, allocator.buffer(4).writeInt(raw.size()));
- for (Object key : raw.keySet()) {
- result.addComponents(true, context.write(key, allocator),
- allocator.buffer(8).writeLong(value.get(key)));
+ try {
+ for (Object key : raw.keySet()) {
+ result.addComponents(true, context.write(key, allocator),
+ allocator.buffer(8).writeLong(value.get(key)));
+ }
+ } catch (Exception ex) {
+ // We should release it as the ByteBuf is not going to be yielded
for a reader
+ result.release();
+ throw ex;
}
return result;
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/ByteCodeSerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/ByteCodeSerializer.java
index 8fe9a4a..3b39eb9 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/ByteCodeSerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/ByteCodeSerializer.java
@@ -67,8 +67,14 @@ public class ByteCodeSerializer extends
SimpleTypeSerializer<Bytecode> {
// 2 buffers for the length + plus 2 buffers per each step and source
final CompositeByteBuf result = allocator.compositeBuffer(2 +
steps.size() * 2 + sources.size() * 2);
- writeInstructions(allocator, context, steps, result);
- writeInstructions(allocator, context, sources, result);
+ try {
+ writeInstructions(allocator, context, steps, result);
+ writeInstructions(allocator, context, sources, result);
+ } catch (Exception ex) {
+ // We should release it as the ByteBuf is not going to be yielded
for a reader
+ result.release();
+ throw ex;
+ }
return result;
}
@@ -79,10 +85,8 @@ public class ByteCodeSerializer extends
SimpleTypeSerializer<Bytecode> {
result.addComponent(true, context.writeValue(instructions.size(),
allocator, false));
for (Bytecode.Instruction instruction : instructions) {
- result.addComponents(
- true,
- context.writeValue(instruction.getOperator(), allocator,
false),
- getArgumentsBuffer(instruction.getArguments(), allocator,
context));
+ result.addComponent(true,
context.writeValue(instruction.getOperator(), allocator, false));
+ result.addComponent(true,
getArgumentsBuffer(instruction.getArguments(), allocator, context));
}
}
@@ -90,8 +94,13 @@ public class ByteCodeSerializer extends
SimpleTypeSerializer<Bytecode> {
final CompositeByteBuf result = allocator.compositeBuffer(1 +
arguments.length);
result.addComponent(true, context.writeValue(arguments.length,
allocator, false));
- for (Object value : arguments) {
- result.addComponent(true, context.write(value, allocator));
+ try {
+ for (Object value : arguments) {
+ result.addComponent(true, context.write(value, allocator));
+ }
+ } catch (Exception ex) {
+ result.release();
+ throw ex;
}
return result;
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/CollectionSerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/CollectionSerializer.java
index 1b09a5f..858483f 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/CollectionSerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/CollectionSerializer.java
@@ -51,8 +51,14 @@ class CollectionSerializer extends
SimpleTypeSerializer<Collection> {
final CompositeByteBuf result = allocator.compositeBuffer(1 +
value.size());
result.addComponent(true, allocator.buffer(4).writeInt(value.size()));
- for (Object item : value) {
- result.addComponent(true, context.write(item, allocator));
+ try {
+ for (Object item : value) {
+ result.addComponent(true, context.write(item, allocator));
+ }
+ } catch (Exception ex) {
+ // We should release it as the ByteBuf is not going to be yielded
for a reader
+ result.release();
+ throw ex;
}
return result;
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/CustomTypeSerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/CustomTypeSerializer.java
index 4d25e7c..d718bce 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/CustomTypeSerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/CustomTypeSerializer.java
@@ -22,9 +22,6 @@ import
org.apache.tinkerpop.gremlin.driver.ser.binary.TypeSerializer;
/**
* Represents a serializer for a custom (provider specific) serializer.
- * <p>
- * Note that invocations to
- * </p>
* @param <T>
*/
public interface CustomTypeSerializer<T> extends TypeSerializer<T> {
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/EdgeSerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/EdgeSerializer.java
index c9e8549..3c9f950 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/EdgeSerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/EdgeSerializer.java
@@ -61,20 +61,27 @@ public class EdgeSerializer extends
SimpleTypeSerializer<Edge> {
@Override
protected ByteBuf writeValue(final Edge value, final ByteBufAllocator
allocator, final GraphBinaryWriter context) throws SerializationException {
final CompositeByteBuf result = allocator.compositeBuffer(8);
- result.addComponent(true, context.write(value.id(), allocator));
- result.addComponent(true, context.writeValue(value.label(), allocator,
false));
- result.addComponent(true, context.write(value.inVertex().id(),
allocator));
- result.addComponent(true, context.writeValue(value.inVertex().label(),
allocator, false));
- result.addComponent(true, context.write(value.outVertex().id(),
allocator));
- result.addComponent(true,
context.writeValue(value.outVertex().label(), allocator, false));
-
- // we don't serialize the parent Vertex for edges. they are
"references", but we leave a place holder
- // here as an option for the future as we've waffled this soooooooooo
many times now
- result.addComponent(true, context.write(null, allocator));
- // we don't serialize properties for graph vertices/edges. they are
"references", but we leave a place holder
- // here as an option for the future as we've waffled this soooooooooo
many times now
- result.addComponent(true, context.write(null, allocator));
+ try {
+ result.addComponent(true, context.write(value.id(), allocator));
+ result.addComponent(true, context.writeValue(value.label(),
allocator, false));
+
+ result.addComponent(true, context.write(value.inVertex().id(),
allocator));
+ result.addComponent(true,
context.writeValue(value.inVertex().label(), allocator, false));
+ result.addComponent(true, context.write(value.outVertex().id(),
allocator));
+ result.addComponent(true,
context.writeValue(value.outVertex().label(), allocator, false));
+
+ // we don't serialize the parent Vertex for edges. they are
"references", but we leave a place holder
+ // here as an option for the future as we've waffled this
soooooooooo many times now
+ result.addComponent(true, context.write(null, allocator));
+ // we don't serialize properties for graph vertices/edges. they
are "references", but we leave a place holder
+ // here as an option for the future as we've waffled this
soooooooooo many times now
+ result.addComponent(true, context.write(null, allocator));
+ } catch (Exception ex) {
+ // We should release it as the ByteBuf is not going to be yielded
for a reader
+ result.release();
+ throw ex;
+ }
return result;
}
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/GraphSerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/GraphSerializer.java
index 8db6e70..0f228c1 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/GraphSerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/GraphSerializer.java
@@ -117,44 +117,68 @@ public class GraphSerializer extends
SimpleTypeSerializer<Graph> {
final List<Edge> edgeList = IteratorUtils.list(value.edges());
final CompositeByteBuf result = allocator.compositeBuffer(2 +
edgeList.size() + vertexList.size());
- result.addComponent(true, context.writeValue(vertexList.size(),
allocator, false));
- for (Vertex v : vertexList) {
- final List<VertexProperty<Object>> vertexProperties =
IteratorUtils.list(v.properties());
- final CompositeByteBuf vbb = allocator.compositeBuffer(3 +
vertexProperties.size());
- vbb.addComponent(true, context.write(v.id(), allocator));
- vbb.addComponent(true, context.writeValue(v.label(), allocator,
false));
+ try {
+ result.addComponent(true, context.writeValue(vertexList.size(),
allocator, false));
+
+ for (Vertex v : vertexList) {
+ result.addComponent(true, writeVertex(allocator, context, v));
+ }
+
+ result.addComponent(true, context.writeValue(edgeList.size(),
allocator, false));
+
+ for (Edge e : edgeList) {
+ result.addComponent(true, writeEdge(allocator, context, e));
+ }
+
+ } catch (Exception ex) {
+ result.release();
+ throw ex;
+ }
+
+ return result;
+ }
+
+ private ByteBuf writeVertex(ByteBufAllocator allocator, GraphBinaryWriter
context, Vertex vertex) throws SerializationException {
+ final List<VertexProperty<Object>> vertexProperties =
IteratorUtils.list(vertex.properties());
+ final CompositeByteBuf vbb = allocator.compositeBuffer(3 +
vertexProperties.size() * 5);
+
+ try {
+ vbb.addComponent(true, context.write(vertex.id(), allocator));
+ vbb.addComponent(true, context.writeValue(vertex.label(),
allocator, false));
vbb.addComponent(true, context.writeValue(vertexProperties.size(),
allocator, false));
for (VertexProperty<Object> vp : vertexProperties) {
- final CompositeByteBuf vpbb = allocator.compositeBuffer(5);
- vpbb.addComponent(true, context.write(vp.id(), allocator));
- vpbb.addComponent(true, context.writeValue(vp.label(),
allocator, false));
- vpbb.addComponent(true, context.write(vp.value(), allocator));
+ vbb.addComponent(true, context.write(vp.id(), allocator));
+ vbb.addComponent(true, context.writeValue(vp.label(),
allocator, false));
+ vbb.addComponent(true, context.write(vp.value(), allocator));
// maintain the VertexProperty format we have with this empty
parent.........
- vpbb.addComponent(true, context.write(null, allocator));
+ vbb.addComponent(true, context.write(null, allocator));
// write those properties out using the standard Property
serializer
- vpbb.addComponent(true,
context.writeValue(IteratorUtils.list(vp.properties()), allocator, false));
-
- vbb.addComponent(true, vpbb);
+ vbb.addComponent(true,
context.writeValue(IteratorUtils.list(vp.properties()), allocator, false));
}
-
- result.addComponent(true, vbb);
+ } catch (Exception ex) {
+ vbb.release();
+ throw ex;
}
- result.addComponent(true, context.writeValue(edgeList.size(),
allocator, false));
- for (Edge e : edgeList) {
- final CompositeByteBuf ebb = allocator.compositeBuffer(8);
- ebb.addComponent(true, context.write(e.id(), allocator));
- ebb.addComponent(true, context.writeValue(e.label(), allocator,
false));
+ return vbb;
+ }
+
+ private ByteBuf writeEdge(ByteBufAllocator allocator, GraphBinaryWriter
context, Edge edge) throws SerializationException {
+ final CompositeByteBuf ebb = allocator.compositeBuffer(8);
+
+ try {
+ ebb.addComponent(true, context.write(edge.id(), allocator));
+ ebb.addComponent(true, context.writeValue(edge.label(), allocator,
false));
- ebb.addComponent(true, context.write(e.inVertex().id(),
allocator));
+ ebb.addComponent(true, context.write(edge.inVertex().id(),
allocator));
// vertex labels aren't needed but maintaining the Edge form that
we have
ebb.addComponent(true, context.write(null, allocator));
- ebb.addComponent(true, context.write(e.outVertex().id(),
allocator));
+ ebb.addComponent(true, context.write(edge.outVertex().id(),
allocator));
// vertex labels aren't needed but maintaining the Edge form that
we have
ebb.addComponent(true, context.write(null, allocator));
@@ -163,12 +187,13 @@ public class GraphSerializer extends
SimpleTypeSerializer<Graph> {
ebb.addComponent(true, context.write(null, allocator));
// write those properties out using the standard Property
serializer
- ebb.addComponent(true,
context.writeValue(IteratorUtils.list(e.properties()), allocator, false));
-
- result.addComponent(true, ebb);
+ ebb.addComponent(true,
context.writeValue(IteratorUtils.list(edge.properties()), allocator, false));
+ } catch (Exception ex) {
+ ebb.release();
+ throw ex;
}
- return result;
+ return ebb;
}
private static Map<String, List<VertexProperty>>
indexedVertexProperties(final Vertex v) {
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/LambdaSerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/LambdaSerializer.java
index c7cd0d5..545d018 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/LambdaSerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/LambdaSerializer.java
@@ -54,9 +54,17 @@ public class LambdaSerializer extends
SimpleTypeSerializer<Lambda> {
@Override
protected ByteBuf writeValue(final Lambda value, final ByteBufAllocator
allocator, final GraphBinaryWriter context) throws SerializationException {
final CompositeByteBuf result = allocator.compositeBuffer(3);
- result.addComponent(true,
context.writeValue(value.getLambdaLanguage(), allocator, false));
- result.addComponent(true, context.writeValue(value.getLambdaScript(),
allocator, false));
- result.addComponent(true,
context.writeValue(value.getLambdaArguments(), allocator, false));
+
+ try {
+ result.addComponent(true,
context.writeValue(value.getLambdaLanguage(), allocator, false));
+ result.addComponent(true,
context.writeValue(value.getLambdaScript(), allocator, false));
+ result.addComponent(true,
context.writeValue(value.getLambdaArguments(), allocator, false));
+ } catch (Exception ex) {
+ // We should release it as the ByteBuf is not going to be yielded
for a reader
+ result.release();
+ throw ex;
+ }
+
return result;
}
}
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/MapSerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/MapSerializer.java
index b03e414..ad76a74 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/MapSerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/MapSerializer.java
@@ -51,11 +51,15 @@ public class MapSerializer extends
SimpleTypeSerializer<Map> {
final CompositeByteBuf result = allocator.compositeBuffer(1 +
value.size() * 2);
result.addComponent(true, allocator.buffer(4).writeInt(value.size()));
- for (Object key : value.keySet()) {
- result.addComponents(
- true,
- context.write(key, allocator),
- context.write(value.get(key), allocator));
+ try {
+ for (Object key : value.keySet()) {
+ result.addComponent(true, context.write(key, allocator));
+ result.addComponent(true, context.write(value.get(key),
allocator));
+ }
+ } catch (Exception ex) {
+ // We should release it as the ByteBuf is not going to be yielded
for a reader
+ result.release();
+ throw ex;
}
return result;
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/MetricsSerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/MetricsSerializer.java
index dc65183..4ce12a0 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/MetricsSerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/MetricsSerializer.java
@@ -20,6 +20,7 @@ package org.apache.tinkerpop.gremlin.driver.ser.binary.types;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufAllocator;
+import io.netty.buffer.CompositeByteBuf;
import org.apache.tinkerpop.gremlin.driver.ser.SerializationException;
import org.apache.tinkerpop.gremlin.driver.ser.binary.DataType;
import org.apache.tinkerpop.gremlin.driver.ser.binary.GraphBinaryReader;
@@ -56,13 +57,23 @@ public class MetricsSerializer extends
SimpleTypeSerializer<Metrics> {
@Override
protected ByteBuf writeValue(final Metrics value, final ByteBufAllocator
allocator, final GraphBinaryWriter context) throws SerializationException {
- return allocator.compositeBuffer(6).addComponents(true,
- context.writeValue(value.getId(), allocator, false),
- context.writeValue(value.getName(), allocator, false),
- context.writeValue(value.getDuration(TimeUnit.NANOSECONDS),
allocator, false),
- context.writeValue(value.getCounts(), allocator, false),
- context.writeValue(value.getAnnotations(), allocator, false),
- // Avoid changing type to List
- collectionSerializer.writeValue(value.getNested(), allocator,
context));
+ final CompositeByteBuf result = allocator.compositeBuffer(6);
+
+ try {
+ result.addComponent(true, context.writeValue(value.getId(),
allocator, false));
+ result.addComponent(true, context.writeValue(value.getName(),
allocator, false));
+ result.addComponent(true,
context.writeValue(value.getDuration(TimeUnit.NANOSECONDS), allocator, false));
+ result.addComponent(true, context.writeValue(value.getCounts(),
allocator, false));
+ result.addComponent(true,
context.writeValue(value.getAnnotations(), allocator, false));
+
+ // Avoid changing type to List
+ result.addComponent(true,
collectionSerializer.writeValue(value.getNested(), allocator, context));
+ } catch (Exception ex) {
+ // We should release the CompositeByteBuf as it's not going to be
yielded for a reader
+ result.release();
+ throw ex;
+ }
+
+ return result;
}
}
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/PathSerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/PathSerializer.java
index 4c640d7..344d9ca 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/PathSerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/PathSerializer.java
@@ -61,8 +61,15 @@ public class PathSerializer extends
SimpleTypeSerializer<Path> {
@Override
protected ByteBuf writeValue(final Path value, final ByteBufAllocator
allocator, final GraphBinaryWriter context) throws SerializationException {
final CompositeByteBuf result = allocator.compositeBuffer(2);
- result.addComponent(true, context.write(value.labels(), allocator));
- result.addComponent(true, context.write(value.objects(), allocator));
+
+ try {
+ result.addComponent(true, context.write(value.labels(),
allocator));
+ result.addComponent(true, context.write(value.objects(),
allocator));
+ } catch (Exception ex) {
+ result.release();
+ throw ex;
+ }
+
return result;
}
}
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/TraversalMetricsSerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/TraversalMetricsSerializer.java
index f2bba8a..90650ef 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/TraversalMetricsSerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/TraversalMetricsSerializer.java
@@ -20,6 +20,7 @@ package org.apache.tinkerpop.gremlin.driver.ser.binary.types;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufAllocator;
+import io.netty.buffer.CompositeByteBuf;
import org.apache.tinkerpop.gremlin.driver.ser.SerializationException;
import org.apache.tinkerpop.gremlin.driver.ser.binary.DataType;
import org.apache.tinkerpop.gremlin.driver.ser.binary.GraphBinaryReader;
@@ -48,8 +49,17 @@ public class TraversalMetricsSerializer extends
SimpleTypeSerializer<TraversalMe
@Override
protected ByteBuf writeValue(TraversalMetrics value, ByteBufAllocator
allocator, GraphBinaryWriter context) throws SerializationException {
- return allocator.compositeBuffer(2).addComponents(true,
- context.writeValue(value.getDuration(TimeUnit.NANOSECONDS),
allocator, false),
- collectionSerializer.writeValue(value.getMetrics(), allocator,
context));
+ final CompositeByteBuf result = allocator.compositeBuffer(2);
+
+ try {
+ result.addComponent(true,
context.writeValue(value.getDuration(TimeUnit.NANOSECONDS), allocator, false));
+ result.addComponent(true,
collectionSerializer.writeValue(value.getMetrics(), allocator, context));
+ } catch (Exception ex) {
+ // We should release it as the ByteBuf is not going to be yielded
for a reader
+ result.release();
+ throw ex;
+ }
+
+ return result;
}
}
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/TraversalStrategySerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/TraversalStrategySerializer.java
index 0de52a2..0b63907 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/TraversalStrategySerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/TraversalStrategySerializer.java
@@ -55,8 +55,15 @@ public class TraversalStrategySerializer extends
SimpleTypeSerializer<TraversalS
@Override
protected ByteBuf writeValue(final TraversalStrategy value, final
ByteBufAllocator allocator, final GraphBinaryWriter context) throws
SerializationException {
final CompositeByteBuf result = allocator.compositeBuffer(2);
- result.addComponent(true, context.writeValue(value.getClass(),
allocator, false));
- result.addComponent(true,
context.writeValue(translateToBytecode(ConfigurationConverter.getMap(value.getConfiguration())),
allocator, false));
+
+ try {
+ result.addComponent(true, context.writeValue(value.getClass(),
allocator, false));
+ result.addComponent(true,
context.writeValue(translateToBytecode(ConfigurationConverter.getMap(value.getConfiguration())),
allocator, false));
+ } catch (Exception ex) {
+ result.release();
+ throw ex;
+ }
+
return result;
}
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/TraverserSerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/TraverserSerializer.java
index 8dc026a..c9f4bd7 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/TraverserSerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/TraverserSerializer.java
@@ -47,8 +47,15 @@ public class TraverserSerializer extends
SimpleTypeSerializer<Traverser> {
@Override
protected ByteBuf writeValue(final Traverser value, final ByteBufAllocator
allocator, final GraphBinaryWriter context) throws SerializationException {
final CompositeByteBuf result = allocator.compositeBuffer(2);
- result.addComponent(true, context.writeValue(value.bulk(), allocator,
false));
- result.addComponent(true, context.write(value.get(), allocator));
+
+ try {
+ result.addComponent(true, context.writeValue(value.bulk(),
allocator, false));
+ result.addComponent(true, context.write(value.get(), allocator));
+ } catch (Exception ex) {
+ result.release();
+ throw ex;
+ }
+
return result;
}
}
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/TreeSerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/TreeSerializer.java
index 0d769dc..bc7fa73 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/TreeSerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/TreeSerializer.java
@@ -49,11 +49,17 @@ public class TreeSerializer extends
SimpleTypeSerializer<Tree> {
final CompositeByteBuf result = allocator.compositeBuffer(1 +
value.size() * 2);
result.addComponent(true, allocator.buffer(4).writeInt(value.size()));
- for (Object key : value.keySet()) {
- result.addComponents(
- true,
- context.write(key, allocator),
- context.writeValue(value.get(key), allocator, false));
+ try {
+ for (Object key : value.keySet()) {
+ result.addComponents(
+ true,
+ context.write(key, allocator),
+ context.writeValue(value.get(key), allocator, false));
+ }
+ } catch (Exception ex) {
+ // We should release it as the ByteBuf is not going to be yielded
for a reader
+ result.release();
+ throw ex;
}
return result;
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/VertexPropertySerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/VertexPropertySerializer.java
index 196ebe6..b1d134c 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/VertexPropertySerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/VertexPropertySerializer.java
@@ -55,15 +55,22 @@ public class VertexPropertySerializer extends
SimpleTypeSerializer<VertexPropert
@Override
protected ByteBuf writeValue(final VertexProperty value, final
ByteBufAllocator allocator, final GraphBinaryWriter context) throws
SerializationException {
final CompositeByteBuf result = allocator.compositeBuffer(5);
- result.addComponent(true, context.write(value.id(), allocator));
- result.addComponent(true, context.writeValue(value.label(), allocator,
false));
- result.addComponent(true, context.write(value.value(), allocator));
- // we don't serialize the parent vertex even as a "reference", but,
let's hold a place for it
- result.addComponent(true, context.write(null, allocator));
- // we don't serialize properties for graph elements. they are
"references", but we leave a place holder
- // here as an option for the future as we've waffled this soooooooooo
many times now
- result.addComponent(true, context.write(null, allocator));
+ try {
+ result.addComponent(true, context.write(value.id(), allocator));
+ result.addComponent(true, context.writeValue(value.label(),
allocator, false));
+ result.addComponent(true, context.write(value.value(), allocator));
+
+ // we don't serialize the parent vertex even as a "reference",
but, let's hold a place for it
+ result.addComponent(true, context.write(null, allocator));
+ // we don't serialize properties for graph elements. they are
"references", but we leave a place holder
+ // here as an option for the future as we've waffled this
soooooooooo many times now
+ result.addComponent(true, context.write(null, allocator));
+ } catch (Exception ex) {
+ // We should release it as the ByteBuf is not going to be yielded
for a reader
+ result.release();
+ throw ex;
+ }
return result;
}
diff --git
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/VertexSerializer.java
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/VertexSerializer.java
index a989ab1..5ed5e16 100644
---
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/VertexSerializer.java
+++
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/VertexSerializer.java
@@ -50,12 +50,19 @@ public class VertexSerializer extends
SimpleTypeSerializer<Vertex> {
@Override
protected ByteBuf writeValue(final Vertex value, final ByteBufAllocator
allocator, final GraphBinaryWriter context) throws SerializationException {
final CompositeByteBuf result = allocator.compositeBuffer(3);
- result.addComponent(true, context.write(value.id(), allocator));
- result.addComponent(true, context.writeValue(value.label(), allocator,
false));
-
- // we don't serialize properties for graph vertices/edges. they are
"references", but we leave a place holder
- // here as an option for the future as we've waffled this soooooooooo
many times now
- result.addComponent(true, context.write(null, allocator));
+
+ try {
+ result.addComponent(true, context.write(value.id(), allocator));
+ result.addComponent(true, context.writeValue(value.label(),
allocator, false));
+
+ // we don't serialize properties for graph vertices/edges. they
are "references", but we leave a place holder
+ // here as an option for the future as we've waffled this
soooooooooo many times now
+ result.addComponent(true, context.write(null, allocator));
+ } catch (Exception ex) {
+ // We should release it as the ByteBuf is not going to be yielded
for a reader
+ result.release();
+ throw ex;
+ }
return result;
}
diff --git
a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ser/binary/TypeSerializerFailureTests.java
b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ser/binary/TypeSerializerFailureTests.java
new file mode 100644
index 0000000..c8f935e
--- /dev/null
+++
b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ser/binary/TypeSerializerFailureTests.java
@@ -0,0 +1,97 @@
+package org.apache.tinkerpop.gremlin.driver.ser.binary;
+
+import io.netty.buffer.UnpooledByteBufAllocator;
+import org.apache.tinkerpop.gremlin.driver.ser.SerializationException;
+import
org.apache.tinkerpop.gremlin.process.remote.traversal.DefaultRemoteTraverser;
+import org.apache.tinkerpop.gremlin.process.traversal.Bytecode;
+import org.apache.tinkerpop.gremlin.process.traversal.step.util.BulkSet;
+import org.apache.tinkerpop.gremlin.process.traversal.step.util.EmptyPath;
+import org.apache.tinkerpop.gremlin.process.traversal.step.util.Tree;
+import
org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalMetrics;
+import org.apache.tinkerpop.gremlin.process.traversal.util.MutableMetrics;
+import org.apache.tinkerpop.gremlin.structure.Vertex;
+import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceEdge;
+import org.apache.tinkerpop.gremlin.structure.util.reference.ReferencePath;
+import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceVertex;
+import
org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceVertexProperty;
+import org.apache.tinkerpop.gremlin.util.function.Lambda;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+@RunWith(Parameterized.class)
+public class TypeSerializerFailureTests {
+
+ private final GraphBinaryWriter writer = new GraphBinaryWriter();
+ private final UnpooledByteBufAllocator allocator = new
UnpooledByteBufAllocator(false);
+
+ @Parameterized.Parameters(name = "Value={0}")
+ public static Collection input() {
+ final Bytecode.Binding b = new Bytecode.Binding(null, "b");
+
+ final ReferenceVertex vertex = new ReferenceVertex("a vertex", null);
+
+ final Bytecode bytecode = new Bytecode();
+ bytecode.addStep(null);
+
+ final BulkSet<Object> bulkSet = new BulkSet<>();
+ bulkSet.add(vertex, 1L);
+
+ final MutableMetrics metrics = new MutableMetrics("a metric", null);
+
+ final Tree<Vertex> tree = new Tree<>();
+ tree.put(vertex, null);
+
+ // Provide instances that are malformed for serialization to fail
+ return Arrays.asList(
+ b,
+ vertex,
+ Collections.singletonMap("one", b),
+ bulkSet,
+ bytecode,
+ Collections.singletonList(vertex),
+ new ReferenceEdge("an edge", null, vertex, vertex),
+ Lambda.supplier(null),
+ metrics,
+ new DefaultTraversalMetrics(1L,
Collections.singletonList(metrics)),
+ new DefaultRemoteTraverser<>(new Object(), 1L),
+ tree,
+ new ReferenceVertexProperty<>("a prop", null, "value"),
+ new InvalidPath()
+ );
+ }
+
+ @Parameterized.Parameter(value = 0)
+ public Object value;
+
+ @Test
+ public void shouldReleaseMemoryWhenFails() {
+ try {
+ writer.write(value, allocator);
+ fail("Should throw exception");
+ } catch (SerializationException | RuntimeException e) {
+ // Do nothing
+ }
+
+ assertEquals(0, allocator.metric().usedHeapMemory());
+ }
+
+ public static class InvalidPath extends ReferencePath {
+ public InvalidPath() {
+ super(EmptyPath.instance());
+ }
+
+ @Override
+ public List<Object> objects() {
+ return Collections.singletonList(new Object());
+ }
+ }
+}
