From bafbc23baa6a1bd2aac6575d66a89379278b815a Mon Sep 17 00:00:00 2001 From: fyrz Date: Tue, 14 Oct 2014 18:38:50 +0200 Subject: [PATCH] Filters getting disposed by System.gc before EOL Previous to this commit Filters passed as parameters to the BlockTableConfig are disposed before they should be disposed. Further Smart pointer usage was corrected. Java holds now the smart pointer to the FilterPolicy correctly and cares about freeing underlying c++ structures. --- java/org/rocksdb/Options.java | 6 ++++++ java/org/rocksdb/test/FilterTest.java | 17 ++++++++++++++--- java/rocksjni/filter.cc | 16 +++++++++++----- java/rocksjni/portal.h | 8 +++++--- java/rocksjni/table.cc | 6 ++++-- 5 files changed, 40 insertions(+), 13 deletions(-) diff --git a/java/org/rocksdb/Options.java b/java/org/rocksdb/Options.java index e0261ff4f..741404e40 100644 --- a/java/org/rocksdb/Options.java +++ b/java/org/rocksdb/Options.java @@ -1154,6 +1154,7 @@ public class Options extends RocksObject { */ public Options setMemTableConfig(MemTableConfig config) throws RocksDBException { + memTableConfig_ = config; setMemTableFactory(nativeHandle_, config.newMemTableFactoryHandle()); return this; } @@ -1168,6 +1169,7 @@ public class Options extends RocksObject { * @throws RocksDBException */ public Options setRateLimiterConfig(RateLimiterConfig config) { + rateLimiterConfig_ = config; setRateLimiter(nativeHandle_, config.newRateLimiterHandle()); return this; } @@ -1191,6 +1193,7 @@ public class Options extends RocksObject { * @return the reference of the current Options. */ public Options setTableFormatConfig(TableFormatConfig config) { + tableFormatConfig_ = config; setTableFactory(nativeHandle_, config.newTableFactoryHandle()); return this; } @@ -2280,4 +2283,7 @@ public class Options extends RocksObject { long cacheSize_; int numShardBits_; RocksEnv env_; + MemTableConfig memTableConfig_; + TableFormatConfig tableFormatConfig_; + RateLimiterConfig rateLimiterConfig_; } diff --git a/java/org/rocksdb/test/FilterTest.java b/java/org/rocksdb/test/FilterTest.java index 00214d033..fc4fabf56 100644 --- a/java/org/rocksdb/test/FilterTest.java +++ b/java/org/rocksdb/test/FilterTest.java @@ -13,19 +13,30 @@ public class FilterTest { } public static void main(String[] args) { Options options = new Options(); - // test table config without filter + // test table config BlockBasedTableConfig blockConfig = new BlockBasedTableConfig(); - options.setTableFormatConfig(blockConfig); + options.setTableFormatConfig(new BlockBasedTableConfig(). + setFilter(new BloomFilter())); options.dispose(); + System.gc(); + System.runFinalization(); // new Bloom filter options = new Options(); blockConfig = new BlockBasedTableConfig(); blockConfig.setFilter(new BloomFilter()); options.setTableFormatConfig(blockConfig); - blockConfig.setFilter(new BloomFilter(10)); + BloomFilter bloomFilter = new BloomFilter(10); + blockConfig.setFilter(bloomFilter); options.setTableFormatConfig(blockConfig); + System.gc(); + System.runFinalization(); blockConfig.setFilter(new BloomFilter(10, false)); options.setTableFormatConfig(blockConfig); + options.dispose(); + options = null; + blockConfig = null; + System.gc(); + System.runFinalization(); System.out.println("Filter test passed"); } } diff --git a/java/rocksjni/filter.cc b/java/rocksjni/filter.cc index 1b5d368b6..2ce17d499 100644 --- a/java/rocksjni/filter.cc +++ b/java/rocksjni/filter.cc @@ -24,9 +24,12 @@ void Java_org_rocksdb_BloomFilter_createNewBloomFilter( JNIEnv* env, jobject jobj, jint bits_per_key, jboolean use_block_base_builder) { - const rocksdb::FilterPolicy* fp = rocksdb::NewBloomFilterPolicy(bits_per_key, - use_block_base_builder); - rocksdb::FilterJni::setHandle(env, jobj, fp); + rocksdb::FilterPolicy* fp = const_cast( + rocksdb::NewBloomFilterPolicy(bits_per_key, use_block_base_builder)); + std::shared_ptr *pFilterPolicy = + new std::shared_ptr; + *pFilterPolicy = std::shared_ptr(fp); + rocksdb::FilterJni::setHandle(env, jobj, pFilterPolicy); } /* @@ -35,6 +38,9 @@ void Java_org_rocksdb_BloomFilter_createNewBloomFilter( * Signature: (J)V */ void Java_org_rocksdb_Filter_disposeInternal( - JNIEnv* env, jobject jobj, jlong handle) { - delete reinterpret_cast(handle); + JNIEnv* env, jobject jobj, jlong jhandle) { + + std::shared_ptr *handle = + reinterpret_cast *>(jhandle); + handle->reset(); } diff --git a/java/rocksjni/portal.h b/java/rocksjni/portal.h index 14b2cb98a..8300a6e66 100644 --- a/java/rocksjni/portal.h +++ b/java/rocksjni/portal.h @@ -313,14 +313,16 @@ class FilterJni { } // Get the pointer to rocksdb::FilterPolicy. - static rocksdb::FilterPolicy* getHandle(JNIEnv* env, jobject jobj) { - return reinterpret_cast( + static std::shared_ptr* getHandle( + JNIEnv* env, jobject jobj) { + return reinterpret_cast + *>( env->GetLongField(jobj, getHandleFieldID(env))); } // Pass the rocksdb::FilterPolicy pointer to the java side. static void setHandle( - JNIEnv* env, jobject jobj, const rocksdb::FilterPolicy* op) { + JNIEnv* env, jobject jobj, std::shared_ptr* op) { env->SetLongField( jobj, getHandleFieldID(env), reinterpret_cast(op)); diff --git a/java/rocksjni/table.cc b/java/rocksjni/table.cc index 846526292..1582900f3 100644 --- a/java/rocksjni/table.cc +++ b/java/rocksjni/table.cc @@ -56,8 +56,10 @@ jlong Java_org_rocksdb_BlockBasedTableConfig_newTableFactoryHandle( options.block_restart_interval = block_restart_interval; options.whole_key_filtering = whole_key_filtering; if (jfilterPolicy > 0) { - options.filter_policy.reset( - reinterpret_cast(jfilterPolicy)); + std::shared_ptr *pFilterPolicy = + reinterpret_cast *>( + jfilterPolicy); + options.filter_policy = *pFilterPolicy; } options.cache_index_and_filter_blocks = cache_index_and_filter_blocks; options.hash_index_allow_collision = hash_index_allow_collision;