diff --git a/rocksdb/options/tableopts.nim b/rocksdb/options/tableopts.nim index ec8e31d..c942b01 100644 --- a/rocksdb/options/tableopts.nim +++ b/rocksdb/options/tableopts.nim @@ -7,14 +7,12 @@ type TableOptionsRef* = ref object cPtr*: TableOptionsPtr cache: CacheRef - filterPolicy: FilterPolicyRef autoClose*: bool # if true then close will be called when it's parent is closed FilterPolicyPtr* = ptr rocksdb_filterpolicy_t FilterPolicyRef* = ref object cPtr*: FilterPolicyPtr - autoClose*: bool # if true then close will be called when it's parent is closed IndexType* {.pure.} = enum binarySearch = rocksdb_block_based_table_index_type_binary_search @@ -27,16 +25,13 @@ type rocksdb_block_based_table_data_block_index_type_binary_search_and_hash proc createRibbon*(bitsPerKey: float, autoClose = false): FilterPolicyRef = - FilterPolicyRef( - cPtr: rocksdb_filterpolicy_create_ribbon(bitsPerKey), autoClose: autoClose - ) + FilterPolicyRef(cPtr: rocksdb_filterpolicy_create_ribbon(bitsPerKey)) proc createRibbonHybrid*( bitsPerKey: float, bloomBeforeLevel: int = 0, autoClose = false ): FilterPolicyRef = FilterPolicyRef( - cPtr: rocksdb_filterpolicy_create_ribbon_hybrid(bitsPerKey, bloomBeforeLevel.cint), - autoClose: autoClose, + cPtr: rocksdb_filterpolicy_create_ribbon_hybrid(bitsPerKey, bloomBeforeLevel.cint) ) proc isClosed*(policy: FilterPolicyRef): bool = @@ -59,7 +54,6 @@ proc close*(opts: TableOptionsRef) = opts.cPtr = nil autoCloseNonNil(opts.cache) - autoCloseNonNil(opts.filterPolicy) template opt(nname, ntyp, ctyp: untyped) = proc `nname=`*(opts: TableOptionsRef, value: ntyp) = @@ -95,11 +89,15 @@ proc `blockCache=`*(opts: TableOptionsRef, cache: CacheRef) = proc `filterPolicy=`*(opts: TableOptionsRef, policy: FilterPolicyRef) = doAssert not opts.isClosed() - doAssert opts.filterPolicy.isNil() - # don't allow overwriting an existing policy which could leak memory + # Destroys the existing policy if there is one attached to the table options + # and takes ownership of the passed in policy. After this call, the TableOptionsRef + # is responsible for cleaning up the policy when it is no longer needed + # so we set the filter policy to nil so that isClosed() will return true + # and prevent the filter policy from being double freed which was causing a seg fault. + # See here: https://github.com/facebook/rocksdb/blob/22fe23edc89e9842ed72b613de172cd80d3b00da/include/rocksdb/filter_policy.h#L152 rocksdb_block_based_options_set_filter_policy(opts.cPtr, policy.cPtr) - opts.filterPolicy = policy + policy.cPtr = nil proc defaultTableOptions*(autoClose = false): TableOptionsRef = # https://github.com/facebook/rocksdb/wiki/Setup-Options-and-Basic-Tuning#other-general-options diff --git a/rocksdb/rocksdb.nim b/rocksdb/rocksdb.nim index e757313..756d955 100644 --- a/rocksdb/rocksdb.nim +++ b/rocksdb/rocksdb.nim @@ -417,10 +417,7 @@ proc close*(db: RocksDbRef) = # opts should be closed after the database is closed autoCloseNonNil(db.dbOpts) autoCloseNonNil(db.readOpts) - - for cfDesc in db.cfDescriptors: - if cfDesc.autoClose: - cfDesc.close() + autoCloseAll(db.cfDescriptors) if db of RocksDbReadWriteRef: let db = RocksDbReadWriteRef(db)