Abstractions for common iterator behaviour

java/org/rocksdb/AbstractRocksIterator.java

@@ -0,0 +1,105 @@
+// Copyright (c) 2014, Facebook, Inc.  All rights reserved.
+// This source code is licensed under the BSD-style license found in the
+// LICENSE file in the root directory of this source tree. An additional grant
+// of patent rights can be found in the PATENTS file in the same directory.
+
+package org.rocksdb;
+
+/**
+ * Base class implementation for Rocks Iterators
+ * in the Java API
+ * <p/>
+ * <p>Multiple threads can invoke const methods on an RocksIterator without
+ * external synchronization, but if any of the threads may call a
+ * non-const method, all threads accessing the same RocksIterator must use
+ * external synchronization.</p>
+ *
+ * @param P The type of the Parent Object from which the Rocks Iterator was
+ *          created. This is used by disposeInternal to avoid double-free
+ *          issues with the underlying C++ object.
+ * @see org.rocksdb.RocksObject
+ */
+public abstract class AbstractRocksIterator<P extends RocksObject>
+    extends RocksObject implements RocksIteratorInterface {
+  final P parent_;
+
+  protected AbstractRocksIterator(P parent, long nativeHandle) {
+    super();
+    nativeHandle_ = nativeHandle;
+    // parent must point to a valid RocksDB instance.
+    assert (parent != null);
+    // RocksIterator must hold a reference to the related parent instance
+    // to guarantee that while a GC cycle starts RocksIterator instances
+    // are freed prior to parent instances.
+    parent_ = parent;
+  }
+
+  @Override
+  public boolean isValid() {
+    assert (isInitialized());
+    return isValid0(nativeHandle_);
+  }
+
+  @Override
+  public void seekToFirst() {
+    assert (isInitialized());
+    seekToFirst0(nativeHandle_);
+  }
+
+  @Override
+  public void seekToLast() {
+    assert (isInitialized());
+    seekToLast0(nativeHandle_);
+  }
+
+  @Override
+  public void seek(byte[] target) {
+    assert (isInitialized());
+    seek0(nativeHandle_, target, target.length);
+  }
+
+  @Override
+  public void next() {
+    assert (isInitialized());
+    next0(nativeHandle_);
+  }
+
+  @Override
+  public void prev() {
+    assert (isInitialized());
+    prev0(nativeHandle_);
+  }
+
+  @Override
+  public void status() throws RocksDBException {
+    assert (isInitialized());
+    status0(nativeHandle_);
+  }
+
+  /**
+   * <p>Deletes underlying C++ iterator pointer.</p>
+   * <p/>
+   * <p>Note: the underlying handle can only be safely deleted if the parent
+   * instance related to a certain RocksIterator is still valid and initialized.
+   * Therefore {@code disposeInternal()} checks if the parent is initialized
+   * before freeing the native handle.</p>
+   */
+  @Override
+  protected void disposeInternal() {
+    synchronized (parent_) {
+      assert (isInitialized());
+      if (parent_.isInitialized()) {
+        disposeInternal(nativeHandle_);
+      }
+    }
+  }
+
+  abstract void disposeInternal(long handle);
+  abstract boolean isValid0(long handle);
+  abstract void seekToFirst0(long handle);
+  abstract void seekToLast0(long handle);
+  abstract void next0(long handle);
+  abstract void prev0(long handle);
+  abstract void seek0(long handle, byte[] target, int targetLen);
+  abstract void status0(long handle) throws RocksDBException;
+}

java/org/rocksdb/RocksIterator.java

@@ -18,58 +18,9 @@ package org.rocksdb;
  *
  * @see org.rocksdb.RocksObject
  */
-public class RocksIterator extends RocksObject implements RocksIteratorInterface {
-  public RocksIterator(RocksDB rocksDB, long nativeHandle) {
-    super();
-    nativeHandle_ = nativeHandle;
-    // rocksDB must point to a valid RocksDB instance.
-    assert(rocksDB != null);
-    // RocksIterator must hold a reference to the related RocksDB instance
-    // to guarantee that while a GC cycle starts RocksDBIterator instances
-    // are freed prior to RocksDB instances.
-    rocksDB_ = rocksDB;
-  }
-
-  @Override
-  public boolean isValid() {
-    assert(isInitialized());
-    return isValid0(nativeHandle_);
-  }
-
-  @Override
-  public void seekToFirst() {
-    assert(isInitialized());
-    seekToFirst0(nativeHandle_);
-  }
-
-  @Override
-  public void seekToLast() {
-    assert(isInitialized());
-    seekToLast0(nativeHandle_);
-  }
-
-  @Override
-  public void seek(byte[] target) {
-    assert(isInitialized());
-    seek0(nativeHandle_, target, target.length);
-  }
-
-  @Override
-  public void next() {
-    assert(isInitialized());
-    next0(nativeHandle_);
-  }
-
-  @Override
-  public void prev() {
-    assert(isInitialized());
-    prev0(nativeHandle_);
-  }
-
-  @Override
-  public void status() throws RocksDBException {
-    assert(isInitialized());
-    status0(nativeHandle_);
+public class RocksIterator extends AbstractRocksIterator<RocksDB> {
+  protected RocksIterator(RocksDB rocksDB, long nativeHandle) {
+    super(rocksDB, nativeHandle);
   }
 
   /**
@@ -99,33 +50,15 @@ public class RocksIterator extends RocksObject implements RocksIteratorInterface
     return value0(nativeHandle_);
   }
 
-  /**
-   * <p>Deletes underlying C++ iterator pointer.</p>
-   *
-   * <p>Note: the underlying handle can only be safely deleted if the RocksDB
-   * instance related to a certain RocksIterator is still valid and initialized.
-   * Therefore {@code disposeInternal()} checks if the RocksDB is initialized
-   * before freeing the native handle.</p>
-   */
-  @Override protected void disposeInternal() {
-    synchronized (rocksDB_) {
-      assert (isInitialized());
-      if (rocksDB_.isInitialized()) {
-        disposeInternal(nativeHandle_);
-      }
-    }
-  }
+  @Override final native void disposeInternal(long handle);
+  @Override final native boolean isValid0(long handle);
+  @Override final native void seekToFirst0(long handle);
+  @Override final native void seekToLast0(long handle);
+  @Override final native void next0(long handle);
+  @Override final native void prev0(long handle);
+  @Override final native void seek0(long handle, byte[] target, int targetLen);
+  @Override final native void status0(long handle) throws RocksDBException;
 
-  private native boolean isValid0(long handle);
-  private native void disposeInternal(long handle);
-  private native void seekToFirst0(long handle);
-  private native void seekToLast0(long handle);
-  private native void next0(long handle);
-  private native void prev0(long handle);
   private native byte[] key0(long handle);
   private native byte[] value0(long handle);
-  private native void seek0(long handle, byte[] target, int targetLen);
-  private native void status0(long handle);
-
-  final RocksDB rocksDB_;
 }

java/rocksjni/iterator.cc

@@ -14,6 +14,17 @@
 #include "rocksjni/portal.h"
 #include "rocksdb/iterator.h"
 
+/*
+ * Class:     org_rocksdb_RocksIterator
+ * Method:    disposeInternal
+ * Signature: (J)V
+ */
+void Java_org_rocksdb_RocksIterator_disposeInternal(
+    JNIEnv* env, jobject jobj, jlong handle) {
+  auto it = reinterpret_cast<rocksdb::Iterator*>(handle);
+  delete it;
+}
+
 /*
  * Class:     org_rocksdb_RocksIterator
  * Method:    isValid0
@@ -36,7 +47,7 @@ void Java_org_rocksdb_RocksIterator_seekToFirst0(
 
 /*
  * Class:     org_rocksdb_RocksIterator
- * Method:    seekToFirst0
+ * Method:    seekToLast0
  * Signature: (J)V
  */
 void Java_org_rocksdb_RocksIterator_seekToLast0(
@@ -46,7 +57,7 @@ void Java_org_rocksdb_RocksIterator_seekToLast0(
 
 /*
  * Class:     org_rocksdb_RocksIterator
- * Method:    seekToLast0
+ * Method:    next0
  * Signature: (J)V
  */
 void Java_org_rocksdb_RocksIterator_next0(
@@ -56,7 +67,7 @@ void Java_org_rocksdb_RocksIterator_next0(
 
 /*
  * Class:     org_rocksdb_RocksIterator
- * Method:    next0
+ * Method:    prev0
  * Signature: (J)V
  */
 void Java_org_rocksdb_RocksIterator_prev0(
@@ -66,41 +77,8 @@ void Java_org_rocksdb_RocksIterator_prev0(
 
 /*
  * Class:     org_rocksdb_RocksIterator
- * Method:    prev0
- * Signature: (J)V
- */
-jbyteArray Java_org_rocksdb_RocksIterator_key0(
-    JNIEnv* env, jobject jobj, jlong handle) {
-  auto it = reinterpret_cast<rocksdb::Iterator*>(handle);
-  rocksdb::Slice key_slice = it->key();
-
-  jbyteArray jkey = env->NewByteArray(static_cast<jsize>(key_slice.size()));
-  env->SetByteArrayRegion(jkey, 0, static_cast<jsize>(key_slice.size()),
-                          reinterpret_cast<const jbyte*>(key_slice.data()));
-  return jkey;
-}
-
-/*
- * Class:     org_rocksdb_RocksIterator
- * Method:    key0
- * Signature: (J)[B
- */
-jbyteArray Java_org_rocksdb_RocksIterator_value0(
-    JNIEnv* env, jobject jobj, jlong handle) {
-  auto it = reinterpret_cast<rocksdb::Iterator*>(handle);
-  rocksdb::Slice value_slice = it->value();
-
-  jbyteArray jkeyValue =
-      env->NewByteArray(static_cast<jsize>(value_slice.size()));
-  env->SetByteArrayRegion(jkeyValue, 0, static_cast<jsize>(value_slice.size()),
-                          reinterpret_cast<const jbyte*>(value_slice.data()));
-  return jkeyValue;
-}
-
-/*
- * Class:     org_rocksdb_RocksIterator
- * Method:    value0
- * Signature: (J)[B
+ * Method:    seek0
+ * Signature: (J[BI)V
  */
 void Java_org_rocksdb_RocksIterator_seek0(
     JNIEnv* env, jobject jobj, jlong handle,
@@ -117,8 +95,8 @@ void Java_org_rocksdb_RocksIterator_seek0(
 
 /*
  * Class:     org_rocksdb_RocksIterator
- * Method:    seek0
- * Signature: (J[BI)V
+ * Method:    status0
+ * Signature: (J)V
  */
 void Java_org_rocksdb_RocksIterator_status0(
     JNIEnv* env, jobject jobj, jlong handle) {
@@ -134,11 +112,33 @@ void Java_org_rocksdb_RocksIterator_status0(
 
 /*
  * Class:     org_rocksdb_RocksIterator
- * Method:    disposeInternal
- * Signature: (J)V
+ * Method:    key0
+ * Signature: (J)[B
  */
-void Java_org_rocksdb_RocksIterator_disposeInternal(
+jbyteArray Java_org_rocksdb_RocksIterator_key0(
     JNIEnv* env, jobject jobj, jlong handle) {
   auto it = reinterpret_cast<rocksdb::Iterator*>(handle);
-  delete it;
+  rocksdb::Slice key_slice = it->key();
+
+  jbyteArray jkey = env->NewByteArray(static_cast<jsize>(key_slice.size()));
+  env->SetByteArrayRegion(jkey, 0, static_cast<jsize>(key_slice.size()),
+                          reinterpret_cast<const jbyte*>(key_slice.data()));
+  return jkey;
+}
+
+/*
+ * Class:     org_rocksdb_RocksIterator
+ * Method:    value0
+ * Signature: (J)[B
+ */
+jbyteArray Java_org_rocksdb_RocksIterator_value0(
+    JNIEnv* env, jobject jobj, jlong handle) {
+  auto it = reinterpret_cast<rocksdb::Iterator*>(handle);
+  rocksdb::Slice value_slice = it->value();
+
+  jbyteArray jkeyValue =
+      env->NewByteArray(static_cast<jsize>(value_slice.size()));
+  env->SetByteArrayRegion(jkeyValue, 0, static_cast<jsize>(value_slice.size()),
+                          reinterpret_cast<const jbyte*>(value_slice.data()));
+  return jkeyValue;
 }