RspRowSequence and SortedRangesRowSequence should invalidate their Ro…

…wSequenceAsChunkImpl on reset instead of closing it (#5302)

engine/rowset/src/main/java/io/deephaven/engine/rowset/impl/RowSequenceAsChunkImpl.java

@@ -9,6 +9,8 @@
 import io.deephaven.chunk.LongChunk;
 import io.deephaven.chunk.WritableLongChunk;
 
+import javax.annotation.OverridingMethodsMustInvokeSuper;
+
 public abstract class RowSequenceAsChunkImpl implements RowSequence {
 
     private WritableLongChunk<OrderedRowKeys> keyIndicesChunk;
@@ -99,10 +101,17 @@ public final LongChunk<OrderedRowKeyRanges> asRowKeyRangesChunk() {
     abstract public long rangesCountUpperBound();
 
     @Override
+    @OverridingMethodsMustInvokeSuper
     public void close() {
         closeRowSequenceAsChunkImpl();
     }
 
+    /**
+     * Close any resources associated with this RowSequenceAsChunkImpl. This is the implementation for {@link #close()
+     * close}, made available for subclasses that have a need to release parent class resources independently of their
+     * own {@link #close() close} implementation. Most uses should prefer to {@link #invalidateRowSequenceAsChunkImpl()
+     * invalidate}, instead.
+     */
     protected final void closeRowSequenceAsChunkImpl() {
         if (keyIndicesChunk != null) {
             keyIndicesChunk.close();

engine/rowset/src/main/java/io/deephaven/engine/rowset/impl/ShiftedRowSequence.java

@@ -106,7 +106,7 @@ public long getRelativePosition() {
     }
 
     @Override
-    public Iterator getRowSequenceIterator() {
+    public RowSequence.Iterator getRowSequenceIterator() {
         return new Iterator();
     }
 
@@ -179,7 +179,7 @@ public boolean forEachRowKeyRange(LongRangeAbortableConsumer consumer) {
 
     @Override
     public void close() {
-        closeRowSequenceAsChunkImpl();
+        super.close();
         clear();
     }
 

engine/rowset/src/main/java/io/deephaven/engine/rowset/impl/WritableRowSetImpl.java

@@ -57,7 +57,7 @@ public final WritableRowSet copy() {
     public TrackingWritableRowSet toTracking() {
         final TrackingWritableRowSet result = new TrackingWritableRowSetImpl(innerSet);
         innerSet = null; // Force NPE on use after tracking
-        closeRowSequenceAsChunkImpl();
+        super.close();
         return result;
     }
 
@@ -66,7 +66,7 @@ public TrackingWritableRowSet toTracking() {
     public void close() {
         innerSet.ixRelease();
         innerSet = null; // Force NPE on use after close
-        closeRowSequenceAsChunkImpl();
+        super.close();
     }
 
     @VisibleForTesting

engine/rowset/src/main/java/io/deephaven/engine/rowset/impl/rsp/RspRowSequence.java

@@ -59,16 +59,12 @@ private static RspArray wrapRspArray(final RspArray arr) {
 
     @Override
     public void close() {
-        closeRspRowSequence();
-    }
-
-    protected final void closeRspRowSequence() {
         if (arr == null) {
             return;
         }
         arr.release();
         arr = null;
-        closeRowSequenceAsChunkImpl();
+        super.close();
     }
 
     private RspRowSequence(final RspArray arr) {
@@ -134,12 +130,12 @@ private void reset(final int startIdx, final long startOffset, final long cardBe
         this.cardBeforeStartIdx = cardBeforeStartIdx;
         this.cardBeforeEndIdx = cardBeforeEndIdx;
         this.firstKey = firstKey;
-        closeRowSequenceAsChunkImpl();
+        invalidateRowSequenceAsChunkImpl();
         lastKey = -1;
     }
 
     @Override
-    public Iterator getRowSequenceIterator() {
+    public RowSequence.Iterator getRowSequenceIterator() {
         return new Iterator(this);
     }
 
@@ -249,9 +245,8 @@ public boolean forEachRowKeyRange(final LongRangeAbortableConsumer lrac) {
         return true;
     }
 
-    // Note unlike RowSet.Iterator, this Iterator will /not/ automatically release its underlying RowSequence
-    // representation
-    // when iteration is exhausted. The API for OK.Iterator makes that impossible.
+    // Note: Unlike RowSet.Iterator, this Iterator will /not/ automatically release its underlying RowSequence
+    // representation when iteration is exhausted. The API for RowSequence.Iterator makes that impossible.
     static class Iterator implements RowSequence.Iterator {
         private static class RSWrapper extends RspRowSequence {
             RSWrapper(final RspArray arr) {
@@ -264,9 +259,8 @@ public void close() {
                     throw new IllegalStateException();
                 }
                 // We purposely /do not/ close the RspRowSequence part as it will get reused.
-                // The API doc for Iterator states that clients should /never/ call close. So that we eneded up here
-                // means
-                // there is some kind of bug.
+                // The API doc for Iterator states that clients should /never/ call close. So that we ended up here
+                // means there is some kind of bug.
                 closeRowSequenceAsChunkImpl();
             }
         }

engine/rowset/src/main/java/io/deephaven/engine/rowset/impl/sortedranges/SortedRangesRowSequence.java

@@ -73,20 +73,16 @@ private SortedRangesRowSequence(final SortedRanges sar, final long startPos) {
 
     @Override
     public void close() {
-        closeSortedArrayRowSequence();
-    }
-
-    protected final void closeSortedArrayRowSequence() {
         if (sar == null) {
             return;
         }
         sar.release();
         sar = null;
-        closeRowSequenceAsChunkImpl();
+        super.close();
     }
 
     @Override
-    public Iterator getRowSequenceIterator() {
+    public RowSequence.Iterator getRowSequenceIterator() {
         return new Iterator(this);
     }
 
@@ -361,7 +357,7 @@ public long rangesCountUpperBound() {
     private void reset(final long startPos, final int startIdx, final long startOffset,
             final int endIdx, final long endOffset, final long size) {
         if (sar != null) {
-            closeRowSequenceAsChunkImpl();
+            invalidateRowSequenceAsChunkImpl();
         }
         this.startPos = startPos;
         this.startIdx = startIdx;
@@ -383,10 +379,9 @@ public void close() {
                 if (SortedRanges.DEBUG) {
                     throw new IllegalStateException();
                 }
-                // We purposely /do not/ close the RspRowSequence part as it will get reused.
-                // The API doc for Iterator states that clients should /never/ call close. So that we eneded up here
-                // means
-                // there is some kind of bug.
+                // We purposely /do not/ close the SortedRangesRowSequence part as it will get reused.
+                // The API doc for Iterator states that clients should /never/ call close. So that we ended up here
+                // means there is some kind of bug.
                 closeRowSequenceAsChunkImpl();
             }
         }