remove deprecated dispose() for Rocks JNI interface Java objects. (#9523)

Summary:
For RocksDB 7. Remove deprecated dispose() And as a consequence remove finalize(), which is good Modern Java hygiene.

It is extremely non-deterministic when `finalize()` is called on an object, and resource closure/recovery of underlying native/C++ objects and/or non-memory resource cannot be adequately controlled through GC finalization. The RocksDB Java/JNI interface provides and encourages the use of AutoCloseable objects with close() methods, allowing predictable disposal of resources at exit from try-with-resource blocks.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/9523

Reviewed By: mrambacher

Differential Revision: D34079843

Pulled By: jay-zhuang

fbshipit-source-id: d1f0463a89a548b5d57bfaa50154379e722d189a
This commit is contained in:
Alan Paxton 2022-02-09 11:31:41 -08:00 committed by Facebook GitHub Bot
parent 685044dff2
commit 99d86252b6
6 changed files with 25 additions and 56 deletions

View File

@ -36,16 +36,14 @@ public abstract class AbstractImmutableNativeReference
* freeing the underlying native C++ object * freeing the underlying native C++ object
* <p> * <p>
* This will prevent the object from attempting to delete the underlying * This will prevent the object from attempting to delete the underlying
* native object in its finalizer. This must be used when another object * native object in {@code close()}. This must be used when another object
* takes over ownership of the native object or both will attempt to delete * takes over ownership of the native object or both will attempt to delete
* the underlying object when garbage collected. * the underlying object when closed.
* <p> * <p>
* When {@code disOwnNativeHandle()} is called, {@code dispose()} will * When {@code disOwnNativeHandle()} is called, {@code close()} will
* subsequently take no action. As a result, incorrect use of this function * subsequently take no action. As a result, incorrect use of this function
* may cause a memory leak. * may cause a memory leak.
* </p> * </p>
*
* @see #dispose()
*/ */
protected final void disOwnNativeHandle() { protected final void disOwnNativeHandle() {
owningHandle_.set(false); owningHandle_.set(false);
@ -59,7 +57,7 @@ public abstract class AbstractImmutableNativeReference
} }
/** /**
* The helper function of {@link AbstractImmutableNativeReference#dispose()} * The helper function of {@link AbstractImmutableNativeReference#close()}
* which all subclasses of {@code AbstractImmutableNativeReference} must * which all subclasses of {@code AbstractImmutableNativeReference} must
* implement to release their underlying native C++ objects. * implement to release their underlying native C++ objects.
*/ */

View File

@ -9,26 +9,28 @@ package org.rocksdb;
* AbstractNativeReference is the base-class of all RocksDB classes that have * AbstractNativeReference is the base-class of all RocksDB classes that have
* a pointer to a native C++ {@code rocksdb} object. * a pointer to a native C++ {@code rocksdb} object.
* <p> * <p>
* AbstractNativeReference has the {@link AbstractNativeReference#dispose()} * AbstractNativeReference has the {@link AbstractNativeReference#close()}
* method, which frees its associated C++ object.</p> * method, which frees its associated C++ object.</p>
* <p> * <p>
* This function should be called manually, however, if required it will be * This function should be called manually, or even better, called implicitly using a
* <a
* href="https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html">try-with-resources</a>
* statement, when you are finished with the object. It is no longer
* called automatically during the regular Java GC process via * called automatically during the regular Java GC process via
* {@link AbstractNativeReference#finalize()}.</p> * {@link AbstractNativeReference#finalize()}.</p>
* <p> * <p>
* Note - Java can only see the long member variable (which is the C++ pointer * Explanatory note - When or if the Garbage Collector calls {@link Object#finalize()}
* value to the native object), as such it does not know the real size of the * depends on the JVM implementation and system conditions, which the programmer
* object and therefore may assign a low GC priority for it; So it is strongly * cannot control. In addition, the GC cannot see through the native reference
* suggested that you manually dispose of objects when you are finished with * long member variable (which is the C++ pointer value to the native object),
* them.</p> * and cannot know what other resources depend on it.
* </p>
*/ */
public abstract class AbstractNativeReference implements AutoCloseable { public abstract class AbstractNativeReference implements AutoCloseable {
/** /**
* Returns true if we are responsible for freeing the underlying C++ object * Returns true if we are responsible for freeing the underlying C++ object
* *
* @return true if we are responsible to free the C++ object * @return true if we are responsible to free the C++ object
* @see #dispose()
*/ */
protected abstract boolean isOwningHandle(); protected abstract boolean isOwningHandle();
@ -39,38 +41,8 @@ public abstract class AbstractNativeReference implements AutoCloseable {
* have finished using the object.</p> * have finished using the object.</p>
* <p> * <p>
* Note, that once an instance of {@link AbstractNativeReference} has been * Note, that once an instance of {@link AbstractNativeReference} has been
* disposed, calling any of its functions will lead to undefined * closed, calling any of its functions will lead to undefined
* behavior.</p> * behavior.</p>
*/ */
@Override @Override public abstract void close();
public abstract void close();
/**
* @deprecated Instead use {@link AbstractNativeReference#close()}
*/
@Deprecated
public final void dispose() {
close();
}
/**
* Simply calls {@link AbstractNativeReference#dispose()} to free
* any underlying C++ object reference which has not yet been manually
* released.
*
* @deprecated You should not rely on GC of Rocks objects, and instead should
* either call {@link AbstractNativeReference#close()} manually or make
* use of some sort of ARM (Automatic Resource Management) such as
* Java 7's <a href="https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html">try-with-resources</a>
* statement
*/
@Override
@Deprecated
protected void finalize() throws Throwable {
if (isOwningHandle()) {
//TODO(AR) log a warning message... developer should have called close()
}
dispose();
super.finalize();
}
} }

View File

@ -552,9 +552,8 @@ public class BlockBasedTableConfig extends TableFormatConfig {
/** /**
* Use the specified filter policy to reduce disk reads. * Use the specified filter policy to reduce disk reads.
* *
* {@link org.rocksdb.Filter} should not be disposed before options instances * {@link org.rocksdb.Filter} should not be closed before options instances
* using this filter is disposed. If {@link Filter#dispose()} function is not * using this filter are closed.
* called, then filter object will be GC'd automatically.
* *
* {@link org.rocksdb.Filter} instance can be re-used in multiple options * {@link org.rocksdb.Filter} instance can be re-used in multiple options
* instances. * instances.

View File

@ -12,8 +12,8 @@ import java.util.*;
* ColumnFamilyOptions to control the behavior of a database. It will be used * ColumnFamilyOptions to control the behavior of a database. It will be used
* during the creation of a {@link org.rocksdb.RocksDB} (i.e., RocksDB.open()). * during the creation of a {@link org.rocksdb.RocksDB} (i.e., RocksDB.open()).
* *
* If {@link #dispose()} function is not called, then it will be GC'd * As a descendent of {@link AbstractNativeReference}, this class is {@link AutoCloseable}
* automatically and native resources will be released as part of the process. * and will be automatically released if opened in the preamble of a try with resources block.
*/ */
public class ColumnFamilyOptions extends RocksObject public class ColumnFamilyOptions extends RocksObject
implements ColumnFamilyOptionsInterface<ColumnFamilyOptions>, implements ColumnFamilyOptionsInterface<ColumnFamilyOptions>,

View File

@ -12,8 +12,8 @@ import java.util.*;
* DBOptions to control the behavior of a database. It will be used * DBOptions to control the behavior of a database. It will be used
* during the creation of a {@link org.rocksdb.RocksDB} (i.e., RocksDB.open()). * during the creation of a {@link org.rocksdb.RocksDB} (i.e., RocksDB.open()).
* *
* If {@link #dispose()} function is not called, then it will be GC'd * As a descendent of {@link AbstractNativeReference}, this class is {@link AutoCloseable}
* automatically and native resources will be released as part of the process. * and will be automatically released if opened in the preamble of a try with resources block.
*/ */
public class DBOptions extends RocksObject public class DBOptions extends RocksObject
implements DBOptionsInterface<DBOptions>, implements DBOptionsInterface<DBOptions>,

View File

@ -12,8 +12,8 @@ import java.util.*;
* Options to control the behavior of a database. It will be used * Options to control the behavior of a database. It will be used
* during the creation of a {@link org.rocksdb.RocksDB} (i.e., RocksDB.open()). * during the creation of a {@link org.rocksdb.RocksDB} (i.e., RocksDB.open()).
* *
* If {@link #dispose()} function is not called, then it will be GC'd * As a descendent of {@link AbstractNativeReference}, this class is {@link AutoCloseable}
* automatically and native resources will be released as part of the process. * and will be automatically released if opened in the preamble of a try with resources block.
*/ */
public class Options extends RocksObject public class Options extends RocksObject
implements DBOptionsInterface<Options>, implements DBOptionsInterface<Options>,