Fix the javadoc and the formatting of the base-classes for objects with native references

java/src/main/java/org/rocksdb/AbstractComparator.java

@@ -14,7 +14,8 @@ package org.rocksdb;
  *   @see org.rocksdb.Comparator
  *   @see org.rocksdb.DirectComparator
  */
-public abstract class AbstractComparator<T extends AbstractSlice<?>> extends NativeReference {
+public abstract class AbstractComparator<T extends AbstractSlice<?>>
+    extends AbstractImmutableNativeReference {
 
   protected AbstractComparator() {
     super(true);

java/src/main/java/org/rocksdb/AbstractImmutableNativeReference.java

@@ -0,0 +1,66 @@
+// Copyright (c) 2016, 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;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+
+/**
+ * Offers functionality for implementations of
+ * {@link AbstractNativeReference} which have an immutable reference to the
+ * underlying native C++ object
+ */
+public abstract class AbstractImmutableNativeReference
+    extends AbstractNativeReference {
+
+  /**
+   * A flag indicating whether the current {@code AbstractNativeReference} is
+   * responsible to free the underlying C++ object
+   */
+  private final AtomicBoolean owningHandle_;
+
+  protected AbstractImmutableNativeReference(final boolean owningHandle) {
+    this.owningHandle_ = new AtomicBoolean(owningHandle);
+  }
+
+  @Override
+  public boolean isOwningHandle() {
+    return owningHandle_.get();
+  }
+
+  /**
+   * Releases this {@code AbstractNativeReference} from  the responsibility of
+   * freeing the underlying native C++ object
+   * <p>
+   * This will prevent the object from attempting to delete the underlying
+   * native object in its finalizer. This must be used when another object
+   * takes over ownership of the native object or both will attempt to delete
+   * the underlying object when garbage collected.
+   * <p>
+   * When {@code disOwnNativeHandle()} is called, {@code dispose()} will
+   * subsequently take no action. As a result, incorrect use of this function
+   * may cause a memory leak.
+   * </p>
+   *
+   * @see #dispose()
+   */
+  protected final void disOwnNativeHandle() {
+    owningHandle_.set(false);
+  }
+
+  @Override
+  public final void dispose() {
+    if (owningHandle_.compareAndSet(true, false)) {
+      disposeInternal();
+    }
+  }
+
+  /**
+   * The helper function of {@link AbstractImmutableNativeReference#dispose()}
+   * which all subclasses of {@code AbstractImmutableNativeReference} must
+   * implement to release their underlying native C++ objects.
+   */
+  protected abstract void disposeInternal();
+}

java/src/main/java/org/rocksdb/AbstractNativeReference.java

@@ -0,0 +1,57 @@
+// Copyright (c) 2016, 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;
+
+/**
+ * AbstractNativeReference is the base-class of all RocksDB classes that have
+ * a pointer to a native C++ {@code rocksdb} object.
+ * <p>
+ * AbstractNativeReference has the {@link AbstractNativeReference#dispose()}
+ * method, which frees its associated C++ object.</p>
+ * <p>
+ * This function should be called manually, however, if required it will be
+ * called automatically during the regular Java GC process via
+ * {@link AbstractNativeReference#finalize()}.</p>
+ * <p>
+ * Note - Java can only see the long member variable (which is the C++ pointer
+ * value to the native object), as such it does not know the real size of the
+ * object and therefore may assign a low GC priority for it; So it is strongly
+ * suggested that you manually dispose of objects when you are finished with
+ * them.</p>
+ */
+public abstract class AbstractNativeReference {
+
+  /**
+   * Returns true if we are responsible for freeing the underlying C++ object
+   *
+   * @return true if we are responsible to free the C++ object
+   * @see #dispose()
+   */
+  protected abstract boolean isOwningHandle();
+
+  /**
+   * Frees the underlying C++ object
+   * <p>
+   * It is strong recommended that the developer calls this after they
+   * have finished using the object.</p>
+   * <p>
+   * Note, that once an instance of {@link AbstractNativeReference} has been
+   * disposed, calling any of its functions will lead to undefined
+   * behavior.</p>
+   */
+  public abstract void dispose();
+
+  /**
+   * Simply calls {@link AbstractNativeReference#dispose()} to free
+   * any underlying C++ object reference which has not yet been manually
+   * released.
+   */
+  @Override
+  protected void finalize() throws Throwable {
+    dispose();
+    super.finalize();
+  }
+}

java/src/main/java/org/rocksdb/Logger.java

@@ -35,7 +35,7 @@ package org.rocksdb;
  * {@link org.rocksdb.InfoLogLevel#FATAL_LEVEL}.
  * </p>
  */
-public abstract class Logger extends NativeReference {
+public abstract class Logger extends AbstractImmutableNativeReference {
 
   final long nativeHandle_;
 

java/src/main/java/org/rocksdb/NativeReference.java

@@ -1,77 +0,0 @@
-package org.rocksdb;
-
-import java.util.concurrent.atomic.AtomicBoolean;
-
-public abstract class NativeReference {
-
-    /**
-     * A flag indicating whether the current {@code RocksObject} is responsible to
-     * release the c++ object stored in its {@code nativeHandle_}.
-     */
-    private final AtomicBoolean owningHandle_;
-
-    protected NativeReference(final boolean owningHandle) {
-        this.owningHandle_ = new AtomicBoolean(owningHandle);
-    }
-
-    public boolean isOwningHandle() {
-        return owningHandle_.get();
-    }
-
-    /**
-     * Revoke ownership of the native object.
-     * <p>
-     * This will prevent the object from attempting to delete the underlying
-     * native object in its finalizer. This must be used when another object
-     * takes over ownership of the native object or both will attempt to delete
-     * the underlying object when garbage collected.
-     * <p>
-     * When {@code disOwnNativeHandle()} is called, {@code dispose()} will simply set
-     * {@code nativeHandle_} to 0 without releasing its associated C++ resource.
-     * As a result, incorrectly use this function may cause memory leak, and this
-     * function call will not affect the return value of {@code isInitialized()}.
-     * </p>
-     * @see #dispose()
-     */
-    protected final void disOwnNativeHandle() {
-        owningHandle_.set(false);
-    }
-
-    /**
-     * Release the c++ object manually pointed by the native handle.
-     * <p>
-     * Note that {@code dispose()} will also be called during the GC process
-     * if it was not called before its {@code RocksObject} went out-of-scope.
-     * However, since Java may wrongly wrongly assume those objects are
-     * small in that they seems to only hold a long variable. As a result,
-     * they might have low priority in the GC process.  To prevent this,
-     * it is suggested to call {@code dispose()} manually.
-     * </p>
-     * <p>
-     * Note that once an instance of {@code RocksObject} has been disposed,
-     * calling its function will lead undefined behavior.
-     * </p>
-     */
-    public final void dispose() {
-        if (owningHandle_.compareAndSet(true, false)) {
-            disposeInternal();
-        }
-    }
-
-    /**
-     * The helper function of {@code dispose()} which all subclasses of
-     * {@code RocksObject} must implement to release their associated
-     * C++ resource.
-     */
-    protected abstract void disposeInternal();
-
-    /**
-     * Simply calls {@code dispose()} and release its c++ resource if it has not
-     * yet released.
-     */
-    @Override
-    protected void finalize() throws Throwable {
-        dispose();
-        super.finalize();
-    }
-}

java/src/main/java/org/rocksdb/RocksMutableObject.java

@@ -1,50 +1,69 @@
+// Copyright (c) 2016, 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;
 
-public abstract class RocksMutableObject /*extends NativeReference*/ {
+/**
+ * RocksMutableObject is an implementation of {@link AbstractNativeReference}
+ * whose reference to the underlying native C++ object can change.
+ *
+ * <p>The use of {@code RocksMutableObject} should be kept to a minimum, as it
+ * has synchronization overheads and introduces complexity. Instead it is
+ * recommended to use {@link RocksObject} where possible.</p>
+ */
+public abstract class RocksMutableObject extends AbstractNativeReference {
 
-    private long nativeHandle_;
-    private boolean owningHandle_;
+  /**
+   * An mutable reference to the value of the C++ pointer pointing to some
+   * underlying native RocksDB C++ object.
+   */
+  private long nativeHandle_;
+  private boolean owningHandle_;
 
-    protected RocksMutableObject() {
+  protected RocksMutableObject() {
+  }
+
+  protected RocksMutableObject(final long nativeHandle) {
+    this.nativeHandle_ = nativeHandle;
+    this.owningHandle_ = true;
+  }
+
+  public synchronized void setNativeHandle(final long nativeHandle,
+      final boolean owningNativeHandle) {
+    this.nativeHandle_ = nativeHandle;
+    this.owningHandle_ = owningNativeHandle;
+  }
+
+  @Override
+  protected synchronized boolean isOwningHandle() {
+    return this.owningHandle_;
+  }
+
+  /**
+   * Gets the value of the C++ pointer pointing to the underlying
+   * native C++ object
+   *
+   * @return the pointer value for the native object
+   */
+  protected synchronized long getNativeHandle() {
+    assert (this.nativeHandle_ != 0);
+    return this.nativeHandle_;
+  }
+
+  @Override
+  public synchronized final void dispose() {
+    if (isOwningHandle()) {
+      disposeInternal();
+      this.owningHandle_ = false;
+      this.nativeHandle_ = 0;
     }
+  }
 
-    protected RocksMutableObject(final long nativeHandle) {
-        this.nativeHandle_ = nativeHandle;
-        this.owningHandle_ = true;
-    }
+  protected void disposeInternal() {
+    disposeInternal(nativeHandle_);
+  }
 
-    public synchronized void setNativeHandle(final long nativeHandle, final boolean owningNativeHandle) {
-        this.nativeHandle_ = nativeHandle;
-        this.owningHandle_ = owningNativeHandle;
-    }
-
-    //@Override
-    protected synchronized boolean isOwningHandle() {
-        return this.owningHandle_;
-    }
-
-    protected synchronized long getNativeHandle() {
-        assert(this.nativeHandle_ != 0);
-        return this.nativeHandle_;
-    }
-
-    public synchronized final void dispose() {
-        if(isOwningHandle()) {
-            disposeInternal();
-            this.owningHandle_ = false;
-            this.nativeHandle_ = 0;
-        }
-    }
-
-    @Override
-    protected void finalize() throws Throwable {
-        dispose();
-        super.finalize();
-    }
-
-    protected void disposeInternal() {
-        disposeInternal(nativeHandle_);
-    }
-
-    protected abstract void disposeInternal(final long handle);
+  protected abstract void disposeInternal(final long handle);
 }

java/src/main/java/org/rocksdb/RocksObject.java

@@ -6,26 +6,21 @@
 package org.rocksdb;
 
 /**
- * RocksObject is the base-class of all RocksDB classes that has a pointer to
- * some c++ {@code rocksdb} object.
- *
+ * RocksObject is an implementation of {@link AbstractNativeReference} which
+ * has an immutable and therefore thread-safe reference to the underlying
+ * native C++ RocksDB object.
  * <p>
- * RocksObject has {@code dispose()} function, which releases its associated c++
- * resource.</p>
+ * RocksObject is the base-class of almost all RocksDB classes that have a
+ * pointer to some underlying native C++ {@code rocksdb} object.</p>
  * <p>
- * This function can be either called manually, or being called automatically
- * during the regular Java GC process. However, since Java may wrongly assume a
- * RocksObject only contains a long member variable and think it is small in size,
- * Java may give {@code RocksObject} low priority in the GC process. For this, it is
- * suggested to call {@code dispose()} manually. However, it is safe to let
- * {@code RocksObject} go out-of-scope without manually calling {@code dispose()}
- * as {@code dispose()} will be called in the finalizer during the
- * regular GC process.</p>
+ * The use of {@code RocksObject} should always be preferred over
+ * {@link RocksMutableObject}.</p>
  */
-public abstract class RocksObject extends NativeReference {
+public abstract class RocksObject extends AbstractImmutableNativeReference {
 
   /**
-   * A long variable holding c++ pointer pointing to some RocksDB C++ object.
+   * An immutable reference to the value of the C++ pointer pointing to some
+   * underlying native RocksDB C++ object.
    */
   protected final long nativeHandle_;
 

java/src/main/java/org/rocksdb/WriteBatch.java

@@ -90,7 +90,7 @@ public class WriteBatch extends AbstractWriteBatch {
   /**
    * Handler callback for iterating over the contents of a batch.
    */
-  public static abstract class Handler extends NativeReference {
+  public static abstract class Handler extends AbstractImmutableNativeReference {
     private final long nativeHandle_;
     public Handler() {
       super(true);