From 10ff7c36a6cad13a4410ac4d4764222159c9dbdc Mon Sep 17 00:00:00 2001 From: Bryan Cutler Date: Wed, 26 Jul 2017 18:37:31 -0700 Subject: [PATCH 1/4] Added regression test --- .../java/org/apache/arrow/vector/TestValueVector.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java b/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java index 63543b09329..ef4d2795989 100644 --- a/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java +++ b/java/vector/src/test/java/org/apache/arrow/vector/TestValueVector.java @@ -516,4 +516,14 @@ public void testCopyFromWithNulls() { } } + @Test + public void testMultipleClose() { + BufferAllocator vectorAllocator = allocator.newChildAllocator("vector_allocator", 0, Long.MAX_VALUE); + NullableIntVector vector = newVector(NullableIntVector.class, EMPTY_SCHEMA_PATH, MinorType.INT, vectorAllocator); + vector.close(); + vectorAllocator.close(); + vector.close(); + vectorAllocator.close(); + } + } From ca38d3d8e8f585006c1855c00a3b86ca15ef97ff Mon Sep 17 00:00:00 2001 From: Bryan Cutler Date: Wed, 26 Jul 2017 18:41:59 -0700 Subject: [PATCH 2/4] use clear to release data, ensure that an empty buffer is never allocated again after closing --- .../java/org/apache/arrow/vector/BaseDataValueVector.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BaseDataValueVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BaseDataValueVector.java index 6d7d3f04a6d..21bdbab59a9 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/BaseDataValueVector.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/BaseDataValueVector.java @@ -74,19 +74,15 @@ public BaseDataValueVector(String name, BufferAllocator allocator) { public void clear() { if (data != null) { data.release(); + data = allocator.getEmpty(); } - data = allocator.getEmpty(); super.clear(); } @Override public void close() { clear(); - if (data != null) { - data.release(); - data = null; - } - super.close(); + data = null; } @Override From e992fc793ccf6a8f197be8c299334300adc1d203 Mon Sep 17 00:00:00 2001 From: Bryan Cutler Date: Mon, 31 Jul 2017 17:08:17 -0700 Subject: [PATCH 3/4] BaseDataValueVector.close will now just clear, which releases previous and assigns an empty buffer --- .../java/org/apache/arrow/memory/BaseAllocator.java | 3 --- .../org/apache/arrow/vector/BaseDataValueVector.java | 12 ++---------- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/java/memory/src/main/java/org/apache/arrow/memory/BaseAllocator.java b/java/memory/src/main/java/org/apache/arrow/memory/BaseAllocator.java index be0ba77f5b2..b38cf679e2a 100644 --- a/java/memory/src/main/java/org/apache/arrow/memory/BaseAllocator.java +++ b/java/memory/src/main/java/org/apache/arrow/memory/BaseAllocator.java @@ -171,7 +171,6 @@ public String getName() { @Override public ArrowBuf getEmpty() { - assertOpen(); return empty; } @@ -236,8 +235,6 @@ public ArrowBuf buffer(final int initialRequestSize) { } private ArrowBuf createEmpty() { - assertOpen(); - return new ArrowBuf(new AtomicInteger(), null, AllocationManager.EMPTY, null, null, 0, 0, true); } diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BaseDataValueVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BaseDataValueVector.java index 21bdbab59a9..cad27a12307 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/BaseDataValueVector.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/BaseDataValueVector.java @@ -72,19 +72,11 @@ public BaseDataValueVector(String name, BufferAllocator allocator) { @Override public void clear() { - if (data != null) { - data.release(); - data = allocator.getEmpty(); - } + data.release(); + data = allocator.getEmpty(); super.clear(); } - @Override - public void close() { - clear(); - data = null; - } - @Override public TransferPair getTransferPair(String ref, BufferAllocator allocator, CallBack callBack) { return getTransferPair(ref, allocator); From 2921d848bd7f1dc29027ec2b00e663d4b9d2327f Mon Sep 17 00:00:00 2001 From: Bryan Cutler Date: Thu, 3 Aug 2017 10:39:40 -0700 Subject: [PATCH 4/4] removed resolved comment --- .../main/java/org/apache/arrow/vector/BaseDataValueVector.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/java/vector/src/main/java/org/apache/arrow/vector/BaseDataValueVector.java b/java/vector/src/main/java/org/apache/arrow/vector/BaseDataValueVector.java index f90df2b9d63..88e02495bfc 100644 --- a/java/vector/src/main/java/org/apache/arrow/vector/BaseDataValueVector.java +++ b/java/vector/src/main/java/org/apache/arrow/vector/BaseDataValueVector.java @@ -59,8 +59,6 @@ public static List unload(List vectors) { return result; } - // TODO: Nullable vectors extend BaseDataValueVector but do not use the data field - // We should fix the inheritance tree protected ArrowBuf data; public BaseDataValueVector(String name, BufferAllocator allocator) {