diff --git a/java/org/rocksdb/BackupableDB.java b/java/org/rocksdb/BackupableDB.java index f57d47df1..90d4a2a9a 100644 --- a/java/org/rocksdb/BackupableDB.java +++ b/java/org/rocksdb/BackupableDB.java @@ -32,7 +32,7 @@ public class BackupableDB extends RocksDB { // Prevent the RocksDB object from attempting to delete // the underly C++ DB object. - db.disOwnNativeObject(); + db.disOwnNativeHandle(); return bdb; } diff --git a/java/org/rocksdb/BackupableDBOptions.java b/java/org/rocksdb/BackupableDBOptions.java index 2c64b60a3..f229d88aa 100644 --- a/java/org/rocksdb/BackupableDBOptions.java +++ b/java/org/rocksdb/BackupableDBOptions.java @@ -32,13 +32,12 @@ public class BackupableDBOptions extends RocksObject { * Release the memory allocated for the current instance * in the c++ side. */ - @Override public synchronized void dispose() { - if (isInitialized()) { - dispose(nativeHandle_); - } + @Override protected void disposeInternal() { + assert(isInitialized()); + disposeInternal(nativeHandle_); } private native void newBackupableDBOptions(String path); private native String backupDir(long handle); - private native void dispose(long handle); + private native void disposeInternal(long handle); } diff --git a/java/org/rocksdb/Filter.java b/java/org/rocksdb/Filter.java index 3a01ad4ee..ce5c41f26 100644 --- a/java/org/rocksdb/Filter.java +++ b/java/org/rocksdb/Filter.java @@ -22,11 +22,10 @@ public abstract class Filter extends RocksObject { * RocksDB instances referencing the filter are closed. * Otherwise an undefined behavior will occur. */ - @Override public synchronized void dispose() { - if (isInitialized()) { - dispose0(nativeHandle_); - } + @Override protected void disposeInternal() { + assert(isInitialized()); + disposeInternal(nativeHandle_); } - private native void dispose0(long handle); + private native void disposeInternal(long handle); } diff --git a/java/org/rocksdb/Options.java b/java/org/rocksdb/Options.java index 02d3e200a..af4b82fab 100644 --- a/java/org/rocksdb/Options.java +++ b/java/org/rocksdb/Options.java @@ -2311,10 +2311,9 @@ public class Options extends RocksObject { * Release the memory allocated for the current instance * in the c++ side. */ - @Override public synchronized void dispose() { - if (isInitialized()) { - dispose0(); - } + @Override protected void disposeInternal() { + assert(isInitialized()); + disposeInternal(nativeHandle_); } static final int DEFAULT_PLAIN_TABLE_BLOOM_BITS_PER_KEY = 10; @@ -2322,7 +2321,7 @@ public class Options extends RocksObject { static final int DEFAULT_PLAIN_TABLE_INDEX_SPARSENESS = 16; private native void newOptions(); - private native void dispose0(); + private native void disposeInternal(long handle); private native void setCreateIfMissing(long handle, boolean flag); private native boolean createIfMissing(long handle); private native void setWriteBufferSize(long handle, long writeBufferSize); diff --git a/java/org/rocksdb/ReadOptions.java b/java/org/rocksdb/ReadOptions.java index 23250fc6f..97c47c7d6 100644 --- a/java/org/rocksdb/ReadOptions.java +++ b/java/org/rocksdb/ReadOptions.java @@ -18,19 +18,6 @@ public class ReadOptions extends RocksObject { } private native void newReadOptions(); - /** - * Release the memory allocated for the current instance - * in the c++ side. - * - * Calling other methods after dispose() leads to undefined behavior. - */ - @Override public synchronized void dispose() { - if (isInitialized()) { - dispose(nativeHandle_); - } - } - private native void dispose(long handle); - /** * If true, all data read from underlying storage will be * verified against corresponding checksums. @@ -127,4 +114,12 @@ public class ReadOptions extends RocksObject { } private native void setTailing( long handle, boolean tailing); + + + @Override protected void disposeInternal() { + assert(isInitialized()); + disposeInternal(nativeHandle_); + } + private native void disposeInternal(long handle); + } diff --git a/java/org/rocksdb/RocksDB.java b/java/org/rocksdb/RocksDB.java index bc0390d9f..1b758e1a2 100644 --- a/java/org/rocksdb/RocksDB.java +++ b/java/org/rocksdb/RocksDB.java @@ -114,11 +114,9 @@ public class RocksDB extends RocksObject { return db; } - @Override public synchronized void dispose() { - if (isInitialized()) { - dispose(nativeHandle_); - nativeHandle_ = 0; - } + @Override protected void disposeInternal() { + assert(isInitialized()); + disposeInternal(nativeHandle_); } /** @@ -315,10 +313,6 @@ public class RocksDB extends RocksObject { return new RocksIterator(iterator0(nativeHandle_)); } - @Override protected void finalize() { - close(); - } - /** * Private constructor. */ @@ -337,18 +331,6 @@ public class RocksDB extends RocksObject { opt.filter_ = null; } - /** - * Revoke ownership of the native object. - * - * This will prevent the object from attempting to delete the underlying - * native object in its finalizer. This must be used when another object - * (e.g. BackupableDB) takes over ownership of the native object or both - * will attempt to delete the underlying object when garbage collected. - */ - protected void disOwnNativeObject() { - nativeHandle_ = 0; - } - // native methods protected native void open( long optionsHandle, long cacheSize, String path) throws RocksDBException; @@ -382,7 +364,7 @@ public class RocksDB extends RocksObject { long handle, long writeOptHandle, byte[] key, int keyLen) throws RocksDBException; protected native long iterator0(long optHandle); - protected native void dispose(long handle); + private native void disposeInternal(long handle); protected Filter filter_; } diff --git a/java/org/rocksdb/RocksIterator.java b/java/org/rocksdb/RocksIterator.java index 5b7d2c172..9ef2e8c24 100644 --- a/java/org/rocksdb/RocksIterator.java +++ b/java/org/rocksdb/RocksIterator.java @@ -118,15 +118,13 @@ public class RocksIterator extends RocksObject { /** * Deletes underlying C++ iterator pointer. */ - @Override public synchronized void dispose() { - if(isInitialized()) { - dispose(nativeHandle_); - nativeHandle_ = 0; - } + @Override protected void disposeInternal() { + assert(isInitialized()); + disposeInternal(nativeHandle_); } private native boolean isValid0(long handle); - private native void dispose(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); diff --git a/java/org/rocksdb/RocksObject.java b/java/org/rocksdb/RocksObject.java index 6e36cbaea..31c347daa 100644 --- a/java/org/rocksdb/RocksObject.java +++ b/java/org/rocksdb/RocksObject.java @@ -16,12 +16,48 @@ package org.rocksdb; public abstract class RocksObject { protected RocksObject() { nativeHandle_ = 0; + owningHandle_ = true; } /** * Release the c++ object pointed by the native handle. + * + * Note that once an instance of RocksObject has been disposed, + * calling its function will lead undefined behavior. */ - public abstract void dispose(); + public final synchronized void dispose() { + if (isOwningNativeHandle() && isInitialized()) { + disposeInternal(); + } + nativeHandle_ = 0; + disOwnNativeHandle(); + } + + /** + * The helper function of dispose() which all subclasses of RocksObject + * must implement to release their associated C++ resource. + */ + protected abstract void disposeInternal(); + + /** + * Revoke ownership of the native object. + * + * 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. + * + * When disOwnNativeHandle is called, dispose() will simply set nativeHandle_ + * to 0 without releasing its associated C++ resource. As a result, + * incorrectly use this function may cause memory leak. + */ + protected void disOwnNativeHandle() { + owningHandle_ = false; + } + + protected boolean isOwningNativeHandle() { + return owningHandle_; + } protected boolean isInitialized() { return (nativeHandle_ != 0); @@ -32,4 +68,5 @@ public abstract class RocksObject { } protected long nativeHandle_; + private boolean owningHandle_; } diff --git a/java/org/rocksdb/WriteBatch.java b/java/org/rocksdb/WriteBatch.java index 1ddbd449d..f538dc1a0 100644 --- a/java/org/rocksdb/WriteBatch.java +++ b/java/org/rocksdb/WriteBatch.java @@ -86,10 +86,9 @@ public class WriteBatch extends RocksObject { /** * Delete the c++ side pointer. */ - @Override public synchronized void dispose() { - if (isInitialized()) { - dispose0(); - } + @Override protected void disposeInternal() { + assert(isInitialized()); + disposeInternal(nativeHandle_); } private native void newWriteBatch(int reserved_bytes); @@ -99,7 +98,7 @@ public class WriteBatch extends RocksObject { byte[] value, int valueLen); private native void remove(byte[] key, int keyLen); private native void putLogData(byte[] blob, int blobLen); - private native void dispose0(); + private native void disposeInternal(long handle); } /** diff --git a/java/org/rocksdb/WriteOptions.java b/java/org/rocksdb/WriteOptions.java index f4a1d6a0f..d26dbb918 100644 --- a/java/org/rocksdb/WriteOptions.java +++ b/java/org/rocksdb/WriteOptions.java @@ -17,10 +17,9 @@ public class WriteOptions extends RocksObject { newWriteOptions(); } - @Override public synchronized void dispose() { - if (isInitialized()) { - dispose0(nativeHandle_); - } + @Override protected void disposeInternal() { + assert(isInitialized()); + disposeInternal(nativeHandle_); } /** @@ -96,5 +95,5 @@ public class WriteOptions extends RocksObject { private native boolean sync(long handle); private native void setDisableWAL(long handle, boolean flag); private native boolean disableWAL(long handle); - private native void dispose0(long handle); + private native void disposeInternal(long handle); } diff --git a/java/rocksjni/backupablejni.cc b/java/rocksjni/backupablejni.cc index 8b57a0c62..956912ef1 100644 --- a/java/rocksjni/backupablejni.cc +++ b/java/rocksjni/backupablejni.cc @@ -72,10 +72,10 @@ jstring Java_org_rocksdb_BackupableDBOptions_backupDir( /* * Class: org_rocksdb_BackupableDBOptions - * Method: dispose + * Method: disposeInternal * Signature: (J)V */ -void Java_org_rocksdb_BackupableDBOptions_dispose( +void Java_org_rocksdb_BackupableDBOptions_disposeInternal( JNIEnv* env, jobject jopt, jlong jhandle) { auto bopt = reinterpret_cast(jhandle); assert(bopt); diff --git a/java/rocksjni/filter.cc b/java/rocksjni/filter.cc index 7ef959814..572b4a66d 100644 --- a/java/rocksjni/filter.cc +++ b/java/rocksjni/filter.cc @@ -29,13 +29,10 @@ void Java_org_rocksdb_BloomFilter_createNewFilter0( /* * Class: org_rocksdb_Filter - * Method: dispose0 + * Method: disposeInternal * Signature: (J)V */ -void Java_org_rocksdb_Filter_dispose0( +void Java_org_rocksdb_Filter_disposeInternal( JNIEnv* env, jobject jobj, jlong handle) { - auto fp = reinterpret_cast(handle); - delete fp; - - rocksdb::FilterJni::setHandle(env, jobj, nullptr); + delete reinterpret_cast(handle); } diff --git a/java/rocksjni/iterator.cc b/java/rocksjni/iterator.cc index 4c18a3491..84b0b3133 100644 --- a/java/rocksjni/iterator.cc +++ b/java/rocksjni/iterator.cc @@ -135,10 +135,10 @@ void Java_org_rocksdb_RocksIterator_status0( /* * Class: org_rocksdb_RocksIterator - * Method: dispose + * Method: disposeInternal * Signature: (J)V */ -void Java_org_rocksdb_RocksIterator_dispose( +void Java_org_rocksdb_RocksIterator_disposeInternal( JNIEnv* env, jobject jobj, jlong handle) { auto it = reinterpret_cast(handle); delete it; diff --git a/java/rocksjni/options.cc b/java/rocksjni/options.cc index c5849ce39..003d353e6 100644 --- a/java/rocksjni/options.cc +++ b/java/rocksjni/options.cc @@ -35,14 +35,12 @@ void Java_org_rocksdb_Options_newOptions(JNIEnv* env, jobject jobj) { /* * Class: org_rocksdb_Options - * Method: dispose0 - * Signature: ()V + * Method: disposeInternal + * Signature: (J)V */ -void Java_org_rocksdb_Options_dispose0(JNIEnv* env, jobject jobj) { - rocksdb::Options* op = rocksdb::OptionsJni::getHandle(env, jobj); - delete op; - - rocksdb::OptionsJni::setHandle(env, jobj, nullptr); +void Java_org_rocksdb_Options_disposeInternal( + JNIEnv* env, jobject jobj, jlong handle) { + delete reinterpret_cast(handle); } /* @@ -1665,10 +1663,10 @@ void Java_org_rocksdb_WriteOptions_newWriteOptions( /* * Class: org_rocksdb_WriteOptions - * Method: dispose0 + * Method: disposeInternal * Signature: ()V */ -void Java_org_rocksdb_WriteOptions_dispose0( +void Java_org_rocksdb_WriteOptions_disposeInternal( JNIEnv* env, jobject jwrite_options, jlong jhandle) { auto write_options = reinterpret_cast(jhandle); delete write_options; @@ -1732,10 +1730,10 @@ void Java_org_rocksdb_ReadOptions_newReadOptions( /* * Class: org_rocksdb_ReadOptions - * Method: dispose + * Method: disposeInternal * Signature: (J)V */ -void Java_org_rocksdb_ReadOptions_dispose( +void Java_org_rocksdb_ReadOptions_disposeInternal( JNIEnv* env, jobject jobj, jlong jhandle) { delete reinterpret_cast(jhandle); rocksdb::ReadOptionsJni::setHandle(env, jobj, nullptr); diff --git a/java/rocksjni/rocksjni.cc b/java/rocksjni/rocksjni.cc index 1f2941f42..697bd0cef 100644 --- a/java/rocksjni/rocksjni.cc +++ b/java/rocksjni/rocksjni.cc @@ -419,14 +419,12 @@ void Java_org_rocksdb_RocksDB_remove__JJ_3BI( /* * Class: org_rocksdb_RocksDB - * Method: dispose + * Method: disposeInternal * Signature: (J)V */ -void Java_org_rocksdb_RocksDB_dispose( +void Java_org_rocksdb_RocksDB_disposeInternal( JNIEnv* env, jobject java_db, jlong jhandle) { - auto db = reinterpret_cast(jhandle); - assert(db != nullptr); - delete db; + delete reinterpret_cast(jhandle); } /* diff --git a/java/rocksjni/write_batch.cc b/java/rocksjni/write_batch.cc index 035b35f6f..e8b2456ee 100644 --- a/java/rocksjni/write_batch.cc +++ b/java/rocksjni/write_batch.cc @@ -134,15 +134,12 @@ void Java_org_rocksdb_WriteBatch_putLogData( /* * Class: org_rocksdb_WriteBatch - * Method: dispose0 - * Signature: ()V + * Method: disposeInternal + * Signature: (J)V */ -void Java_org_rocksdb_WriteBatch_dispose0(JNIEnv* env, jobject jobj) { - rocksdb::WriteBatch* wb = rocksdb::WriteBatchJni::getHandle(env, jobj); - assert(wb != nullptr); - delete wb; - - rocksdb::WriteBatchJni::setHandle(env, jobj, nullptr); +void Java_org_rocksdb_WriteBatch_disposeInternal( + JNIEnv* env, jobject jobj, jlong handle) { + delete reinterpret_cast(handle); } /*